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

Sun Aug 13 13:42:12 PDT 2023

mstorsjo added a comment.

In D150646#4581664 <https://reviews.llvm.org/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 reasonable - what do we do for other MSVC-specific builtins today?

In D150646#4581672 <https://reviews.llvm.org/D150646#4581672>, @aidengrossman wrote:

> 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.

That's indeed the issue

> I wouldn't be opposed to exposing things more often, but I'm fine with the builtins being exposed under `-fms-extensions`, we just need to expose the preprocessor macro when we expose the builtins.

As far as I can see, @rnk's suggestion is the opposite - to tie whether the builtin is enabled to the target only, not to `-fms-extensions`. I.e. mingw + `-fms-extensions` doesn't get the builtin. Then the preprocessor check for it would simply be `!defined(_MSC_VER)`.

Unfortunately, for Clang tests that invoke `clang -cc1` directly, with an MSVC target triple, don't get `_MSC_VER` defined automatically, only if they pass `-fms-compatibility-version=` (which the Driver usually passes), so in such test conditions, the main way of detecting MSVC right now is `defined(_WIN32) && !defined(__MINGW32__)` which isn't very pretty either.

I played with a patch to make Clang always define `_MSC_VER` for MSVC targets, even if `-fms-compatibility-version=` wasn't set. That broke quite a few tests, since `_MSC_VER` unlocks lots of codepaths that only work when `-fms-extensions` is set (which the Driver usually does for MSVC targets, but usually isn't set in `%clang_cc1` tests). So if we want to default to defining `_MSC_VER` on the cc1 level without an explicit `-fms-compatibility-version=`, we'd also need to imply `-fms-extensions` for MSVC targets, which I guess would go against a lot of the driver/frontend logic split.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646