RenderKit / embree

Embree ray tracing kernels repository.
Apache License 2.0
2.32k stars 383 forks source link

Fix use of undeclared identifier __cpuidex error on MinGW #487

Open TCROC opened 1 month ago

TCROC commented 1 month ago

This was discovered in the godot game engine and the PR for that fix can be found over here: https://github.com/godotengine/godot/pull/92490.

Although we will likely again patch that with whatever fix the maintainers of this repo decide if they want a different fix. Without further ado, here we go! (copying in the description from the above PR)

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! :)

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.