[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

mstorsjo added a comment. In D150646#4581664 , @rnk wrote: > I think we should be exposing the `__cpudex` builtin any time we are being > MSVC-like, not under `fms-compatibility`. Would that make things sensible > again? I think that might sound

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. The main issue here is that there's no way to reliably detect whether the built in is defined (can't check if a function is defined in the preprocessor, preprocessor macro isn't exposed correctly in all configurations), which ends up breaking some configurations.

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

rnk added a comment. I think MSVC defines `_MSC_EXTENSIONS` under one of its compatibility modes as well, so it's not a good indicator of "is this compiler MSVC-like". I think we should be exposing the `__cpudex` builtin any time we are being MSVC-like, not under `fms-compatibility`. Would

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aaron.ballman added subscribers: jdoerfert, ABataev, rnk. aaron.ballman added a comment. In D150646#4558543 , @aidengrossman wrote: > I'm not opposed to a revert, but I know some downstream users like MinGW have > already adapted to this change so I'm

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. I'm not opposed to a revert, but I know some downstream users like MinGW have already adapted to this change so I'm not sure how much headache it would cause them to do a revert. Maybe I'm not understanding things correctly, but I originally `_MSC_EXTENSIONS`

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aaron.ballman added a comment. In D150646#4450915 , @aidengrossman wrote: > In D150646#4450551 , @glandium > wrote: > >> Did you find something? > > Not yet. I just finished finals a week ago so I haven't had

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. In D150646#4450551 , @glandium wrote: > Did you find something? Not yet. I just finished finals a week ago so I haven't had as much time as I would've liked to get through this. I'll contact some people tonight in regards

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

glandium added a comment. In D150646#4382763 , @aidengrossman wrote: > Interesting. I would've thought `-fms-extensions` by itself would've declared > that macro (which is what I gathered reading what caused the MSVC compatible > builtins to get

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. In D150646#4379543 , @glandium wrote: > There seem to still be two problems with this change, with mingw: > > - with `-fms-extensions`: > > echo '#include "cpuid.h"' | ./clang/bin/clang++ >

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

glandium added a comment. In D150646#4380058 , @mstorsjo wrote: > Regarding "fixed" - do you see any better way of fixing it there? As it's a > static inline function, there's no simple way of knowing whether it was > already defined or not; I went

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

mstorsjo added a comment. In D150646#4379543 , @glandium wrote: > There seem to still be two problems with this change, with mingw: > > - with `-fms-extensions`: > > echo '#include "cpuid.h"' | ./clang/bin/clang++ > --target=x86_64-w64-windows-gnu

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

glandium added a comment. There seem to still be two problems with this change, with mingw: - with `-fms-extensions`: echo '#include "cpuid.h"' | ./clang/bin/clang++ --target=x86_64-w64-windows-gnu -xc++ - -fms-extensions In file included from :1:

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. Do you have any more information? Do you have your own implementation of `__cpuidex` somewhere else before including `cpuid.h`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150646/new/

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

ronlieb added a comment. seeing this issue in our downstream builders: [2023-05-24T11:46:07.333Z] /opt/rocm-5.6.0-12074/llvm/lib/clang/17.0.0/include/cpuid.h:333:22: error: static declaration of '__cpuidex' follows non-static declaration [2023-05-24T11:46:07.333Z] static __inline void

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2df77ac20a1e: [clang][X86] Add __cpuidex function to cpuid.h (authored by aidengrossman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman updated this revision to Diff 523686. aidengrossman added a comment. Fix conflict with MS compatibility __cpuidex builtin. This update adds a preprocessor guard around the __cpuidex definition in cpuid.h that checks if _MSC_EXTENSIONS is defined. If _MSC_EXTENSIONS is defined, we

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

glandium added a comment. This is at least one of the spots where it happens: https://searchfox.org/mozilla-central/rev/2ca95198a2a0806de358a0484d96d4354e3cbaab/third_party/highway/hwy/targets.cc#42 (HWY_COMPILER_MSVC is explicitly 0 with clang-cl per

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

craig.topper added a comment. In D150646#4351981 , @aidengrossman wrote: > Thanks for notifying me of this. I've reverted it as I don't have too much > time currently to thoroughly go through things and I don't want to keep you > blocked. Essentially,

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. Thanks for notifying me of this. I've reverted it as I don't have too much time currently to thoroughly go through things and I don't want to keep you blocked. Essentially, the built-in for `__cpuidex` on Windows platforms landed in

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

glandium added a comment. Repro: $ echo '#include "cpuid.h"' | ./clang/bin/clang-cl -Tp - clang-cl: warning: unable to find a Visual Studio installation; try running Clang from a developer command prompt [-Wmsvc-not-found] In file included from :1:

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

glandium added a comment. This causes errors like: /builds/worker/fetches/clang/lib/clang/17/include/cpuid.h(331,22): error: definition of builtin function '__cpuidex' static __inline void __cpuidex (int __cpu_info[4], int __leaf, int __subleaf) ^ 1 error

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG286cefcf35d0: [clang][X86] Add __cpuidex function to cpuid.h (authored by aidengrossman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

aidengrossman added a comment. Thanks for the review! Definitely a little bit odd and it probably would've been better if originally replicated in some other way, but definitely agree on the matching behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM I find it a little odd that we have if you include cpuid.h you get the gcc interface for __cpuid and the MSVC interface for __cpuidex. But gcc did it first so I guess we

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

craig.topper added a comment. For MSVC compatibility this was already implemented as a builtin for intrin.h in https://reviews.llvm.org/D121653 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150646/new/ https://reviews.llvm.org/D150646

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

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. MSVC has a __cpuidex function implemented to call the underlying cpuid instruction which accepts a leaf,