[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-07 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a subscriber: eandrews.
aaron.ballman added a comment.

Thank you for the revert -- I've confirmed it fixed the issues for our 
downstream. Also, @eandrews said she may try to tackle the underlying issue 
with builtins in the relatively near future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-04 Thread Aiden Grossman via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3baf63d9a1b: Revert [clang][X86] Add __cpuidex 
function to cpuid.h (authored by aidengrossman).

Changed prior to commit:
  https://reviews.llvm.org/D157115?vs=547247=547329#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
-  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- clang/test/Headers/__cpuidex_conflict.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
-// compatibility built in by ensuring compilation succeeds:
-// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
-// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
-
-typedef __SIZE_TYPE__ size_t;
-
-#include 
-#include 
-
-int cpuid_info[4];
-
-void test_cpuidex(unsigned level, unsigned count) {
-  __cpuidex(cpuid_info, level, count);
-}
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,14 +328,4 @@
 return 1;
 }
 
-// If MS extensions are enabled, __cpuidex is defined as a builtin which will
-// conflict with the __cpuidex definition below.
-#ifndef _MSC_EXTENSIONS
-static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
-{
-  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
-__cpu_info[3]);
-}
-#endif
-
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
 
 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;
 
-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
-  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-04 Thread Aiden Grossman via Phabricator via cfe-commits 
aidengrossman added a comment.

Thanks for the quick response!

Sorry for all the churn with this patch. I'm fine leaving this out of LLVM 17, 
so I'll land this patch, backport the commit to the 17 release branch, and then 
once we have the aux triple and MS extensions flag fixes I'll reland in main 
and won't backport it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-04 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In D157115#4561497 , @aaron.ballman 
wrote:

> Thank you for this! The revert LGTM, but please wait for @mstorsjo to confirm 
> this won't cause problems for MinGW folks.

I guess this is fine.

We should probably sync mingw-w64 and revert 
https://github.com/mingw-w64/mingw-w64/commit/2b6c9247613aa830374e3686e09d3b8d582a92a6
 after that. There's less rush in reverting it though (not many projects use 
`__cpuidex` so it's not so bad to not have it defined it at all - while having 
a double conflicting definition is fatal), but I guess I should update it at 
least once the 17.0.0 release gets closer and we see where we're settled (maybe 
bump the version threshold to 18 instead of 17, if we're somewhat sure we'll 
have it in main even if it's reverted in 17.x).

For mingw, I'd at least appreciate if we don't switch it back and forth further 
on the 17.x release branch after 17.0.0 is released.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-04 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

Thank you for this! The revert LGTM, but please wait for @mstorsjo to confirm 
this won't cause problems for MinGW folks.

In terms of solving the underlying issue with aux triple so that we can re-land 
this, I went digging to see if there's an existing issue tracking this and I 
could not find one. However, related to the functionality in this patch, I did 
see: https://github.com/llvm/llvm-project/issues/60383. I filed 
https://github.com/llvm/llvm-project/issues/64436 to track the aux-triple 
issue; hopefully we can find someone with the bandwidth to look into that issue 
(I'll see if I can get someone at Intel to look into it, but if someone else 
wanted to tackle it first, that would be great).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-04 Thread Aiden Grossman via Phabricator via cfe-commits 
aidengrossman added a comment.

Assuming everyone agrees to this going in, I'll land it and then open a 
backport for the 17 release and reland once the auxiliary triple issues are 
resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157115

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

[PATCH] D157115: Revert "[clang][X86] Add __cpuidex function to cpuid.h"

2023-08-04 Thread Aiden Grossman via Phabricator via cfe-commits 
aidengrossman created this revision.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit 2df77ac20a1ed996706b164b0c4ed5ad140f635f 
.

This has been causing some issues with some windows builds as `_MSC_EXTENSIONS` 
isn't defined when only `-fms-extensions` is set, but the builtin that 
conflicts with __cpuidex is. This was also causing problems as it exposed some 
latent issues with how auxiliary triples are handled in clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157115

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/__cpuidex_conflict.c
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@

 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})

 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", 
"={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, 
i32 %{{[a-z0-9]+}})

 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;

-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1, edx1);
   __cpuid_count(level, count, eax0, ebx0, ecx0, edx0);
-  __cpuidex(cpuid_info, level, count);
 }
Index: clang/test/Headers/__cpuidex_conflict.c
===
--- clang/test/Headers/__cpuidex_conflict.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// Make sure that __cpuidex in cpuid.h doesn't conflict with the MS
-// compatibility built in by ensuring compilation succeeds:
-// RUN: %clang_cc1 %s -ffreestanding -fms-extensions -fms-compatibility \
-// RUN:  -fms-compatibility-version=19.00 -triple x86_64-pc-windows-msvc 
-emit-llvm -o -
-
-typedef __SIZE_TYPE__ size_t;
-
-#include 
-#include 
-
-int cpuid_info[4];
-
-void test_cpuidex(unsigned level, unsigned count) {
-  __cpuidex(cpuid_info, level, count);
-}
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -328,14 +328,4 @@
 return 1;
 }

-// If MS extensions are enabled, __cpuidex is defined as a builtin which will
-// conflict with the __cpuidex definition below.
-#ifndef _MSC_EXTENSIONS
-static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf)
-{
-  __cpuid_count(__leaf, __subleaf, __cpu_info[0], __cpu_info[1], __cpu_info[2],
-__cpu_info[3]);
-}
-#endif
-
 #endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -6,19 +6,14 @@

 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})

 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
-// CHECK-32: {{.*}} call { i32, i32, i32, i32 } asm "cpuid", "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})

 unsigned eax0, ebx0, ecx0, edx0;
 unsigned eax1, ebx1, ecx1, edx1;

-int cpuid_info[4];
-
 void test_cpuid(unsigned level, unsigned count) {
   __cpuid(level, eax1, ebx1, ecx1,