Re: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

2019-01-18 Thread Jordan Rupprecht via cfe-commits 
Confirmed that opencv builds now, thanks :)

On Fri, Jan 18, 2019 at 10:05 AM Jordan Rupprecht 
wrote:

> Thanks! I'll rebuild clang from trunk and give it another try.
> Yes, for the second two, I think it should be a more straightforward fix a
> la https://chromium-review.googlesource.com/c/v8/v8/+/1414858
>
> On Fri, Jan 18, 2019 at 9:58 AM Topper, Craig 
> wrote:
>
>> I just committed a change to go back to only definining 
>> _XCR_XFEATURE_ENABLED_MASK
>> when clang is in MSVC compatibility mode. I don’t think I meant to define
>> it outside of that since it doesn’t appear gcc does.
>>
>>
>>
>> For the other two files, I assume you’re getting a collision of names
>> similar to what Benjamin Kramer reported on V8?
>>
>>
>>
>> *From:* Jordan Rupprecht [mailto:ruppre...@google.com]
>> *Sent:* Friday, January 18, 2019 9:30 AM
>> *To:* Topper, Craig 
>> *Cc:* cfe-commits@lists.llvm.org
>> *Subject:* Re: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on
>> non-windows platforms"
>>
>>
>>
>> Hi Craig,
>>
>> We're seeing issues building a few other things now; notably this part in
>> opencv:
>> https://github.com/opencv/opencv/blob/master/modules/core/src/system.cpp#L422
>>
>> #ifdef _XCR_XFEATURE_ENABLED_MASK // requires immintrin.h
>>
>> xcr0 = (int)_xgetbv(_XCR_XFEATURE_ENABLED_MASK);
>>
>> => error: '__builtin_ia32_xgetbv' needs target feature xsave
>>
>> We can workaround by building with -mavx... but we also have code running
>> on non-avx machines that pull in this dependency. (It likely doesn't
>> actually *use* the code, but still depends on it compiling as a transitive
>> dep).
>>
>> Is there a more accurate preprocessor macro (e.g. one that says if xsave
>> is available) we could use to selectively disable this?
>>
>>
>>
>> There are similar breakages here:
>>
>>
>> https://chromium.googlesource.com/aosp/platform/external/libchrome/+/master/base/cpu.cc#82
>>
>> https://github.com/google/sling/blob/master/third_party/jit/cpu.cc#L58
>>
>> Advice there too would be appreciated, if possible :)
>>
>>
>>
>> On Wed, Jan 16, 2019 at 3:00 PM Craig Topper via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> Author: ctopper
>> Date: Wed Jan 16 14:56:25 2019
>> New Revision: 351391
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=351391&view=rev
>> Log:
>> Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"
>>
>> V8 has been fixed now.
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/BuiltinsX86.def
>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> cfe/trunk/lib/Headers/immintrin.h
>> cfe/trunk/lib/Headers/intrin.h
>> cfe/trunk/lib/Headers/xsaveintrin.h
>> cfe/trunk/test/CodeGen/builtins-x86.c
>> cfe/trunk/test/CodeGen/x86_32-xsave.c
>> cfe/trunk/test/CodeGen/x86_64-xsave.c
>> cfe/trunk/test/Headers/ms-intrin.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351391&r1=351390&r2=351391&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
>> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jan 16 14:56:25 2019
>> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>>  // XSAVE
>>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
>> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
>> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
>> ALL_MS_LANGUAGES, "")
>> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
>> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
>> ALL_MS_LANGUAGES, "")
>>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>>
&g

Re: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

2019-01-18 Thread Jordan Rupprecht via cfe-commits 
Thanks! I'll rebuild clang from trunk and give it another try.
Yes, for the second two, I think it should be a more straightforward fix a
la https://chromium-review.googlesource.com/c/v8/v8/+/1414858

On Fri, Jan 18, 2019 at 9:58 AM Topper, Craig 
wrote:

> I just committed a change to go back to only definining 
> _XCR_XFEATURE_ENABLED_MASK
> when clang is in MSVC compatibility mode. I don’t think I meant to define
> it outside of that since it doesn’t appear gcc does.
>
>
>
> For the other two files, I assume you’re getting a collision of names
> similar to what Benjamin Kramer reported on V8?
>
>
>
> *From:* Jordan Rupprecht [mailto:ruppre...@google.com]
> *Sent:* Friday, January 18, 2019 9:30 AM
> *To:* Topper, Craig 
> *Cc:* cfe-commits@lists.llvm.org
> *Subject:* Re: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on
> non-windows platforms"
>
>
>
> Hi Craig,
>
> We're seeing issues building a few other things now; notably this part in
> opencv:
> https://github.com/opencv/opencv/blob/master/modules/core/src/system.cpp#L422
>
> #ifdef _XCR_XFEATURE_ENABLED_MASK // requires immintrin.h
>
> xcr0 = (int)_xgetbv(_XCR_XFEATURE_ENABLED_MASK);
>
> => error: '__builtin_ia32_xgetbv' needs target feature xsave
>
> We can workaround by building with -mavx... but we also have code running
> on non-avx machines that pull in this dependency. (It likely doesn't
> actually *use* the code, but still depends on it compiling as a transitive
> dep).
>
> Is there a more accurate preprocessor macro (e.g. one that says if xsave
> is available) we could use to selectively disable this?
>
>
>
> There are similar breakages here:
>
>
> https://chromium.googlesource.com/aosp/platform/external/libchrome/+/master/base/cpu.cc#82
>
> https://github.com/google/sling/blob/master/third_party/jit/cpu.cc#L58
>
> Advice there too would be appreciated, if possible :)
>
>
>
> On Wed, Jan 16, 2019 at 3:00 PM Craig Topper via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: ctopper
> Date: Wed Jan 16 14:56:25 2019
> New Revision: 351391
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351391&view=rev
> Log:
> Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"
>
> V8 has been fixed now.
>
> Modified:
> cfe/trunk/include/clang/Basic/BuiltinsX86.def
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Headers/immintrin.h
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/lib/Headers/xsaveintrin.h
> cfe/trunk/test/CodeGen/builtins-x86.c
> cfe/trunk/test/CodeGen/x86_32-xsave.c
> cfe/trunk/test/CodeGen/x86_64-xsave.c
> cfe/trunk/test/Headers/ms-intrin.cpp
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351391&r1=351390&r2=351391&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jan 16 14:56:25 2019
> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>  // XSAVE
>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351391&r1=351390&r2=351391&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jan 16 14:56:25 2019
> @@ -9833,7 +9833,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>case X86::BI__builtin_ia32_xsavec:
>case

RE: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

2019-01-18 Thread Topper, Craig via cfe-commits 
I just committed a change to go back to only definining 
_XCR_XFEATURE_ENABLED_MASK when clang is in MSVC compatibility mode. I don’t 
think I meant to define it outside of that since it doesn’t appear gcc does.

For the other two files, I assume you’re getting a collision of names similar 
to what Benjamin Kramer reported on V8?

From: Jordan Rupprecht [mailto:ruppre...@google.com]
Sent: Friday, January 18, 2019 9:30 AM
To: Topper, Craig 
Cc: cfe-commits@lists.llvm.org
Subject: Re: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on 
non-windows platforms"

Hi Craig,
We're seeing issues building a few other things now; notably this part in 
opencv: 
https://github.com/opencv/opencv/blob/master/modules/core/src/system.cpp#L422
#ifdef _XCR_XFEATURE_ENABLED_MASK // requires immintrin.h
xcr0 = (int)_xgetbv(_XCR_XFEATURE_ENABLED_MASK);
=> error: '__builtin_ia32_xgetbv' needs target feature xsave
We can workaround by building with -mavx... but we also have code running on 
non-avx machines that pull in this dependency. (It likely doesn't actually 
*use* the code, but still depends on it compiling as a transitive dep).
Is there a more accurate preprocessor macro (e.g. one that says if xsave is 
available) we could use to selectively disable this?

There are similar breakages here:
https://chromium.googlesource.com/aosp/platform/external/libchrome/+/master/base/cpu.cc#82
https://github.com/google/sling/blob/master/third_party/jit/cpu.cc#L58
Advice there too would be appreciated, if possible :)

On Wed, Jan 16, 2019 at 3:00 PM Craig Topper via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: ctopper
Date: Wed Jan 16 14:56:25 2019
New Revision: 351391

URL: http://llvm.org/viewvc/llvm-project?rev=351391&view=rev
Log:
Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

V8 has been fixed now.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/immintrin.h
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Headers/xsaveintrin.h
cfe/trunk/test/CodeGen/builtins-x86.c
cfe/trunk/test/CodeGen/x86_32-xsave.c
cfe/trunk/test/CodeGen/x86_64-xsave.c
cfe/trunk/test/Headers/ms-intrin.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351391&r1=351390&r2=351391&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jan 16 14:56:25 2019
@@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
 // XSAVE
 TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
 TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
+TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
+TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h", ALL_MS_LANGUAGES, 
"")
+TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
+TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
 TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
 TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351391&r1=351390&r2=351391&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jan 16 14:56:25 2019
@@ -9833,7 +9833,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
   case X86::BI__builtin_ia32_xsavec:
   case X86::BI__builtin_ia32_xsavec64:
   case X86::BI__builtin_ia32_xsaves:
-  case X86::BI__builtin_ia32_xsaves64: {
+  case X86::BI__builtin_ia32_xsaves64:
+  case X86::BI__builtin_ia32_xsetbv:
+  case X86::BI_xsetbv: {
 Intrinsic::ID ID;
 #define INTRINSIC_X86_XSAVE_ID(NAME) \
 case X86::BI__builtin_ia32_##NAME: \
@@ -9853,6 +9855,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 INTRINSIC_X86_XSAVE_ID(xsavec64);
 INTRINSIC_X86_XSAVE_ID(xsaves);
 INTRINSIC_X86_XSAVE_ID(xsaves64);
+INTRINSIC_X86_XSAVE_ID(xsetbv);
+case X86::BI_xsetbv:
+  ID = Intrinsic::x86_xsetbv;
+  break;
 }
 #undef INTRINSIC_X86_XSAVE_ID
 Value *Mhi = Builder.CreateTrunc(
@@ -9862,6 +9868,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 Ops.push_back(Mlo);
 return Bu

Re: r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

2019-01-18 Thread Jordan Rupprecht via cfe-commits 
Hi Craig,
We're seeing issues building a few other things now; notably this part in
opencv:
https://github.com/opencv/opencv/blob/master/modules/core/src/system.cpp#L422
#ifdef _XCR_XFEATURE_ENABLED_MASK // requires immintrin.h
xcr0 = (int)_xgetbv(_XCR_XFEATURE_ENABLED_MASK);
=> error: '__builtin_ia32_xgetbv' needs target feature xsave
We can workaround by building with -mavx... but we also have code running
on non-avx machines that pull in this dependency. (It likely doesn't
actually *use* the code, but still depends on it compiling as a transitive
dep).
Is there a more accurate preprocessor macro (e.g. one that says if xsave is
available) we could use to selectively disable this?

There are similar breakages here:
https://chromium.googlesource.com/aosp/platform/external/libchrome/+/master/base/cpu.cc#82
https://github.com/google/sling/blob/master/third_party/jit/cpu.cc#L58
Advice there too would be appreciated, if possible :)

On Wed, Jan 16, 2019 at 3:00 PM Craig Topper via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ctopper
> Date: Wed Jan 16 14:56:25 2019
> New Revision: 351391
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351391&view=rev
> Log:
> Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"
>
> V8 has been fixed now.
>
> Modified:
> cfe/trunk/include/clang/Basic/BuiltinsX86.def
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Headers/immintrin.h
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/lib/Headers/xsaveintrin.h
> cfe/trunk/test/CodeGen/builtins-x86.c
> cfe/trunk/test/CodeGen/x86_32-xsave.c
> cfe/trunk/test/CodeGen/x86_64-xsave.c
> cfe/trunk/test/Headers/ms-intrin.cpp
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351391&r1=351390&r2=351391&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jan 16 14:56:25 2019
> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>  // XSAVE
>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351391&r1=351390&r2=351391&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jan 16 14:56:25 2019
> @@ -9833,7 +9833,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>case X86::BI__builtin_ia32_xsavec:
>case X86::BI__builtin_ia32_xsavec64:
>case X86::BI__builtin_ia32_xsaves:
> -  case X86::BI__builtin_ia32_xsaves64: {
> +  case X86::BI__builtin_ia32_xsaves64:
> +  case X86::BI__builtin_ia32_xsetbv:
> +  case X86::BI_xsetbv: {
>  Intrinsic::ID ID;
>  #define INTRINSIC_X86_XSAVE_ID(NAME) \
>  case X86::BI__builtin_ia32_##NAME: \
> @@ -9853,6 +9855,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>  INTRINSIC_X86_XSAVE_ID(xsavec64);
>  INTRINSIC_X86_XSAVE_ID(xsaves);
>  INTRINSIC_X86_XSAVE_ID(xsaves64);
> +INTRINSIC_X86_XSAVE_ID(xsetbv);
> +case X86::BI_xsetbv:
> +  ID = Intrinsic::x86_xsetbv;
> +  break;
>  }
>  #undef INTRINSIC_X86_XSAVE_ID
>  Value *Mhi = Builder.CreateTrunc(
> @@ -9862,6 +9868,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>  Ops.push_back(Mlo);
>  return Builder.CreateCall(CGM.getIntrinsic(ID), Ops);
>}
> +  case X86::BI__builtin_ia32_xgetbv:
> +  case X86::BI_xgetbv:
> +return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_xgetbv),
> Ops);
>case X86:

r351391 - Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

2019-01-16 Thread Craig Topper via cfe-commits 
Author: ctopper
Date: Wed Jan 16 14:56:25 2019
New Revision: 351391

URL: http://llvm.org/viewvc/llvm-project?rev=351391&view=rev
Log:
Recommit r351160 "[X86] Make _xgetbv/_xsetbv on non-windows platforms"

V8 has been fixed now.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/immintrin.h
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Headers/xsaveintrin.h
cfe/trunk/test/CodeGen/builtins-x86.c
cfe/trunk/test/CodeGen/x86_32-xsave.c
cfe/trunk/test/CodeGen/x86_64-xsave.c
cfe/trunk/test/Headers/ms-intrin.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351391&r1=351390&r2=351391&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jan 16 14:56:25 2019
@@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
 // XSAVE
 TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
 TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
+TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
+TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h", ALL_MS_LANGUAGES, 
"")
+TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
+TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
 TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
 TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351391&r1=351390&r2=351391&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jan 16 14:56:25 2019
@@ -9833,7 +9833,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
   case X86::BI__builtin_ia32_xsavec:
   case X86::BI__builtin_ia32_xsavec64:
   case X86::BI__builtin_ia32_xsaves:
-  case X86::BI__builtin_ia32_xsaves64: {
+  case X86::BI__builtin_ia32_xsaves64:
+  case X86::BI__builtin_ia32_xsetbv:
+  case X86::BI_xsetbv: {
 Intrinsic::ID ID;
 #define INTRINSIC_X86_XSAVE_ID(NAME) \
 case X86::BI__builtin_ia32_##NAME: \
@@ -9853,6 +9855,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 INTRINSIC_X86_XSAVE_ID(xsavec64);
 INTRINSIC_X86_XSAVE_ID(xsaves);
 INTRINSIC_X86_XSAVE_ID(xsaves64);
+INTRINSIC_X86_XSAVE_ID(xsetbv);
+case X86::BI_xsetbv:
+  ID = Intrinsic::x86_xsetbv;
+  break;
 }
 #undef INTRINSIC_X86_XSAVE_ID
 Value *Mhi = Builder.CreateTrunc(
@@ -9862,6 +9868,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 Ops.push_back(Mlo);
 return Builder.CreateCall(CGM.getIntrinsic(ID), Ops);
   }
+  case X86::BI__builtin_ia32_xgetbv:
+  case X86::BI_xgetbv:
+return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_xgetbv), Ops);
   case X86::BI__builtin_ia32_storedqudi128_mask:
   case X86::BI__builtin_ia32_storedqusi128_mask:
   case X86::BI__builtin_ia32_storedquhi128_mask:

Modified: cfe/trunk/lib/Headers/immintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=351391&r1=351390&r2=351391&view=diff
==
--- cfe/trunk/lib/Headers/immintrin.h (original)
+++ cfe/trunk/lib/Headers/immintrin.h Wed Jan 16 14:56:25 2019
@@ -378,9 +378,8 @@ _storebe_i64(void * __P, long long __D)
 #include 
 #endif
 
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__XSAVE__)
+/* No feature check desired due to internal MSC_VER checks */
 #include 
-#endif
 
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__XSAVEOPT__)
 #include 

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=351391&r1=351390&r2=351391&view=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Wed Jan 16 14:56:25 2019
@@ -200,10 +200,7 @@ __attribute__((__deprecated__("use other
 _WriteBarrier(void);
 unsigned __int32 xbegin(void);
 void _xend(void);
-static __inline__
 #define _XCR_XFEATURE_ENABLED_MASK 0
-unsigned __int64 __cdecl _xgetbv(unsigned int);
-void __cdecl _xsetbv(unsigned int, unsigned __int64);
 
 /* Thes

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-16 Thread Benjamin Kramer via cfe-commits 
v8 is fixed, feel free to land this after the release branch :)

On Wed, Jan 16, 2019 at 12:57 AM Craig Topper 
wrote:

> This isn't blocking anything. Just doing some archaeology because I
> noticed we had an intrinsic in the backend, but it wasn't used by the
> frontend due to a previous revert a couple years ago.
>
> ~Craig
>
>
> On Tue, Jan 15, 2019 at 3:47 PM Benjamin Kramer 
> wrote:
>
>> I think the only viable solution is to make v8 not define reserved
>> identifiers & reland this change. That will take some time, so unless this
>> is blocking something important I'd prefer to reland after the release cut
>> so the world can catch up before the next release. I can take care of
>> sending v8 a patch if nobody else feels like doing it.
>>
>> On Tue, Jan 15, 2019 at 11:28 PM Craig Topper 
>> wrote:
>>
>>> any suggestions on how to proceed here?
>>>
>>> ~Craig
>>>
>>>
>>> On Tue, Jan 15, 2019 at 10:56 AM Benjamin Kramer 
>>> wrote:
>>>
 I think the main issue is that libstdc++ 4.9 includes x86intrin.h
 transitively from . That's probably broken with all compilers :(

 On Tue, Jan 15, 2019 at 7:31 PM Craig Topper 
 wrote:

> Does V8 work with gcc which also has _xgetbv? Or is it because I had
> to make _xgetbv a macro to make the patch work?
>
> ~Craig
>
>
> On Tue, Jan 15, 2019 at 9:28 AM Benjamin Kramer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I rolled it back for now in r351210, this pattern seems to be quite
>> common even outside of v8. Let's figure out if we can keep the code 
>> working
>> or if it needs to be fixed all over the place :(
>>
>> On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer 
>> wrote:
>>
>>> I'm seeing breakages on v8 with this, it defines its own _xgetbv.
>>> Any ideas what do do about this?
>>>
>>>
>>> https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36
>>>
>>> src/x64/assembler-x64.cc:35:1: error: inline variables are
>>> incompatible with C++ standards before C++17
>>> [-Werror,-Wc++98-c++11-c++14-compat]
>>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>> ^
>>> include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
>>> # define V8_INLINE inline __attribute__((always_inline))
>>>^
>>> src/x64/assembler-x64.cc:35:41: error: expected ')'
>>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>> ^
>>> src/x64/assembler-x64.cc:35:20: note: to match this '('
>>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>>^
>>> lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro
>>> '_xgetbv'
>>> #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
>>>
>>> On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: ctopper
 Date: Mon Jan 14 21:03:18 2019
 New Revision: 351160

 URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
 Log:
 [X86] Make _xgetbv/_xsetbv on non-windows platforms

 Summary:
 This patch attempts to redo what was tried in r278783, but was
 reverted.

 These intrinsics should be available on non-windows platforms with
 "xsave" feature check. But on Windows platforms they shouldn't have 
 feature
 check since that's how MSVC behaves.

 To accomplish this I've added a MS builtin with no feature check.
 And a normal gcc builtin with a feature check. When _MSC_VER is not 
 defined
 _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.

 I've moved the forward declarations from intrin.h to immintrin.h to
 match the MSDN documentation and used that as the header file for the 
 MS
 builtin.

 I'm not super happy with this implementation, and I'm open to
 suggestions for better ways to do it.

 Reviewers: rnk, RKSimon, spatel

 Reviewed By: rnk

 Subscribers: cfe-commits

 Differential Revision: https://reviews.llvm.org/D56686

 Modified:
 cfe/trunk/include/clang/Basic/BuiltinsX86.def
 cfe/trunk/lib/CodeGen/CGBuiltin.cpp
 cfe/trunk/lib/Headers/immintrin.h
 cfe/trunk/lib/Headers/intrin.h
 cfe/trunk/lib/Headers/xsaveintrin.h
 cfe/trunk/test/CodeGen/builtins-x86.c
 cfe/trunk/test/CodeGen/x86_32-xsave.c
 cfe/trunk/test/CodeGen/x86_64-xsave.c
 cfe/trunk/test/Headers/ms-intrin.cpp

 Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX8

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Craig Topper via cfe-commits 
This isn't blocking anything. Just doing some archaeology because I noticed
we had an intrinsic in the backend, but it wasn't used by the frontend due
to a previous revert a couple years ago.

~Craig


On Tue, Jan 15, 2019 at 3:47 PM Benjamin Kramer  wrote:

> I think the only viable solution is to make v8 not define reserved
> identifiers & reland this change. That will take some time, so unless this
> is blocking something important I'd prefer to reland after the release cut
> so the world can catch up before the next release. I can take care of
> sending v8 a patch if nobody else feels like doing it.
>
> On Tue, Jan 15, 2019 at 11:28 PM Craig Topper 
> wrote:
>
>> any suggestions on how to proceed here?
>>
>> ~Craig
>>
>>
>> On Tue, Jan 15, 2019 at 10:56 AM Benjamin Kramer 
>> wrote:
>>
>>> I think the main issue is that libstdc++ 4.9 includes x86intrin.h
>>> transitively from . That's probably broken with all compilers :(
>>>
>>> On Tue, Jan 15, 2019 at 7:31 PM Craig Topper 
>>> wrote:
>>>
 Does V8 work with gcc which also has _xgetbv? Or is it because I had to
 make _xgetbv a macro to make the patch work?

 ~Craig


 On Tue, Jan 15, 2019 at 9:28 AM Benjamin Kramer via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> I rolled it back for now in r351210, this pattern seems to be quite
> common even outside of v8. Let's figure out if we can keep the code 
> working
> or if it needs to be fixed all over the place :(
>
> On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer 
> wrote:
>
>> I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any
>> ideas what do do about this?
>>
>>
>> https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36
>>
>> src/x64/assembler-x64.cc:35:1: error: inline variables are
>> incompatible with C++ standards before C++17
>> [-Werror,-Wc++98-c++11-c++14-compat]
>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>> ^
>> include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
>> # define V8_INLINE inline __attribute__((always_inline))
>>^
>> src/x64/assembler-x64.cc:35:41: error: expected ')'
>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>> ^
>> src/x64/assembler-x64.cc:35:20: note: to match this '('
>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>^
>> lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro
>> '_xgetbv'
>> #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
>>
>> On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: ctopper
>>> Date: Mon Jan 14 21:03:18 2019
>>> New Revision: 351160
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
>>> Log:
>>> [X86] Make _xgetbv/_xsetbv on non-windows platforms
>>>
>>> Summary:
>>> This patch attempts to redo what was tried in r278783, but was
>>> reverted.
>>>
>>> These intrinsics should be available on non-windows platforms with
>>> "xsave" feature check. But on Windows platforms they shouldn't have 
>>> feature
>>> check since that's how MSVC behaves.
>>>
>>> To accomplish this I've added a MS builtin with no feature check.
>>> And a normal gcc builtin with a feature check. When _MSC_VER is not 
>>> defined
>>> _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.
>>>
>>> I've moved the forward declarations from intrin.h to immintrin.h to
>>> match the MSDN documentation and used that as the header file for the MS
>>> builtin.
>>>
>>> I'm not super happy with this implementation, and I'm open to
>>> suggestions for better ways to do it.
>>>
>>> Reviewers: rnk, RKSimon, spatel
>>>
>>> Reviewed By: rnk
>>>
>>> Subscribers: cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D56686
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Basic/BuiltinsX86.def
>>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>>> cfe/trunk/lib/Headers/immintrin.h
>>> cfe/trunk/lib/Headers/intrin.h
>>> cfe/trunk/lib/Headers/xsaveintrin.h
>>> cfe/trunk/test/CodeGen/builtins-x86.c
>>> cfe/trunk/test/CodeGen/x86_32-xsave.c
>>> cfe/trunk/test/CodeGen/x86_64-xsave.c
>>> cfe/trunk/test/Headers/ms-intrin.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
>>> +++ cfe/trunk/include/clang/Basic/BuiltinsX86

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Benjamin Kramer via cfe-commits 
I think the only viable solution is to make v8 not define reserved
identifiers & reland this change. That will take some time, so unless this
is blocking something important I'd prefer to reland after the release cut
so the world can catch up before the next release. I can take care of
sending v8 a patch if nobody else feels like doing it.

On Tue, Jan 15, 2019 at 11:28 PM Craig Topper 
wrote:

> any suggestions on how to proceed here?
>
> ~Craig
>
>
> On Tue, Jan 15, 2019 at 10:56 AM Benjamin Kramer 
> wrote:
>
>> I think the main issue is that libstdc++ 4.9 includes x86intrin.h
>> transitively from . That's probably broken with all compilers :(
>>
>> On Tue, Jan 15, 2019 at 7:31 PM Craig Topper 
>> wrote:
>>
>>> Does V8 work with gcc which also has _xgetbv? Or is it because I had to
>>> make _xgetbv a macro to make the patch work?
>>>
>>> ~Craig
>>>
>>>
>>> On Tue, Jan 15, 2019 at 9:28 AM Benjamin Kramer via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 I rolled it back for now in r351210, this pattern seems to be quite
 common even outside of v8. Let's figure out if we can keep the code working
 or if it needs to be fixed all over the place :(

 On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer 
 wrote:

> I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any
> ideas what do do about this?
>
>
> https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36
>
> src/x64/assembler-x64.cc:35:1: error: inline variables are
> incompatible with C++ standards before C++17
> [-Werror,-Wc++98-c++11-c++14-compat]
> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
> ^
> include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
> # define V8_INLINE inline __attribute__((always_inline))
>^
> src/x64/assembler-x64.cc:35:41: error: expected ')'
> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
> ^
> src/x64/assembler-x64.cc:35:20: note: to match this '('
> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>^
> lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro
> '_xgetbv'
> #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
>
> On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ctopper
>> Date: Mon Jan 14 21:03:18 2019
>> New Revision: 351160
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
>> Log:
>> [X86] Make _xgetbv/_xsetbv on non-windows platforms
>>
>> Summary:
>> This patch attempts to redo what was tried in r278783, but was
>> reverted.
>>
>> These intrinsics should be available on non-windows platforms with
>> "xsave" feature check. But on Windows platforms they shouldn't have 
>> feature
>> check since that's how MSVC behaves.
>>
>> To accomplish this I've added a MS builtin with no feature check. And
>> a normal gcc builtin with a feature check. When _MSC_VER is not defined
>> _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.
>>
>> I've moved the forward declarations from intrin.h to immintrin.h to
>> match the MSDN documentation and used that as the header file for the MS
>> builtin.
>>
>> I'm not super happy with this implementation, and I'm open to
>> suggestions for better ways to do it.
>>
>> Reviewers: rnk, RKSimon, spatel
>>
>> Reviewed By: rnk
>>
>> Subscribers: cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D56686
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/BuiltinsX86.def
>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> cfe/trunk/lib/Headers/immintrin.h
>> cfe/trunk/lib/Headers/intrin.h
>> cfe/trunk/lib/Headers/xsaveintrin.h
>> cfe/trunk/test/CodeGen/builtins-x86.c
>> cfe/trunk/test/CodeGen/x86_32-xsave.c
>> cfe/trunk/test/CodeGen/x86_64-xsave.c
>> cfe/trunk/test/Headers/ms-intrin.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
>> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18
>> 2019
>> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>>  // XSAVE
>>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
>> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
>> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Craig Topper via cfe-commits 
any suggestions on how to proceed here?

~Craig


On Tue, Jan 15, 2019 at 10:56 AM Benjamin Kramer 
wrote:

> I think the main issue is that libstdc++ 4.9 includes x86intrin.h
> transitively from . That's probably broken with all compilers :(
>
> On Tue, Jan 15, 2019 at 7:31 PM Craig Topper 
> wrote:
>
>> Does V8 work with gcc which also has _xgetbv? Or is it because I had to
>> make _xgetbv a macro to make the patch work?
>>
>> ~Craig
>>
>>
>> On Tue, Jan 15, 2019 at 9:28 AM Benjamin Kramer via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> I rolled it back for now in r351210, this pattern seems to be quite
>>> common even outside of v8. Let's figure out if we can keep the code working
>>> or if it needs to be fixed all over the place :(
>>>
>>> On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer 
>>> wrote:
>>>
 I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any
 ideas what do do about this?


 https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36

 src/x64/assembler-x64.cc:35:1: error: inline variables are incompatible
 with C++ standards before C++17 [-Werror,-Wc++98-c++11-c++14-compat]
 V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
 ^
 include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
 # define V8_INLINE inline __attribute__((always_inline))
^
 src/x64/assembler-x64.cc:35:41: error: expected ')'
 V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
 ^
 src/x64/assembler-x64.cc:35:20: note: to match this '('
 V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
^
 lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro
 '_xgetbv'
 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))

 On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: ctopper
> Date: Mon Jan 14 21:03:18 2019
> New Revision: 351160
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
> Log:
> [X86] Make _xgetbv/_xsetbv on non-windows platforms
>
> Summary:
> This patch attempts to redo what was tried in r278783, but was
> reverted.
>
> These intrinsics should be available on non-windows platforms with
> "xsave" feature check. But on Windows platforms they shouldn't have 
> feature
> check since that's how MSVC behaves.
>
> To accomplish this I've added a MS builtin with no feature check. And
> a normal gcc builtin with a feature check. When _MSC_VER is not defined
> _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.
>
> I've moved the forward declarations from intrin.h to immintrin.h to
> match the MSDN documentation and used that as the header file for the MS
> builtin.
>
> I'm not super happy with this implementation, and I'm open to
> suggestions for better ways to do it.
>
> Reviewers: rnk, RKSimon, spatel
>
> Reviewed By: rnk
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D56686
>
> Modified:
> cfe/trunk/include/clang/Basic/BuiltinsX86.def
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Headers/immintrin.h
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/lib/Headers/xsaveintrin.h
> cfe/trunk/test/CodeGen/builtins-x86.c
> cfe/trunk/test/CodeGen/x86_32-xsave.c
> cfe/trunk/test/CodeGen/x86_64-xsave.c
> cfe/trunk/test/Headers/ms-intrin.cpp
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18
> 2019
> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>  // XSAVE
>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Benjamin Kramer via cfe-commits 
I think the main issue is that libstdc++ 4.9 includes x86intrin.h
transitively from . That's probably broken with all compilers :(

On Tue, Jan 15, 2019 at 7:31 PM Craig Topper  wrote:

> Does V8 work with gcc which also has _xgetbv? Or is it because I had to
> make _xgetbv a macro to make the patch work?
>
> ~Craig
>
>
> On Tue, Jan 15, 2019 at 9:28 AM Benjamin Kramer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I rolled it back for now in r351210, this pattern seems to be quite
>> common even outside of v8. Let's figure out if we can keep the code working
>> or if it needs to be fixed all over the place :(
>>
>> On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer 
>> wrote:
>>
>>> I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any
>>> ideas what do do about this?
>>>
>>>
>>> https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36
>>>
>>> src/x64/assembler-x64.cc:35:1: error: inline variables are incompatible
>>> with C++ standards before C++17 [-Werror,-Wc++98-c++11-c++14-compat]
>>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>> ^
>>> include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
>>> # define V8_INLINE inline __attribute__((always_inline))
>>>^
>>> src/x64/assembler-x64.cc:35:41: error: expected ')'
>>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>> ^
>>> src/x64/assembler-x64.cc:35:20: note: to match this '('
>>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>>^
>>> lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro
>>> '_xgetbv'
>>> #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
>>>
>>> On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: ctopper
 Date: Mon Jan 14 21:03:18 2019
 New Revision: 351160

 URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
 Log:
 [X86] Make _xgetbv/_xsetbv on non-windows platforms

 Summary:
 This patch attempts to redo what was tried in r278783, but was reverted.

 These intrinsics should be available on non-windows platforms with
 "xsave" feature check. But on Windows platforms they shouldn't have feature
 check since that's how MSVC behaves.

 To accomplish this I've added a MS builtin with no feature check. And a
 normal gcc builtin with a feature check. When _MSC_VER is not defined
 _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.

 I've moved the forward declarations from intrin.h to immintrin.h to
 match the MSDN documentation and used that as the header file for the MS
 builtin.

 I'm not super happy with this implementation, and I'm open to
 suggestions for better ways to do it.

 Reviewers: rnk, RKSimon, spatel

 Reviewed By: rnk

 Subscribers: cfe-commits

 Differential Revision: https://reviews.llvm.org/D56686

 Modified:
 cfe/trunk/include/clang/Basic/BuiltinsX86.def
 cfe/trunk/lib/CodeGen/CGBuiltin.cpp
 cfe/trunk/lib/Headers/immintrin.h
 cfe/trunk/lib/Headers/intrin.h
 cfe/trunk/lib/Headers/xsaveintrin.h
 cfe/trunk/test/CodeGen/builtins-x86.c
 cfe/trunk/test/CodeGen/x86_32-xsave.c
 cfe/trunk/test/CodeGen/x86_64-xsave.c
 cfe/trunk/test/Headers/ms-intrin.cpp

 Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff

 ==
 --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
 +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18
 2019
 @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
  // XSAVE
  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
 +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
 +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
 ALL_MS_LANGUAGES, "")
 +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
 +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
 ALL_MS_LANGUAGES, "")
  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")

 Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351160&r1=351159&r2=351160&view=diff

 ==
 --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
 +++ cfe/trunk/li

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Craig Topper via cfe-commits 
Does V8 work with gcc which also has _xgetbv? Or is it because I had to
make _xgetbv a macro to make the patch work?

~Craig


On Tue, Jan 15, 2019 at 9:28 AM Benjamin Kramer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I rolled it back for now in r351210, this pattern seems to be quite common
> even outside of v8. Let's figure out if we can keep the code working or if
> it needs to be fixed all over the place :(
>
> On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer 
> wrote:
>
>> I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any
>> ideas what do do about this?
>>
>>
>> https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36
>>
>> src/x64/assembler-x64.cc:35:1: error: inline variables are incompatible
>> with C++ standards before C++17 [-Werror,-Wc++98-c++11-c++14-compat]
>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>> ^
>> include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
>> # define V8_INLINE inline __attribute__((always_inline))
>>^
>> src/x64/assembler-x64.cc:35:41: error: expected ')'
>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>> ^
>> src/x64/assembler-x64.cc:35:20: note: to match this '('
>> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>>^
>> lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro '_xgetbv'
>> #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
>>
>> On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: ctopper
>>> Date: Mon Jan 14 21:03:18 2019
>>> New Revision: 351160
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
>>> Log:
>>> [X86] Make _xgetbv/_xsetbv on non-windows platforms
>>>
>>> Summary:
>>> This patch attempts to redo what was tried in r278783, but was reverted.
>>>
>>> These intrinsics should be available on non-windows platforms with
>>> "xsave" feature check. But on Windows platforms they shouldn't have feature
>>> check since that's how MSVC behaves.
>>>
>>> To accomplish this I've added a MS builtin with no feature check. And a
>>> normal gcc builtin with a feature check. When _MSC_VER is not defined
>>> _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.
>>>
>>> I've moved the forward declarations from intrin.h to immintrin.h to
>>> match the MSDN documentation and used that as the header file for the MS
>>> builtin.
>>>
>>> I'm not super happy with this implementation, and I'm open to
>>> suggestions for better ways to do it.
>>>
>>> Reviewers: rnk, RKSimon, spatel
>>>
>>> Reviewed By: rnk
>>>
>>> Subscribers: cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D56686
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Basic/BuiltinsX86.def
>>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>>> cfe/trunk/lib/Headers/immintrin.h
>>> cfe/trunk/lib/Headers/intrin.h
>>> cfe/trunk/lib/Headers/xsaveintrin.h
>>> cfe/trunk/test/CodeGen/builtins-x86.c
>>> cfe/trunk/test/CodeGen/x86_32-xsave.c
>>> cfe/trunk/test/CodeGen/x86_64-xsave.c
>>> cfe/trunk/test/Headers/ms-intrin.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
>>> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18
>>> 2019
>>> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>>>  // XSAVE
>>>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>>>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
>>> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
>>> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
>>> ALL_MS_LANGUAGES, "")
>>> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
>>> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
>>> ALL_MS_LANGUAGES, "")
>>>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>>>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>>>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351160&r1=351159&r2=351160&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jan 14 21:03:18 2019
>>> @@ -9831,7 +9831,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>>>case X86::BI__builtin_ia32_xsavec:
>>>case X86::BI__builtin_ia32_xsavec64:
>>>case X86::BI__builtin_ia32_xsaves:
>>> -  case X86::BI__builtin_ia32_xsaves64: {
>>> +  case X86::BI_

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Benjamin Kramer via cfe-commits 
I rolled it back for now in r351210, this pattern seems to be quite common
even outside of v8. Let's figure out if we can keep the code working or if
it needs to be fixed all over the place :(

On Tue, Jan 15, 2019 at 3:02 PM Benjamin Kramer  wrote:

> I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any
> ideas what do do about this?
>
>
> https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36
>
> src/x64/assembler-x64.cc:35:1: error: inline variables are incompatible
> with C++ standards before C++17 [-Werror,-Wc++98-c++11-c++14-compat]
> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
> ^
> include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
> # define V8_INLINE inline __attribute__((always_inline))
>^
> src/x64/assembler-x64.cc:35:41: error: expected ')'
> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
> ^
> src/x64/assembler-x64.cc:35:20: note: to match this '('
> V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
>^
> lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro '_xgetbv'
> #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
>
> On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ctopper
>> Date: Mon Jan 14 21:03:18 2019
>> New Revision: 351160
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
>> Log:
>> [X86] Make _xgetbv/_xsetbv on non-windows platforms
>>
>> Summary:
>> This patch attempts to redo what was tried in r278783, but was reverted.
>>
>> These intrinsics should be available on non-windows platforms with
>> "xsave" feature check. But on Windows platforms they shouldn't have feature
>> check since that's how MSVC behaves.
>>
>> To accomplish this I've added a MS builtin with no feature check. And a
>> normal gcc builtin with a feature check. When _MSC_VER is not defined
>> _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.
>>
>> I've moved the forward declarations from intrin.h to immintrin.h to match
>> the MSDN documentation and used that as the header file for the MS builtin.
>>
>> I'm not super happy with this implementation, and I'm open to suggestions
>> for better ways to do it.
>>
>> Reviewers: rnk, RKSimon, spatel
>>
>> Reviewed By: rnk
>>
>> Subscribers: cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D56686
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/BuiltinsX86.def
>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> cfe/trunk/lib/Headers/immintrin.h
>> cfe/trunk/lib/Headers/intrin.h
>> cfe/trunk/lib/Headers/xsaveintrin.h
>> cfe/trunk/test/CodeGen/builtins-x86.c
>> cfe/trunk/test/CodeGen/x86_32-xsave.c
>> cfe/trunk/test/CodeGen/x86_64-xsave.c
>> cfe/trunk/test/Headers/ms-intrin.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
>> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18 2019
>> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>>  // XSAVE
>>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
>> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
>> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
>> ALL_MS_LANGUAGES, "")
>> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
>> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
>> ALL_MS_LANGUAGES, "")
>>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>>
>> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351160&r1=351159&r2=351160&view=diff
>>
>> ==
>> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jan 14 21:03:18 2019
>> @@ -9831,7 +9831,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>>case X86::BI__builtin_ia32_xsavec:
>>case X86::BI__builtin_ia32_xsavec64:
>>case X86::BI__builtin_ia32_xsaves:
>> -  case X86::BI__builtin_ia32_xsaves64: {
>> +  case X86::BI__builtin_ia32_xsaves64:
>> +  case X86::BI__builtin_ia32_xsetbv:
>> +  case X86::BI_xsetbv: {
>>  Intrinsic::ID ID;
>>  #define INTRINSIC_X86_XSAVE_ID(NAME) \
>>  case X86::BI__builtin_ia32_##NAME: \
>> @@ -9851,6 +9853,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>>  INTRINSIC_X86_XSAVE_ID(xsavec64);
>>  INTRINSIC_X86_XSAVE_ID(xsav

Re: r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-15 Thread Benjamin Kramer via cfe-commits 
I'm seeing breakages on v8 with this, it defines its own _xgetbv. Any ideas
what do do about this?

https://chromium.googlesource.com/v8/v8.git/+/master/src/x64/assembler-x64.cc#36

src/x64/assembler-x64.cc:35:1: error: inline variables are incompatible
with C++ standards before C++17 [-Werror,-Wc++98-c++11-c++14-compat]
V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
^
include/v8config.h:294:20: note: expanded from macro 'V8_INLINE'
# define V8_INLINE inline __attribute__((always_inline))
   ^
src/x64/assembler-x64.cc:35:41: error: expected ')'
V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
^
src/x64/assembler-x64.cc:35:20: note: to match this '('
V8_INLINE uint64_t _xgetbv(unsigned int xcr) {
   ^
lib/clang/include/xsaveintrin.h:49:53: note: expanded from macro '_xgetbv'
#define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))

On Tue, Jan 15, 2019 at 6:06 AM Craig Topper via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ctopper
> Date: Mon Jan 14 21:03:18 2019
> New Revision: 351160
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
> Log:
> [X86] Make _xgetbv/_xsetbv on non-windows platforms
>
> Summary:
> This patch attempts to redo what was tried in r278783, but was reverted.
>
> These intrinsics should be available on non-windows platforms with "xsave"
> feature check. But on Windows platforms they shouldn't have feature check
> since that's how MSVC behaves.
>
> To accomplish this I've added a MS builtin with no feature check. And a
> normal gcc builtin with a feature check. When _MSC_VER is not defined
> _xgetbv/_xsetbv will be macros pointing to the gcc builtin name.
>
> I've moved the forward declarations from intrin.h to immintrin.h to match
> the MSDN documentation and used that as the header file for the MS builtin.
>
> I'm not super happy with this implementation, and I'm open to suggestions
> for better ways to do it.
>
> Reviewers: rnk, RKSimon, spatel
>
> Reviewed By: rnk
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D56686
>
> Modified:
> cfe/trunk/include/clang/Basic/BuiltinsX86.def
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Headers/immintrin.h
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/lib/Headers/xsaveintrin.h
> cfe/trunk/test/CodeGen/builtins-x86.c
> cfe/trunk/test/CodeGen/x86_32-xsave.c
> cfe/trunk/test/CodeGen/x86_64-xsave.c
> cfe/trunk/test/Headers/ms-intrin.cpp
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18 2019
> @@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
>  // XSAVE
>  TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
>  TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
> +TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
> +TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
> +TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h",
> ALL_MS_LANGUAGES, "")
>  TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
>  TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
>  TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351160&r1=351159&r2=351160&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jan 14 21:03:18 2019
> @@ -9831,7 +9831,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>case X86::BI__builtin_ia32_xsavec:
>case X86::BI__builtin_ia32_xsavec64:
>case X86::BI__builtin_ia32_xsaves:
> -  case X86::BI__builtin_ia32_xsaves64: {
> +  case X86::BI__builtin_ia32_xsaves64:
> +  case X86::BI__builtin_ia32_xsetbv:
> +  case X86::BI_xsetbv: {
>  Intrinsic::ID ID;
>  #define INTRINSIC_X86_XSAVE_ID(NAME) \
>  case X86::BI__builtin_ia32_##NAME: \
> @@ -9851,6 +9853,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>  INTRINSIC_X86_XSAVE_ID(xsavec64);
>  INTRINSIC_X86_XSAVE_ID(xsaves);
>  INTRINSIC_X86_XSAVE_ID(xsaves64);
> +INTRINSIC_X86_XSAVE_ID(xsetbv);
> +case X86::BI_xsetbv:
> +  ID = Intrinsic::x86_xsetbv;
> +  break;
>  }
>  #undef INTRINSIC_X86_XSAVE_ID
>  Value *Mhi = Builder.CreateTrunc(
> @@ -9860,6 +9866,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>  Ops.push_back(Mlo);
>  return Builder.CreateCall(CGM.getIn

r351160 - [X86] Make _xgetbv/_xsetbv on non-windows platforms

2019-01-14 Thread Craig Topper via cfe-commits 
Author: ctopper
Date: Mon Jan 14 21:03:18 2019
New Revision: 351160

URL: http://llvm.org/viewvc/llvm-project?rev=351160&view=rev
Log:
[X86] Make _xgetbv/_xsetbv on non-windows platforms

Summary:
This patch attempts to redo what was tried in r278783, but was reverted.

These intrinsics should be available on non-windows platforms with "xsave" 
feature check. But on Windows platforms they shouldn't have feature check since 
that's how MSVC behaves.

To accomplish this I've added a MS builtin with no feature check. And a normal 
gcc builtin with a feature check. When _MSC_VER is not defined _xgetbv/_xsetbv 
will be macros pointing to the gcc builtin name.

I've moved the forward declarations from intrin.h to immintrin.h to match the 
MSDN documentation and used that as the header file for the MS builtin.

I'm not super happy with this implementation, and I'm open to suggestions for 
better ways to do it.

Reviewers: rnk, RKSimon, spatel

Reviewed By: rnk

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D56686

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/immintrin.h
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Headers/xsaveintrin.h
cfe/trunk/test/CodeGen/builtins-x86.c
cfe/trunk/test/CodeGen/x86_32-xsave.c
cfe/trunk/test/CodeGen/x86_64-xsave.c
cfe/trunk/test/Headers/ms-intrin.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=351160&r1=351159&r2=351160&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan 14 21:03:18 2019
@@ -693,6 +693,10 @@ TARGET_BUILTIN(__builtin_ia32_fxsave, "v
 // XSAVE
 TARGET_BUILTIN(__builtin_ia32_xsave, "vv*ULLi", "n", "xsave")
 TARGET_BUILTIN(__builtin_ia32_xrstor, "vv*ULLi", "n", "xsave")
+TARGET_BUILTIN(__builtin_ia32_xgetbv, "ULLiUi", "n", "xsave")
+TARGET_HEADER_BUILTIN(_xgetbv, "UWiUi", "nh", "immintrin.h", ALL_MS_LANGUAGES, 
"")
+TARGET_BUILTIN(__builtin_ia32_xsetbv, "vUiULLi", "n", "xsave")
+TARGET_HEADER_BUILTIN(_xsetbv, "vUiUWi", "nh", "immintrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_xsaveopt, "vv*ULLi", "n", "xsaveopt")
 TARGET_BUILTIN(__builtin_ia32_xrstors, "vv*ULLi", "n", "xsaves")
 TARGET_BUILTIN(__builtin_ia32_xsavec, "vv*ULLi", "n", "xsavec")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351160&r1=351159&r2=351160&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jan 14 21:03:18 2019
@@ -9831,7 +9831,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
   case X86::BI__builtin_ia32_xsavec:
   case X86::BI__builtin_ia32_xsavec64:
   case X86::BI__builtin_ia32_xsaves:
-  case X86::BI__builtin_ia32_xsaves64: {
+  case X86::BI__builtin_ia32_xsaves64:
+  case X86::BI__builtin_ia32_xsetbv:
+  case X86::BI_xsetbv: {
 Intrinsic::ID ID;
 #define INTRINSIC_X86_XSAVE_ID(NAME) \
 case X86::BI__builtin_ia32_##NAME: \
@@ -9851,6 +9853,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 INTRINSIC_X86_XSAVE_ID(xsavec64);
 INTRINSIC_X86_XSAVE_ID(xsaves);
 INTRINSIC_X86_XSAVE_ID(xsaves64);
+INTRINSIC_X86_XSAVE_ID(xsetbv);
+case X86::BI_xsetbv:
+  ID = Intrinsic::x86_xsetbv;
+  break;
 }
 #undef INTRINSIC_X86_XSAVE_ID
 Value *Mhi = Builder.CreateTrunc(
@@ -9860,6 +9866,9 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 Ops.push_back(Mlo);
 return Builder.CreateCall(CGM.getIntrinsic(ID), Ops);
   }
+  case X86::BI__builtin_ia32_xgetbv:
+  case X86::BI_xgetbv:
+return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_xgetbv), Ops);
   case X86::BI__builtin_ia32_storedqudi128_mask:
   case X86::BI__builtin_ia32_storedqusi128_mask:
   case X86::BI__builtin_ia32_storedquhi128_mask:

Modified: cfe/trunk/lib/Headers/immintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=351160&r1=351159&r2=351160&view=diff
==
--- cfe/trunk/lib/Headers/immintrin.h (original)
+++ cfe/trunk/lib/Headers/immintrin.h Mon Jan 14 21:03:18 2019
@@ -378,9 +378,8 @@ _storebe_i64(void * __P, long long __D)
 #include 
 #endif
 
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__XSAVE__)
+/* No feature check desired due to internal MSC_VER checks */
 #include 
-#endif
 
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__XSAVEOPT__)
 #include 

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=351160&r1=351159&r2=351160&view=diff
=