[PATCH] D122789: [compiler-rt] [scudo] Use -mcrc32 on x86 when available

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In D122789#3447413 , @pengfei wrote:

> In D122789#3446865 , @MaskRay wrote:
>
>> To kurly (original Gentoo reporter):
>>
>>   printf '#include \n#include \nuint32_t 
>> computeHardwareCRC32(uint32_t Crc, uint32_t Data) { return 
>> _mm_crc32_u32(Crc, Data); }' > a.c
>>
>>
>>
>>   % clang -m32 -march=i686 -msse4.2 -c a.c  # seems to compile fine
>>   % gcc -m32 -march=i686 -msse4.2 -c a.c
>>   % gcc -m32 -march=i686 -mcrc32 -c a.c
>>   In file included from a.c:1:
>>   a.c: In function ‘computeHardwareCRC32’:
>>   /usr/lib/gcc/x86_64-linux-gnu/11/include/smmintrin.h:839:1: error: 
>> inlining failed in call to ‘always_inline’ ‘_mm_crc32_u32’: target specific 
>> option mismatch
>> 839 | _mm_crc32_u32 (unsigned int __C, unsigned int __V)
>> | ^
>>   a.c:3:69: note: called from here
>>   3 | uint32_t computeHardwareCRC32(uint32_t Crc, uint32_t Data) { 
>> return _mm_crc32_u32(Crc, Data); }
>> |
>>  ^~~~
>>
>> I have some older GCC and latest GCC (2022-04, multilib), `gcc -m32 
>> -march=i686 -msse4.2 -c a.c`  builds while `-mcrc32` doesn't.
>>
>> I suspect we should revert the `-mcrc32` change. The `__CRC32__` macro may 
>> be fine.
>
> Thanks for the infromation. I got the same result. That's weird. I believe at 
> some point, GCC supported `-mcrc32`. @hjl.tools, did GCC intentionally remove 
> the support for `-mcrc32`?

"gcc -m32 -march=i686 -mcrc32" works with GCC 11.2: 
https://godbolt.org/z/nWzoofeKs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122789/new/

https://reviews.llvm.org/D122789

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In D108643#2999724 , @aaron.ballman 
wrote:

> In D108643#2991995 , @hjl.tools 
> wrote:
>
 The choice that high bits are unspecified rather than extended is an 
 interesting one.  Can you speak to that?  That's good for +, -, *, &, |, 
 ^, <<, and narrowing conversions, but bad for ==, <, /, >>, and widening 
 conversions.
>>>
>>> I've added @hjl.tools to the review for his opinions, as he was the primary 
>>> driver for the x64 ABI proposal. HJ, can you help me out here?
>>
>> Please follow up x86-64 psABI proposal with any feedbacks:
>>
>> https://groups.google.com/g/x86-64-abi/c/XQiSj-zU3w8
>
> We don't have feedback yet, @hjl.tools; we're asking for rationale on the 
> behavior of unused bits in the proposed psABI for x86-64. Can you help us 
> understand why the bits are unspecified rather than extended and whether 
> there are potential performance concerns from this decision (before it gets 
> solidified)? Thanks!

It was suggested by people who proposed _BitInt.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108643/new/

https://reviews.llvm.org/D108643

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

>> The choice that high bits are unspecified rather than extended is an 
>> interesting one.  Can you speak to that?  That's good for +, -, *, &, |, ^, 
>> <<, and narrowing conversions, but bad for ==, <, /, >>, and widening 
>> conversions.
>
> I've added @hjl.tools to the review for his opinions, as he was the primary 
> driver for the x64 ABI proposal. HJ, can you help me out here?

Please follow up x86-64 psABI proposal with any feedbacks:

https://groups.google.com/g/x86-64-abi/c/XQiSj-zU3w8


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108643/new/

https://reviews.llvm.org/D108643

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105462: [X86] Add CRC32 feature.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added inline comments.



Comment at: llvm/lib/Support/X86TargetParser.cpp:531
 constexpr FeatureBitset ImpliedFeaturesSSSE3 = FeatureSSE3;
 constexpr FeatureBitset ImpliedFeaturesSSE4_1 = FeatureSSSE3;
 constexpr FeatureBitset ImpliedFeaturesSSE4_2 = FeatureSSE4_1;

pengfei wrote:
> Can we let `ImpliedFeaturesSSE4_1 = FeatureSSSE3 | FeaturesCRC32` so that we 
> don't need to add `crc32` on sse4.1 and above?
SSE4.1 implies CRC32.  But CRC32 shouldn't imply SSE4.1.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105462/new/

https://reviews.llvm.org/D105462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105462: [X86] Add CRC32 feature.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added inline comments.



Comment at: clang/lib/Basic/Targets/X86.cpp:159
+  // Enable CRC32 if SSE4.2 is enabled.
+  // NOTE: In conformance with GCC behavior, CRC32 is still available even if
+  // it's explicitly disabled.

craig.topper wrote:
> hjl.tools wrote:
> > tianqing wrote:
> > > craig.topper wrote:
> > > > This doesn't seem to be true. It causes gcc to crash. 
> > > > https://godbolt.org/z/39rEbsejh
> > > Well I was using GCC 11.1, it compiles.
> > > 
> > > The way I see it, crash means a bug (not surprising since it's trunk), 
> > > and can be interpreted as incompletely defined behavior until it's fixed.
> > > 
> > > Some tests on GCC trunk:
> > > 1. -msse4.2: Pass - sse4.2 enables crc32.
> > > 2. -mcrc32 -mno-sse4.2: Pass - no-sse4.2 doesn't disable crc32.
> > > 3. -msse4.2 -mno-sse4.2: Error - no-sse4.2 disables crc32.
> > > 4. -mno-crc32 -msse4.2: Crash - undefined behavior
> > > 5. -msse4.2 -mno-crc32: Crash - undefined behavior
> > > 
> > > 
> > > It's hard to extract some consistent underlying logic from the GCC 
> > > results.
> > I posted a patch: 
> > https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575741.html
> @hjl.tools does that turn the crash into making -mno-crc32 into making crc32 
> instruction disabled?
Correct.  GCC issues an error now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105462/new/

https://reviews.llvm.org/D105462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105462: [X86] Add CRC32 feature.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added inline comments.



Comment at: clang/lib/Basic/Targets/X86.cpp:159
+  // Enable CRC32 if SSE4.2 is enabled.
+  // NOTE: In conformance with GCC behavior, CRC32 is still available even if
+  // it's explicitly disabled.

tianqing wrote:
> craig.topper wrote:
> > This doesn't seem to be true. It causes gcc to crash. 
> > https://godbolt.org/z/39rEbsejh
> Well I was using GCC 11.1, it compiles.
> 
> The way I see it, crash means a bug (not surprising since it's trunk), and 
> can be interpreted as incompletely defined behavior until it's fixed.
> 
> Some tests on GCC trunk:
> 1. -msse4.2: Pass - sse4.2 enables crc32.
> 2. -mcrc32 -mno-sse4.2: Pass - no-sse4.2 doesn't disable crc32.
> 3. -msse4.2 -mno-sse4.2: Error - no-sse4.2 disables crc32.
> 4. -mno-crc32 -msse4.2: Crash - undefined behavior
> 5. -msse4.2 -mno-crc32: Crash - undefined behavior
> 
> 
> It's hard to extract some consistent underlying logic from the GCC results.
I posted a patch: 
https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575741.html


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105462/new/

https://reviews.llvm.org/D105462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105462: [X86] Add CRC32 feature.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:84
   "Enable SSE 4.2 instructions",
-  [FeatureSSE41]>;
+  [FeatureSSE41, FeatureCRC32]>;
 // The MMX subtarget feature is separate from the rest of the SSE features

craig.topper wrote:
> hjl.tools wrote:
> > craig.topper wrote:
> > > tianqing wrote:
> > > > craig.topper wrote:
> > > > > Doesn't this make -mno-crc32 disable sse4.2? Is that what we want?
> > > > > 
> > > > > Or should we be doing this like popcnt where we loosely enable it at 
> > > > > the end of X86TargetInfo::initFeatureMap
> > > > It does. But it's not a big deal in this case. The scenario described 
> > > > in the commit message doesn't require crc32 capable to be disabled 
> > > > separately.
> > > What does gcc do?
> > ```
> > [hjl@gnu-skx-1 gcc]$ cat /tmp/x.c
> > #include 
> > 
> > int
> > foo (int x, char c)
> > {
> >   return __crc32b (x, c);
> > }
> > [hjl@gnu-skx-1 gcc]$ /usr/gcc-12.0.0-x32/bin/gcc -S -O2 /tmp/x.c -S 
> > -msse4.2 
> > [hjl@gnu-skx-1 gcc]$ /usr/gcc-12.0.0-x32/bin/gcc -S -O2 /tmp/x.c -S -mcrc32
> > [hjl@gnu-skx-1 gcc]$ /usr/gcc-12.0.0-x32/bin/gcc -S -O2 /tmp/x.c -S 
> > -msse4.2 -mno-crc32
> > In file included from 
> > /usr/gcc-12.0.0-x32/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/x86gprintrin.h:27,
> >  from 
> > /usr/gcc-12.0.0-x32/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/x86intrin.h:27,
> >  from /tmp/x.c:1:
> > /tmp/x.c: In function ??foo??:
> > /usr/gcc-12.0.0-x32/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/ia32intrin.h:63:1:
> >  error: inlining failed in call to ??always_inline?? ??__crc32b??: target 
> > specific option mismatch
> >63 | __crc32b (unsigned int __C, unsigned char __V)
> >   | ^~~~
> > /tmp/x.c:6:10: note: called from here
> > 6 |   return __crc32b (x, c);
> >   |  ^~~
> > [hjl@gnu-skx-1 gcc]$ 
> > ```
> What does gcc do for an sse4.2 intrinsic that isn't crc32 with "-msse4.2 
> -mno-crc32"?
-mno-crc32 has no impact on non-crc32 intrinsics.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105462/new/

https://reviews.llvm.org/D105462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105462: [X86] Add CRC32 feature.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:84
   "Enable SSE 4.2 instructions",
-  [FeatureSSE41]>;
+  [FeatureSSE41, FeatureCRC32]>;
 // The MMX subtarget feature is separate from the rest of the SSE features

craig.topper wrote:
> tianqing wrote:
> > craig.topper wrote:
> > > Doesn't this make -mno-crc32 disable sse4.2? Is that what we want?
> > > 
> > > Or should we be doing this like popcnt where we loosely enable it at the 
> > > end of X86TargetInfo::initFeatureMap
> > It does. But it's not a big deal in this case. The scenario described in 
> > the commit message doesn't require crc32 capable to be disabled separately.
> What does gcc do?
```
[hjl@gnu-skx-1 gcc]$ cat /tmp/x.c
#include 

int
foo (int x, char c)
{
  return __crc32b (x, c);
}
[hjl@gnu-skx-1 gcc]$ /usr/gcc-12.0.0-x32/bin/gcc -S -O2 /tmp/x.c -S -msse4.2 
[hjl@gnu-skx-1 gcc]$ /usr/gcc-12.0.0-x32/bin/gcc -S -O2 /tmp/x.c -S -mcrc32
[hjl@gnu-skx-1 gcc]$ /usr/gcc-12.0.0-x32/bin/gcc -S -O2 /tmp/x.c -S -msse4.2 
-mno-crc32
In file included from 
/usr/gcc-12.0.0-x32/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/x86gprintrin.h:27,
 from 
/usr/gcc-12.0.0-x32/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/x86intrin.h:27,
 from /tmp/x.c:1:
/tmp/x.c: In function ??foo??:
/usr/gcc-12.0.0-x32/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/ia32intrin.h:63:1:
 error: inlining failed in call to ??always_inline?? ??__crc32b??: target 
specific option mismatch
   63 | __crc32b (unsigned int __C, unsigned char __V)
  | ^~~~
/tmp/x.c:6:10: note: called from here
6 |   return __crc32b (x, c);
  |  ^~~
[hjl@gnu-skx-1 gcc]$ 
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105462/new/

https://reviews.llvm.org/D105462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In D99708#2664372 , @craig.topper 
wrote:

> In D99708#2664351 , @hjl.tools wrote:
>
>> In D99708#2664218 , @craig.topper 
>> wrote:
>>
>>> In D99708#2664164 , @hjl.tools 
>>> wrote:
>>>
 In D99708#2664076 , @LuoYuanke 
 wrote:

> In D99708#2663989 , 
> @craig.topper wrote:
>
>> A user interrupt is different than a regular interrupt right? It doesn't 
>> make sense that we would change the behavior of the interrupt calling 
>> convention just because the the user interrupt instructions are enabled. 
>> That would occur just from passing a -march for a newer CPU wouldn't it?
>
> Maybe need support another attribute "__attribute__ ((user_interrupt))" 
> for functions? However this is what gcc does 
> (https://gcc.godbolt.org/z/8ojTMG6bT).

 Since there won't be both user interrupt handler and kernel interrupt 
 handler in the source, there is no need for another
 attribute.   We discussed that kernel might need to use UINTR 
 instructions.  We decided that kernel could use inline asm
 statements if needed.
>>>
>>> So if write kernel code and compile with -march=haswell today, I get IRET. 
>>> If tomorrow I change my command line to -march=sapphirerapids, now my 
>>> kernel interrupt code generates user interrupt instructions. That seems 
>>> surprising.
>>
>> -mcmodel=kernel should disable uiret.:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870
>
> That makes sense. Can we put that in this patch?

The feedback is that don't enable UINTR for kernel build.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99708/new/

https://reviews.llvm.org/D99708

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In D99708#2664218 , @craig.topper 
wrote:

> In D99708#2664164 , @hjl.tools wrote:
>
>> In D99708#2664076 , @LuoYuanke 
>> wrote:
>>
>>> In D99708#2663989 , @craig.topper 
>>> wrote:
>>>
 A user interrupt is different than a regular interrupt right? It doesn't 
 make sense that we would change the behavior of the interrupt calling 
 convention just because the the user interrupt instructions are enabled. 
 That would occur just from passing a -march for a newer CPU wouldn't it?
>>>
>>> Maybe need support another attribute "__attribute__ ((user_interrupt))" for 
>>> functions? However this is what gcc does 
>>> (https://gcc.godbolt.org/z/8ojTMG6bT).
>>
>> Since there won't be both user interrupt handler and kernel interrupt 
>> handler in the source, there is no need for another
>> attribute.   We discussed that kernel might need to use UINTR instructions.  
>> We decided that kernel could use inline asm
>> statements if needed.
>
> So if write kernel code and compile with -march=haswell today, I get IRET. If 
> tomorrow I change my command line to -march=sapphirerapids, now my kernel 
> interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99708/new/

https://reviews.llvm.org/D99708

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In D99708#2664076 , @LuoYuanke wrote:

> In D99708#2663989 , @craig.topper 
> wrote:
>
>> A user interrupt is different than a regular interrupt right? It doesn't 
>> make sense that we would change the behavior of the interrupt calling 
>> convention just because the the user interrupt instructions are enabled. 
>> That would occur just from passing a -march for a newer CPU wouldn't it?
>
> Maybe need support another attribute "__attribute__ ((user_interrupt))" for 
> functions? However this is what gcc does 
> (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler 
in the source, there is no need for another
attribute.   We discussed that kernel might need to use UINTR instructions.  We 
decided that kernel could use inline asm
statements if needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99708/new/

https://reviews.llvm.org/D99708

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79617: Add cet.h for writing CET-enabled assembly code

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In D79617#2045552 , @rsmith wrote:

> I would like a specification for this header to be added somewhere. We 
> shouldn't be implementing random things with no specification. (Suppose 
> someone claims that our `` is wrong in some way. How would we know 
> whether they're right?)
>
> Ideally, I'd also like this header to be installed somewhere where we look 
> for assembler-with-cpp preprocessing but not for regular compilation; it 
> doesn't make sense to me to pollute the header namespace for all C and C++ 
> compilations with a header that is not meaningful in C and C++. But knowing 
> whether that change is correct depends on having, you know, a specification.


 from GCC is as close as you can get for a reference 
implementation/specification.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79617/new/

https://reviews.llvm.org/D79617



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1287510, @echristo wrote:

> In https://reviews.llvm.org/D53919#1282994, @hjl.tools wrote:
>
> > In https://reviews.llvm.org/D53919#1282952, @efriedma wrote:
> >
> > > With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if it 
> > > would be helpful):
> >
> >
> > Please try clang 2.6 on both testcases.
>
>
> From the releases:
>
> 23 Oct 2009   2.6
>
> ... why would you care about a 9 year old version of clang?


It is about ABI consistency.  When AVX isn't enabled, the old and compilers 
should
use the same calling convention.


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1282952, @efriedma wrote:

> With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if it would 
> be helpful):


Please try clang 2.6 on both testcases.


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1282811, @efriedma wrote:

> No, AVX512 wasn't even announced when clang 3.3 came out.


Try this with clang 3.3 and clang 7.0.

[hjl@gnu-cfl-1 tmp]$ cat z.c
typedef int __attribute__((mode(SI))) si;
typedef si __attribute__((vector_size (64))) v;

v
foo (void)
{
 v y = {'0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f' };
 return y;
}
[hjl@gnu-cfl-1 tmp]$ gcc -S -O2 z.c
z.c: In function ‘foo’:
z.c:6:1: warning: AVX512F vector return without AVX512F enabled changes the ABI 
[-Wpsabi]
 {
 ^
[hjl@gnu-cfl-1 tmp]$


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1282797, @efriedma wrote:

> I just tried clang 3.3, and as far as I can tell it behaves the same way as 
> trunk.
>
> If the argument is that we should be compatible with gcc on Linux, that's 
> fine, but we should explicitly state that in a comment.


Does clang 3.3 support AVX512?


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1282748, @efriedma wrote:

> The ABI document before AVX was introduced didn't say anything at all about 
> 256-bit vectors.  So we're talking about compatibility with some other 
> compiler.  Which compiler?


X86-64 compilers created before AVX was introduced.   Say clang 1.0 doesn't 
know AVX and returns it in memory.  So should clang 7.0 with -mno-avx.


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1282734, @efriedma wrote:

> Whatever you call it, the question remains; what specification and/or 
> compiler are we trying to be compatible with?


The ABI before AVX was introduced.


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added a comment.

In https://reviews.llvm.org/D53919#1282692, @efriedma wrote:

> As far as I know, according to the ABI docs, it's impossible to return a 
> 256-bit vector from a function if AVX is disabled.
>
> Given that we aren't rejecting the testcase, we're effectively implementing a 
> new "no-AVX" ABI variant.  That variant is not defined anywhere, so we might 
> as well pick the fastest convention, returning the value in registers.  I'm 
> not sure why you would want to return the vector in memory instead.


Not  a new "no-AVX" ABI variant.  It should be pre-AVX ABI.


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value.

[H.J Lu via Phabricator via cfe-commits]

hjl.tools added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:2862
   // place naturally.
-  if (!isAggregateTypeForABI(Ty)) {
+  if (!isAggregateTypeForABI(Ty) && !IsIllegalVectorType(Ty)) {
 // Treat an enum type as its underlying type.

Doesn't -m32 have the same issue?


Repository:
  rC Clang

https://reviews.llvm.org/D53919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits