RE: r262688 - [X86] Pass __m64 types via SSE registers for GCC compatibility
> 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
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, Paulwrote: >> 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
> 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
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-commitswrote: >> 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