microsoft / DirectXMath

DirectXMath is an all inline SIMD C++ linear algebra library for use in games and graphics apps
https://walbourn.github.io/introducing-directxmath/
MIT License
1.51k stars 231 forks source link

Use two-argument cpuid when using recent MinGW #172

Closed nirbheek closed 6 months ago

nirbheek commented 7 months ago

Since 2021, when building with MinGW if intrin.h is included (directly or indirectly), it will redefine __cpuid to use the intrinsic version of it, instead of the macro defined by GCC's (and clang's) cpuid.h. Detect when __cpuid is a define, and use the 5-argument version only in that case.

https://github.com/mingw-w64/mingw-w64/commit/d2374f898457b0f4ea8bd4084a94f2dafc87a99a

This change does not apply to Clang.

walbourn commented 7 months ago

Thanks. Is this change post MinGW 12.2?

nirbheek commented 7 months ago

No, this was in 10.0.0:

$ git describe d2374f898457b0f4ea8bd4084a94f2dafc87a99a
v9.0.0-2-gd2374f898

To reproduce, you need to #include <intrin.h> or something that includes that, like wrl.h (which was how I saw it).

walbourn commented 7 months ago

/azp run DirectXMath-GitHub,DirectXMath-GitHub-Dev17,DirectXMath-GitHub-WSL,DirectXMath-GitHub-WSL-11,DirectXMath-GitHub-MinGW,DirectXMath-GitHub-CMake-Dev17

azure-pipelines[bot] commented 7 months ago
Azure Pipelines successfully started running 3 pipeline(s).
JPeterMugaas commented 6 months ago

I'm having a similar problem when trying to compile DirectXTex with OpenEXR support with MINGW. Just for fun, I search the source-code using the command, find -type f -exec grep -nH "cpuid" {} \; and found these:

./Extensions/DirectXMathAVX.h:32:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathAVX.h:34:    __cpuid( CPUInfo, 0 );
./Extensions/DirectXMathAVX.h:41:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathAVX.h:43:    __cpuid(CPUInfo, 1 );
./Extensions/DirectXMathAVX2.h:33:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathAVX2.h:35:    __cpuid(CPUInfo, 0);
./Extensions/DirectXMathAVX2.h:42:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathAVX2.h:44:    __cpuid(CPUInfo, 1);
./Extensions/DirectXMathAVX2.h:52:    __cpuid_count(7, 0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathAVX2.h:54:    __cpuidex(CPUInfo, 7, 0);
./Extensions/DirectXMathBE.h:63:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathBE.h:65:    __cpuid(CPUInfo, 0);
./Extensions/DirectXMathBE.h:72:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathBE.h:74:    __cpuid(CPUInfo, 1);
./Extensions/DirectXMathF16C.h:33:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathF16C.h:35:    __cpuid(CPUInfo, 0);
./Extensions/DirectXMathF16C.h:42:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathF16C.h:44:    __cpuid(CPUInfo, 1);
./Extensions/DirectXMathFMA3.h:32:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathFMA3.h:34:    __cpuid(CPUInfo, 0);
./Extensions/DirectXMathFMA3.h:41:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathFMA3.h:43:    __cpuid(CPUInfo, 1);
./Extensions/DirectXMathFMA4.h:37:   __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathFMA4.h:39:   __cpuid(CPUInfo, 0);
./Extensions/DirectXMathFMA4.h:46:   __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathFMA4.h:48:   __cpuid(CPUInfo, 1);
./Extensions/DirectXMathFMA4.h:56:    __cpuid(0x80000000, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathFMA4.h:58:    __cpuid(CPUInfo, 0x80000000);
./Extensions/DirectXMathFMA4.h:66:    __cpuid(0x80000001, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathFMA4.h:68:    __cpuid(CPUInfo, 0x80000001);
./Extensions/DirectXMathSSE3.h:33:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathSSE3.h:35:    __cpuid(CPUInfo, 0);
./Extensions/DirectXMathSSE3.h:41:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathSSE3.h:43:    __cpuid(CPUInfo, 1);
./Extensions/DirectXMathSSE4.h:33:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathSSE4.h:35:    __cpuid(CPUInfo, 0);
./Extensions/DirectXMathSSE4.h:41:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Extensions/DirectXMathSSE4.h:43:    __cpuid(CPUInfo, 1);
./HISTORY.md:59:* XMVerifyCPUSupport updated for clang/LLVM cpuid implementation on x86/x64
./Inc/DirectXMath.h:125:#include <cpuid.h>
./Inc/DirectXMathMisc.inl:1977:    __cpuid(0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Inc/DirectXMathMisc.inl:1979:    __cpuid(CPUInfo, 0);
./Inc/DirectXMathMisc.inl:1991:    __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Inc/DirectXMathMisc.inl:1993:    __cpuid(CPUInfo, 1);
./Inc/DirectXMathMisc.inl:2026:    __cpuid_count(7, 0, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
./Inc/DirectXMathMisc.inl:2028:    __cpuidex(CPUInfo, 7, 0);

It seems that there is a conflict between intrin.h and cpuid.h in MINGW.

nirbheek commented 6 months ago

I'm having a similar problem when trying to compile DirectXTex with OpenEXR support with MINGW

@JPeterMugaas I've updated the PR, could you try again with this branch? I think I've covered every case now.

@walbourn a colleague tested on mingw with clang and found that this workaround was also needed there, so I've updated the patch for that too.

nirbheek commented 6 months ago

@microsoft-github-policy-service agree company="Centricular Ltd"

nirbheek commented 6 months ago

@microsoft-github-policy-service agree company="Centricular"

JPeterMugaas commented 6 months ago

I checked the patch for this PR and it didn't work right. I got these error:

In file included from C:/msys64/home/jpmugaas/directxtex/Auxiliary/DirectXTexEXR.cpp:14:
In file included from C:/msys64/home/jpmugaas/directxtex/DirectXTex/DirectXTexP.h:155:
In file included from C:/msys64/clang64/include/directxmath/DirectXPackedVector.h:12:
In file included from C:/msys64/clang64/include/directxmath/DirectXMath.h:2272:
C:/msys64/clang64/include/directxmath/DirectXMathMisc.inl:1991:5: error: no matching function for call to '__cpuid'
 1991 |     __cpuid(1, CPUInfo[0], CPUInfo[1], CPUInfo[2], CPUInfo[3]);
      |     ^~~~~~~
C:/msys64/clang64/include/intrin.h:1139:21: note: candidate function not viable: requires 2 arguments, but 5 were provided
 1139 |     __MACHINEI(void __cpuid(int a[4],int b))
      |                     ^       ~~~~~~~~~~~~~~
C:/msys64/clang64/include/intrin.h:178:20: note: expanded from macro '__MACHINEI'
  178 | #define __MACHINEI __MACHINE
      |                    ^
C:/msys64/clang64/include/intrin.h:192:22: note: expanded from macro '__MACHINE'
  192 | #define __MACHINE(X) X;
      |                      ^
2 errors generated.

instead of: #if (defined(__clang__) || (defined(__GNUC__)) && defined(__cpuid))

the line should read: #if (defined(__clang__) || (defined(__GNUC__))) && defined(__cpuid)

We want the or to be evaluated as one part and the "defined(__cpuid)" as another part of the and. Notice the difference between the two lines.

The second one works for me.

nirbheek commented 6 months ago

Sorry about the typo, the construct is now: #if (defined(__clang__) || defined(__GNUC__)) && defined(__cpuid) which is equivalent to the second one you pasted.

JPeterMugaas commented 6 months ago

Alright, your latest patch works in both GNU 13.2.0 and Clang 17.0.6.

Merry Christmas!!!

walbourn commented 6 months ago

/azp run DirectXMath-GitHub,DirectXMath-GitHub-Dev17,DirectXMath-GitHub-WSL,DirectXMath-GitHub-WSL-11,DirectXMath-GitHub-MinGW,DirectXMath-GitHub-CMake-Dev17

azure-pipelines[bot] commented 6 months ago
Azure Pipelines successfully started running 3 pipeline(s).
walbourn commented 6 months ago

@nirbheek Can you comment on why this change wasn't sufficient to resolve the problem for @JPeterMugaas

https://github.com/microsoft/DirectXMath/pull/176