Fix use of undeclared identifier __cpuidex error on MinGW

[TCROC]

This change requires the target to be msvc in order to reference the method __cpuidex. The reasoning is due to a breaking change that appears to have come into effect recently across all major c / c++ compilers. Details are as follows:

LLVM, Zig (which uses LLVM under the hood), and GCC all recently updated their compilers changing what was a warning to an error. The compiler will now error if attempting to reference __cpuidex and not targetting msvc. It will no longer work for MINGW targets.

Here are relevant docs / source code for reference

Discovered by @Rexicon226, here is the LLVM code and how it handles it:

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/BLAKE3/blake3_dispatch.c#L46-L60

Another helpful zig dev who's discord username is "random internet person" found: https://github.com/ziglang/zig/blob/master/lib/zig.h#L3902-L3913

Which appears similar to LLVM's new implementation.

And GNU posted a more formal announcement notifying of the warn to error change that appears to have propagated across all major compilers recently: https://gcc.gnu.org/gcc-14/porting_to.html#warnings-as-errors

Shoutout to @paperdave for all the help and guidance along the way! :)

[TCROC]

For further clarification. The warning-to-error is not specific to __cpuidex. That is one of the effected methods. Through conversation in the zig discord, I believe it has to do with no longer allowing implicit function definitions at compile time. Fortunately this is the only affected spot I've found so far.

[akien-mga]

Thanks for the research and the fix. Some comments:

• This should be reported / PR'ed upstream to https://github.com/RenderKit/embree. We can have a downstream patch temporarily but eventually this needs to be fixed in the original library.
• I haven't looked into details yet, but would it work to use the #else (non _WIN32) branch, which I supposed is meant for GCC/Clang? It might work with MinGW GCC/Clang. So the fix would then be:

diff --git a/thirdparty/embree/common/sys/sysinfo.cpp b/thirdparty/embree/common/sys/sysinfo.cpp
index d01eab3c9d..4c5495c2ce 100644
--- a/thirdparty/embree/common/sys/sysinfo.cpp
+++ b/thirdparty/embree/common/sys/sysinfo.cpp
@@ -293,11 +293,8 @@ namespace embree
     int cpuid_leaf_7[4] = { 0,0,0,0 };
     int cpuid_leaf_e1[4] = { 0,0,0,0 };
     if (nIds >= 1) __cpuid (cpuid_leaf_1,0x00000001);
-#if _WIN32
-#if _MSC_VER && (_MSC_FULL_VER < 160040219)
-#else
+#if _MSC_VER && (_MSC_FULL_VER >= 160040219)
     if (nIds >= 7) __cpuidex(cpuid_leaf_7,0x00000007,0);
-#endif
 #else
     if (nIds >= 7) __cpuid_count(cpuid_leaf_7,0x00000007,0);
 #endif

[TCROC]

Thanks for the research and the fix. Some comments:

* This should be reported / PR'ed upstream to https://github.com/RenderKit/embree. We can have a downstream patch temporarily but eventually this needs to be fixed in the original library.

* I haven't looked into details yet, but would it work to use the `#else` (non `_WIN32`) branch, which I supposed is meant for GCC/Clang? It might work with MinGW GCC/Clang. So the fix would then be:

diff --git a/thirdparty/embree/common/sys/sysinfo.cpp b/thirdparty/embree/common/sys/sysinfo.cpp
index d01eab3c9d..4c5495c2ce 100644
--- a/thirdparty/embree/common/sys/sysinfo.cpp
+++ b/thirdparty/embree/common/sys/sysinfo.cpp
@@ -293,11 +293,8 @@ namespace embree
     int cpuid_leaf_7[4] = { 0,0,0,0 };
     int cpuid_leaf_e1[4] = { 0,0,0,0 };
     if (nIds >= 1) __cpuid (cpuid_leaf_1,0x00000001);
-#if _WIN32
-#if _MSC_VER && (_MSC_FULL_VER < 160040219)
-#else
+#if _MSC_VER && (_MSC_FULL_VER >= 160040219)
     if (nIds >= 7) __cpuidex(cpuid_leaf_7,0x00000007,0);
-#endif
 #else
     if (nIds >= 7) __cpuid_count(cpuid_leaf_7,0x00000007,0);
 #endif

I will try and see if that works

[TCROC]

@akien-mga

I can confirm that it does not like that. It errors with:

[ 29%] thirdparty/embree/common/sys/sysinfo.cpp:299:59: error: too few arguments provided to function-like macro invocation
  299 |     if (nIds >= 7) __cpuid_count(cpuid_leaf_7,0x00000007,0);
      |                                                           ^
/home/tcroc/dev/BlockyBallOT/submodules/godot-src/gitignore/zig/0.13.0-dev.267+793f820b3/zig-linux-x86_64-0.13.0-dev.267+793f820b3/lib/include/cpuid.h:265:9: note: macro '__cpuid_count' defined here
  265 | #define __cpuid_count(__leaf, __count, __eax, __ebx, __ecx, __edx) \
      |         ^
thirdparty/embree/common/sys/sysinfo.cpp:299:20: error: use of undeclared identifier '__cpuid_count'
  299 |     if (nIds >= 7) __cpuid_count(cpuid_leaf_7,0x00000007,0);
      |                    ^
[ 30%] 2 errors generated.

Anything else you would like me to try or should we leave it as is? Should I put a PR upstream to see if they have any feedback?

[akien-mga]

Should I put a PR upstream to see if they have any feedback?

Yes, please do. If they're reactive, they should point to the proper / best solution for their codebase.

[TCROC]

Should I put a PR upstream to see if they have any feedback?

Yes, please do. If they're reactive, they should point to the proper / best solution for their codebase.

Perfect! Then we can replace our patch with their fix assuming its appropriate for our codebase as well :)

[TCROC]

Put in an upstream PR over here: RenderKit/embree#487

[akien-mga]

Could you also include the patch we made to thirdparty code as a patch file in the same commit?

I.e. thirdparty/embree/patches/mingw-no-cpuidex.patch:

diff --git a/thirdparty/embree/common/sys/sysinfo.cpp b/thirdparty/embree/common/sys/sysinfo.cpp
index d01eab3c9d..4ecab05265 100644
--- a/thirdparty/embree/common/sys/sysinfo.cpp
+++ b/thirdparty/embree/common/sys/sysinfo.cpp
@@ -295,7 +295,7 @@ namespace embree
     if (nIds >= 1) __cpuid (cpuid_leaf_1,0x00000001);
 #if _WIN32
 #if _MSC_VER && (_MSC_FULL_VER < 160040219)
-#else
+#elif defined(_MSC_VER)
     if (nIds >= 7) __cpuidex(cpuid_leaf_7,0x00000007,0);
 #endif
 #else

This is how we document (and reapply on update) the downstream changes we made.

[TCROC]

@akien-mga Got distracted by some things on our game. I'll look into that this week.

[TCROC]

@akien-mga I added the patch file :)

[akien-mga]

Thanks!