RE: r262688 - [X86] Pass __m64 types via SSE registers for GCC compatibility

2016-03-04 Thread Robinson, Paul via cfe-commits
> Ah, great! I always love it when people document their ABIs.
> 
> Is your ABI document public? If so, could you add it to
> docs/CompilerWriterInfo.rst?

I don't think I was aware of that page...  AFAIK ours is not available
on-line, but I can raise the question internally.
--paulr

> 
> On Fri, Mar 4, 2016 at 11:54 AM, Robinson, Paul
>  wrote:
> >> It'd be nice to have a comment here that mentions that the clang
> >> behavior which is being preserved for Darwin, FreeBSD, and PS4 is a
> >> *bug* which is being intentionally left unfixed. The previous clang
> >> behavior directly contradicts the x86_64 ABI document, which I believe
> >> all of these platforms claim to follow. :)
> >
> > Well, PS4 uses x86_64 ABI as a base document, but we have a handful of
> > variances.  We had already documented this one to our licensees.  So,
> > from our perspective, it's not a bug, it's a feature. :-)  Describing
> > it as a bug (at least for PS4) would be technically incorrect.
> > --paulr
> >
> >>
> >> On Fri, Mar 4, 2016 at 2:03 AM, Robinson, Paul via cfe-commits
> >>  wrote:
> >> >> To: cfe-commits@lists.llvm.org
> >> >> Subject: r262688 - [X86] Pass __m64 types via SSE registers for GCC
> >> >> compatibility
> >> >>
> >> >> Author: majnemer
> >> >> Date: Thu Mar  3 23:26:16 2016
> >> >> New Revision: 262688
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=262688=rev
> >> >> Log:
> >> >> [X86] Pass __m64 types via SSE registers for GCC compatibility
> >> >>
> >> >> For compatibility with GCC, classify __m64 as SSE.
> >> >> However, clang is a platform compiler for certain targets; retain
> our
> >> >> old behavior on those targets: classify __m64 as integer.
> >> >
> >> > Thank you very much for that!
> >> > --paulr
> >> >
> >> >>
> >> >> This fixes PR26832.
> >> >>
> >> >> Modified:
> >> >> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> >> >> cfe/trunk/test/CodeGen/3dnow-builtins.c
> >> >> cfe/trunk/test/CodeGen/x86_64-arguments.c
> >> >>
> >> >> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> >> >> URL: http://llvm.org/viewvc/llvm-
> >> >>
> >>
> project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=262688=262687=26268
> >> >> 8=diff
> >> >>
> >>
> ==
> >> >> 
> >> >> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> >> >> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Mar  3 23:26:16 2016
> >> >> @@ -1857,6 +1857,17 @@ class X86_64ABIInfo : public ABIInfo {
> >> >>  return !getTarget().getTriple().isOSDarwin();
> >> >>}
> >> >>
> >> >> +  /// GCC classifies <1 x long long> as SSE but compatibility with
> >> older
> >> >> clang
> >> >> +  // compilers require us to classify it as INTEGER.
> >> >> +  bool classifyIntegerMMXAsSSE() const {
> >> >> +const llvm::Triple  = getTarget().getTriple();
> >> >> +if (Triple.isOSDarwin() || Triple.getOS() == llvm::Triple::PS4)
> >> >> +  return false;
> >> >> +if (Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 10)
> >> >> +  return false;
> >> >> +return true;
> >> >> +  }
> >> >> +
> >> >>X86AVXABILevel AVXLevel;
> >> >>// Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit
> pointers
> >> on
> >> >>// 64-bit hardware.
> >> >> @@ -2298,15 +2309,20 @@ void X86_64ABIInfo::classify(QualType Ty
> >> >>if (EB_Lo != EB_Hi)
> >> >>  Hi = Lo;
> >> >>  } else if (Size == 64) {
> >> >> +  QualType ElementType = VT->getElementType();
> >> >> +
> >> >>// gcc passes <1 x double> in memory. :(
> >> >> -  if (VT->getElementType()-
> >> >> >isSpecificBuiltinType(BuiltinType::Double))
> >> >> +  if (ElementType->isSpecificBuiltinType(BuiltinType::Double))
> >> >>  return;
> >> >>
> >> >> -  // gcc passes <1 x long long> as INTEGER.
> >> >> -  if (VT->getElementType()-
> >> >> >isSpecificBuiltinType(BuiltinType::LongLong) ||
> >> >> -  VT->getElementType()-
> >> >> >isSpecificBuiltinType(BuiltinType::ULongLong) ||
> >> >> -  VT->getElementType()-
> >> >isSpecificBuiltinType(BuiltinType::Long)
> >> >> ||
> >> >> -  VT->getElementType()-
> >> >> >isSpecificBuiltinType(BuiltinType::ULong))
> >> >> +  // gcc passes <1 x long long> as SSE but clang used to
> >> >> unconditionally
> >> >> +  // pass them as integer.  For platforms where clang is the de
> >> facto
> >> >> +  // platform compiler, we must continue to use integer.
> >> >> +  if (!classifyIntegerMMXAsSSE() &&
> >> >> +  (ElementType-
> >isSpecificBuiltinType(BuiltinType::LongLong)
> >> ||
> >> >> +   ElementType-
> >isSpecificBuiltinType(BuiltinType::ULongLong)
> >> ||
> >> >> +   ElementType->isSpecificBuiltinType(BuiltinType::Long) ||
> >> >> +   ElementType->isSpecificBuiltinType(BuiltinType::ULong)))
> >> >>  Current = Integer;
> >> >>else
> >> >>  Current = SSE;
> >> >>
> 

Re: r262688 - [X86] Pass __m64 types via SSE registers for GCC compatibility

2016-03-04 Thread James Y Knight via cfe-commits
Ah, great! I always love it when people document their ABIs.

Is your ABI document public? If so, could you add it to
docs/CompilerWriterInfo.rst?

On Fri, Mar 4, 2016 at 11:54 AM, Robinson, Paul
 wrote:
>> It'd be nice to have a comment here that mentions that the clang
>> behavior which is being preserved for Darwin, FreeBSD, and PS4 is a
>> *bug* which is being intentionally left unfixed. The previous clang
>> behavior directly contradicts the x86_64 ABI document, which I believe
>> all of these platforms claim to follow. :)
>
> Well, PS4 uses x86_64 ABI as a base document, but we have a handful of
> variances.  We had already documented this one to our licensees.  So,
> from our perspective, it's not a bug, it's a feature. :-)  Describing
> it as a bug (at least for PS4) would be technically incorrect.
> --paulr
>
>>
>> On Fri, Mar 4, 2016 at 2:03 AM, Robinson, Paul via cfe-commits
>>  wrote:
>> >> To: cfe-commits@lists.llvm.org
>> >> Subject: r262688 - [X86] Pass __m64 types via SSE registers for GCC
>> >> compatibility
>> >>
>> >> Author: majnemer
>> >> Date: Thu Mar  3 23:26:16 2016
>> >> New Revision: 262688
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=262688=rev
>> >> Log:
>> >> [X86] Pass __m64 types via SSE registers for GCC compatibility
>> >>
>> >> For compatibility with GCC, classify __m64 as SSE.
>> >> However, clang is a platform compiler for certain targets; retain our
>> >> old behavior on those targets: classify __m64 as integer.
>> >
>> > Thank you very much for that!
>> > --paulr
>> >
>> >>
>> >> This fixes PR26832.
>> >>
>> >> Modified:
>> >> cfe/trunk/lib/CodeGen/TargetInfo.cpp
>> >> cfe/trunk/test/CodeGen/3dnow-builtins.c
>> >> cfe/trunk/test/CodeGen/x86_64-arguments.c
>> >>
>> >> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
>> >> URL: http://llvm.org/viewvc/llvm-
>> >>
>> project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=262688=262687=26268
>> >> 8=diff
>> >>
>> ==
>> >> 
>> >> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
>> >> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Mar  3 23:26:16 2016
>> >> @@ -1857,6 +1857,17 @@ class X86_64ABIInfo : public ABIInfo {
>> >>  return !getTarget().getTriple().isOSDarwin();
>> >>}
>> >>
>> >> +  /// GCC classifies <1 x long long> as SSE but compatibility with
>> older
>> >> clang
>> >> +  // compilers require us to classify it as INTEGER.
>> >> +  bool classifyIntegerMMXAsSSE() const {
>> >> +const llvm::Triple  = getTarget().getTriple();
>> >> +if (Triple.isOSDarwin() || Triple.getOS() == llvm::Triple::PS4)
>> >> +  return false;
>> >> +if (Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 10)
>> >> +  return false;
>> >> +return true;
>> >> +  }
>> >> +
>> >>X86AVXABILevel AVXLevel;
>> >>// Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers
>> on
>> >>// 64-bit hardware.
>> >> @@ -2298,15 +2309,20 @@ void X86_64ABIInfo::classify(QualType Ty
>> >>if (EB_Lo != EB_Hi)
>> >>  Hi = Lo;
>> >>  } else if (Size == 64) {
>> >> +  QualType ElementType = VT->getElementType();
>> >> +
>> >>// gcc passes <1 x double> in memory. :(
>> >> -  if (VT->getElementType()-
>> >> >isSpecificBuiltinType(BuiltinType::Double))
>> >> +  if (ElementType->isSpecificBuiltinType(BuiltinType::Double))
>> >>  return;
>> >>
>> >> -  // gcc passes <1 x long long> as INTEGER.
>> >> -  if (VT->getElementType()-
>> >> >isSpecificBuiltinType(BuiltinType::LongLong) ||
>> >> -  VT->getElementType()-
>> >> >isSpecificBuiltinType(BuiltinType::ULongLong) ||
>> >> -  VT->getElementType()-
>> >isSpecificBuiltinType(BuiltinType::Long)
>> >> ||
>> >> -  VT->getElementType()-
>> >> >isSpecificBuiltinType(BuiltinType::ULong))
>> >> +  // gcc passes <1 x long long> as SSE but clang used to
>> >> unconditionally
>> >> +  // pass them as integer.  For platforms where clang is the de
>> facto
>> >> +  // platform compiler, we must continue to use integer.
>> >> +  if (!classifyIntegerMMXAsSSE() &&
>> >> +  (ElementType->isSpecificBuiltinType(BuiltinType::LongLong)
>> ||
>> >> +   ElementType->isSpecificBuiltinType(BuiltinType::ULongLong)
>> ||
>> >> +   ElementType->isSpecificBuiltinType(BuiltinType::Long) ||
>> >> +   ElementType->isSpecificBuiltinType(BuiltinType::ULong)))
>> >>  Current = Integer;
>> >>else
>> >>  Current = SSE;
>> >>
>> >> Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c
>> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/3dnow-
>> >> builtins.c?rev=262688=262687=262688=diff
>> >>
>> ==
>> >> 
>> >> --- cfe/trunk/test/CodeGen/3dnow-builtins.c (original)
>> >> +++ 

RE: r262688 - [X86] Pass __m64 types via SSE registers for GCC compatibility

2016-03-04 Thread Robinson, Paul via cfe-commits
> It'd be nice to have a comment here that mentions that the clang
> behavior which is being preserved for Darwin, FreeBSD, and PS4 is a
> *bug* which is being intentionally left unfixed. The previous clang
> behavior directly contradicts the x86_64 ABI document, which I believe
> all of these platforms claim to follow. :)

Well, PS4 uses x86_64 ABI as a base document, but we have a handful of
variances.  We had already documented this one to our licensees.  So,
from our perspective, it's not a bug, it's a feature. :-)  Describing
it as a bug (at least for PS4) would be technically incorrect.
--paulr

> 
> On Fri, Mar 4, 2016 at 2:03 AM, Robinson, Paul via cfe-commits
>  wrote:
> >> To: cfe-commits@lists.llvm.org
> >> Subject: r262688 - [X86] Pass __m64 types via SSE registers for GCC
> >> compatibility
> >>
> >> Author: majnemer
> >> Date: Thu Mar  3 23:26:16 2016
> >> New Revision: 262688
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=262688=rev
> >> Log:
> >> [X86] Pass __m64 types via SSE registers for GCC compatibility
> >>
> >> For compatibility with GCC, classify __m64 as SSE.
> >> However, clang is a platform compiler for certain targets; retain our
> >> old behavior on those targets: classify __m64 as integer.
> >
> > Thank you very much for that!
> > --paulr
> >
> >>
> >> This fixes PR26832.
> >>
> >> Modified:
> >> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> >> cfe/trunk/test/CodeGen/3dnow-builtins.c
> >> cfe/trunk/test/CodeGen/x86_64-arguments.c
> >>
> >> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> >> URL: http://llvm.org/viewvc/llvm-
> >>
> project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=262688=262687=26268
> >> 8=diff
> >>
> ==
> >> 
> >> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> >> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Mar  3 23:26:16 2016
> >> @@ -1857,6 +1857,17 @@ class X86_64ABIInfo : public ABIInfo {
> >>  return !getTarget().getTriple().isOSDarwin();
> >>}
> >>
> >> +  /// GCC classifies <1 x long long> as SSE but compatibility with
> older
> >> clang
> >> +  // compilers require us to classify it as INTEGER.
> >> +  bool classifyIntegerMMXAsSSE() const {
> >> +const llvm::Triple  = getTarget().getTriple();
> >> +if (Triple.isOSDarwin() || Triple.getOS() == llvm::Triple::PS4)
> >> +  return false;
> >> +if (Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 10)
> >> +  return false;
> >> +return true;
> >> +  }
> >> +
> >>X86AVXABILevel AVXLevel;
> >>// Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers
> on
> >>// 64-bit hardware.
> >> @@ -2298,15 +2309,20 @@ void X86_64ABIInfo::classify(QualType Ty
> >>if (EB_Lo != EB_Hi)
> >>  Hi = Lo;
> >>  } else if (Size == 64) {
> >> +  QualType ElementType = VT->getElementType();
> >> +
> >>// gcc passes <1 x double> in memory. :(
> >> -  if (VT->getElementType()-
> >> >isSpecificBuiltinType(BuiltinType::Double))
> >> +  if (ElementType->isSpecificBuiltinType(BuiltinType::Double))
> >>  return;
> >>
> >> -  // gcc passes <1 x long long> as INTEGER.
> >> -  if (VT->getElementType()-
> >> >isSpecificBuiltinType(BuiltinType::LongLong) ||
> >> -  VT->getElementType()-
> >> >isSpecificBuiltinType(BuiltinType::ULongLong) ||
> >> -  VT->getElementType()-
> >isSpecificBuiltinType(BuiltinType::Long)
> >> ||
> >> -  VT->getElementType()-
> >> >isSpecificBuiltinType(BuiltinType::ULong))
> >> +  // gcc passes <1 x long long> as SSE but clang used to
> >> unconditionally
> >> +  // pass them as integer.  For platforms where clang is the de
> facto
> >> +  // platform compiler, we must continue to use integer.
> >> +  if (!classifyIntegerMMXAsSSE() &&
> >> +  (ElementType->isSpecificBuiltinType(BuiltinType::LongLong)
> ||
> >> +   ElementType->isSpecificBuiltinType(BuiltinType::ULongLong)
> ||
> >> +   ElementType->isSpecificBuiltinType(BuiltinType::Long) ||
> >> +   ElementType->isSpecificBuiltinType(BuiltinType::ULong)))
> >>  Current = Integer;
> >>else
> >>  Current = SSE;
> >>
> >> Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/3dnow-
> >> builtins.c?rev=262688=262687=262688=diff
> >>
> ==
> >> 
> >> --- cfe/trunk/test/CodeGen/3dnow-builtins.c (original)
> >> +++ cfe/trunk/test/CodeGen/3dnow-builtins.c Thu Mar  3 23:26:16 2016
> >> @@ -1,4 +1,5 @@
> >> -// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature
> >> +3dnowa -emit-llvm -o - -Werror | FileCheck %s
> >> +// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature
> >> +3dnowa -emit-llvm -o - -Werror | FileCheck %s -check-prefix=GCC -
> check-
> >> prefix=CHECK
> >> +// RUN: 

Re: r262688 - [X86] Pass __m64 types via SSE registers for GCC compatibility

2016-03-04 Thread James Y Knight via cfe-commits
It'd be nice to have a comment here that mentions that the clang
behavior which is being preserved for Darwin, FreeBSD, and PS4 is a
*bug* which is being intentionally left unfixed. The previous clang
behavior directly contradicts the x86_64 ABI document, which I believe
all of these platforms claim to follow. :)

On Fri, Mar 4, 2016 at 2:03 AM, Robinson, Paul via cfe-commits
 wrote:
>> To: cfe-commits@lists.llvm.org
>> Subject: r262688 - [X86] Pass __m64 types via SSE registers for GCC
>> compatibility
>>
>> Author: majnemer
>> Date: Thu Mar  3 23:26:16 2016
>> New Revision: 262688
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=262688=rev
>> Log:
>> [X86] Pass __m64 types via SSE registers for GCC compatibility
>>
>> For compatibility with GCC, classify __m64 as SSE.
>> However, clang is a platform compiler for certain targets; retain our
>> old behavior on those targets: classify __m64 as integer.
>
> Thank you very much for that!
> --paulr
>
>>
>> This fixes PR26832.
>>
>> Modified:
>> cfe/trunk/lib/CodeGen/TargetInfo.cpp
>> cfe/trunk/test/CodeGen/3dnow-builtins.c
>> cfe/trunk/test/CodeGen/x86_64-arguments.c
>>
>> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=262688=262687=26268
>> 8=diff
>> ==
>> 
>> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Mar  3 23:26:16 2016
>> @@ -1857,6 +1857,17 @@ class X86_64ABIInfo : public ABIInfo {
>>  return !getTarget().getTriple().isOSDarwin();
>>}
>>
>> +  /// GCC classifies <1 x long long> as SSE but compatibility with older
>> clang
>> +  // compilers require us to classify it as INTEGER.
>> +  bool classifyIntegerMMXAsSSE() const {
>> +const llvm::Triple  = getTarget().getTriple();
>> +if (Triple.isOSDarwin() || Triple.getOS() == llvm::Triple::PS4)
>> +  return false;
>> +if (Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 10)
>> +  return false;
>> +return true;
>> +  }
>> +
>>X86AVXABILevel AVXLevel;
>>// Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers on
>>// 64-bit hardware.
>> @@ -2298,15 +2309,20 @@ void X86_64ABIInfo::classify(QualType Ty
>>if (EB_Lo != EB_Hi)
>>  Hi = Lo;
>>  } else if (Size == 64) {
>> +  QualType ElementType = VT->getElementType();
>> +
>>// gcc passes <1 x double> in memory. :(
>> -  if (VT->getElementType()-
>> >isSpecificBuiltinType(BuiltinType::Double))
>> +  if (ElementType->isSpecificBuiltinType(BuiltinType::Double))
>>  return;
>>
>> -  // gcc passes <1 x long long> as INTEGER.
>> -  if (VT->getElementType()-
>> >isSpecificBuiltinType(BuiltinType::LongLong) ||
>> -  VT->getElementType()-
>> >isSpecificBuiltinType(BuiltinType::ULongLong) ||
>> -  VT->getElementType()->isSpecificBuiltinType(BuiltinType::Long)
>> ||
>> -  VT->getElementType()-
>> >isSpecificBuiltinType(BuiltinType::ULong))
>> +  // gcc passes <1 x long long> as SSE but clang used to
>> unconditionally
>> +  // pass them as integer.  For platforms where clang is the de facto
>> +  // platform compiler, we must continue to use integer.
>> +  if (!classifyIntegerMMXAsSSE() &&
>> +  (ElementType->isSpecificBuiltinType(BuiltinType::LongLong) ||
>> +   ElementType->isSpecificBuiltinType(BuiltinType::ULongLong) ||
>> +   ElementType->isSpecificBuiltinType(BuiltinType::Long) ||
>> +   ElementType->isSpecificBuiltinType(BuiltinType::ULong)))
>>  Current = Integer;
>>else
>>  Current = SSE;
>>
>> Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/3dnow-
>> builtins.c?rev=262688=262687=262688=diff
>> ==
>> 
>> --- cfe/trunk/test/CodeGen/3dnow-builtins.c (original)
>> +++ cfe/trunk/test/CodeGen/3dnow-builtins.c Thu Mar  3 23:26:16 2016
>> @@ -1,4 +1,5 @@
>> -// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature
>> +3dnowa -emit-llvm -o - -Werror | FileCheck %s
>> +// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature
>> +3dnowa -emit-llvm -o - -Werror | FileCheck %s -check-prefix=GCC -check-
>> prefix=CHECK
>> +// RUN: %clang_cc1 %s -triple=x86_64-scei-ps4 -target-feature +3dnowa -
>> emit-llvm -o - -Werror | FileCheck %s -check-prefix=PS4 -check-
>> prefix=CHECK
>>
>>  // Don't include mm_malloc.h, it's system specific.
>>  #define __MM_MALLOC_H
>> @@ -6,151 +7,176 @@
>>  #include 
>>
>>  __m64 test_m_pavgusb(__m64 m1, __m64 m2) {
>> -  // CHECK-LABEL: define i64 @test_m_pavgusb
>> +  // PS4-LABEL: define i64 @test_m_pavgusb
>> +  // GCC-LABEL: define double @test_m_pavgusb
>>// CHECK: @llvm.x86.3dnow.pavgusb
>>return