AcademySoftwareFoundation / openexr

The OpenEXR project provides the specification and reference implementation of the EXR file format, the professional-grade image storage format of the motion picture industry.
http://www.openexr.com/
BSD 3-Clause "New" or "Revised" License
1.62k stars 608 forks source link

MINGW : bad use of __cpuid() #1445

Open claudeha opened 1 year ago

claudeha commented 1 year ago

When cross-compiling OpenEXR-3.1.8 from the release tarball for Windows from Debian Bookworm (current testing distribution) using x86_64-w64-mingw32-g++ (GCC) 12-posix, the build fails because __cpuid() is called with 2 arguments instead of 5. This small patch seems to fix it:

diff --git a/openexr-3.1.8.old/src/lib/OpenEXRCore/internal_cpuid.h b/openexr-3.1.8.new/src/lib/OpenEXRCore/internal_cpuid.h
in
dex 9eb89d0..777a61f 100644
--- a/openexr-3.1.8.old/src/lib/OpenEXRCore/internal_cpuid.h
+++ b/openexr-3.1.8.new/src/lib/OpenEXRCore/internal_cpuid.h
@@ -35,7 +35,7 @@ static inline void check_for_x86_simd (int *f16c, int *avx, int *sse2)
     *avx = 1;
     *sse2 = 1;
 #        else
-#            ifdef _WIN32
+#            if defined(_MSC_VER) && defined(_WIN32)
     int regs[4], osxsave;

     __cpuid (regs, 0);
kmilos commented 1 year ago

No problems w/ MinGW GCC/Clang when not cross-compiling (e.g. using MSYS2), so restricting to _MSC_VER maybe not the best fix?

kmilos commented 1 year ago

Does it work w/ the win32 toolchain (which I guess is closer to native) rather than the posix one?

claudeha commented 1 year ago

I don't have Microsoft Windows so I can't check non-cross-compiling. Good to know that this patch may be bogus, even though it works for me.

I haven't tried win32 toolchain recently, it caused me too many issues with threading etc in the past. I have no time to try again soon unfortunately.

kmilos commented 1 year ago

Perhaps the real/better fix is to remove/change _MSC_VER here and here (i.e. just check for _WIN32):

https://github.com/AcademySoftwareFoundation/openexr/blob/e504fb2ec3952e5f4202c33e0baea21d8ea8f9e1/src/lib/OpenEXRCore/internal_cpuid.h#L9

https://github.com/AcademySoftwareFoundation/openexr/blob/e504fb2ec3952e5f4202c33e0baea21d8ea8f9e1/src/lib/OpenEXR/ImfSystemSpecific.cpp#L11

This is what I found in the MinGW shipped intrin.h:

/* Undefine the GCC one taking 5 parameters to prefer the mingw-w64 one. */
#undef __cpuid

This has popped up before IIRC - gotta keep in mind Windows as a platforms is not just MSVC... ;)

There might be other places where _MSC_VER is wrongly used as a catchall "Windows" alias.

kdt3rd commented 1 year ago

at least in the OpenEXRCore library, _MSC_VER should only be used to truly detect msvc, not the win32 api vs. not - although at this level of builtin functions vs. not, this may fall over occasionally. I believe I had actually addressed this already in #1467 (noticing that disconnect). Once that is merged, do let us know if there are other related issues. Thanks for noticing this

cary-ilm commented 1 year ago

@claudeha, could you confirm whether this is still and issue with your build configuration on the main branch? Hopefully, #1467 fixes it.

claudeha commented 1 year ago

On 10/07/2023 03:28, Cary Phillips wrote:

@claudeha https://github.com/claudeha, could you confirm whether this is still and issue with your build configuration on the |main| branch? Hopefully, #1467 https://github.com/AcademySoftwareFoundation/openexr/pull/1467 fixes it.

The library seems to build ok now, thanks.  But there is still an issue with src/test/OpenEXRCoreTest:

$ cd openexr
$ rm -rf build/
$ mkdir build
$ cd build/
$ PKG_CONFIG_PATH=${HOME}/tmp/windows/lib/pkgconfig cmake 
-DBUILD_SHARED_LIBS=OFF 
-DCMAKE_TOOLCHAIN_FILE=../cmake/Toolchain-mingw.cmake 
-DCMAKE_CXX_FLAGS="-I${HOME}/tmp/windows" 
-DCMAKE_EXE_LINKER_FLAGS="-L${HOME}/tmp/windows/lib" 
-DCMAKE_INSTALL_PREFIX="${HOME}/tmp/windows" ..
...
$ make -k
...
$ make -k
...
[ 63%] Building CXX object 
src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/base_units.cpp.obj
In file included from /usr/share/mingw-w64/include/intrin.h:41,
                  from /home/claude/tmp/windows/include/Imath/half.h:183,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr/src/test/OpenEXRCoreTest/../../lib/OpenEXRCore/internal_coding.h:24,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr/src/test/OpenEXRCoreTest/base_units.cpp:24:
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2013:42: error: 
macro "__cpuid" requires 5 arguments, but only 2 given
  2013 | void __cpuid(int CPUInfo[4], int InfoType);
       |                                          ^
In file included from 
/home/claude/opt/windows/posix/x86_64/src/openexr/src/test/OpenEXRCoreTest/../../lib/OpenEXRCore/internal_cpuid.h:18,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr/src/test/OpenEXRCoreTest/base_units.cpp:23:
/usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/cpuid.h:223: note: 
macro "__cpuid" defined here
   223 | #define __cpuid(level, a, b, c, 
d)                                      \
       |
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:42: error: 
macro "__cpuid" requires 5 arguments, but only 2 given
  2016 | void __cpuid(int CPUInfo[4], int InfoType) {
       |                                          ^
/usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/cpuid.h:223: note: 
macro "__cpuid" defined here
   223 | #define __cpuid(level, a, b, c, 
d)                                      \
       |
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2013:6: error: 
variable or field ‘__cpuid’ declared void
  2013 | void __cpuid(int CPUInfo[4], int InfoType);
       |      ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:6: error: 
variable or field ‘__cpuid’ declared void
  2016 | void __cpuid(int CPUInfo[4], int InfoType) {
       |      ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2017:4: error: 
expected primary-expression before ‘__asm__’
  2017 |    __asm__ __volatile__ (
       |    ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2017:4: error: 
expected ‘}’ before ‘__asm__’
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:44: note: to 
match this ‘{’
  2016 | void __cpuid(int CPUInfo[4], int InfoType) {
       |                                            ^
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2280:1: error: 
expected declaration before ‘}’ token
  2280 | }
       | ^
make[2]: *** 
[src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/build.make:77: 
src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/base_units.cpp.obj] 
Error 1
make[2]: Target 
'src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/build' not 
remade because of errors.
...

Host is Debian Bookworm compiling with MINGW mingw-w64 10.0.0-3 (x86_64-w64-mingw32-g++ (GCC) 12-posix).

claudeha commented 1 year ago

for me there is still an issue with the tests with openexr-3.2.0-rc:

claude@eiskaffee:~/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/build$ make -k >/dev/null 2>/dev/null && make
[  2%] Built target Iex
[  5%] Built target IlmThread
[ 17%] Built target OpenEXRCore
[ 49%] Built target OpenEXR
[ 54%] Built target OpenEXRUtil
[ 55%] Built target exr2aces
[ 56%] Built target exrheader
[ 57%] Built target exrinfo
[ 58%] Built target exrmaketiled
[ 59%] Built target exrstdattr
[ 60%] Built target exrmakepreview
[ 63%] Built target exrenvmap
[ 64%] Built target exrmultiview
[ 65%] Built target exrmultipart
[ 65%] Built target exrcheck
[ 69%] Built target OpenEXRExamples
[ 71%] Built target IexTest
[ 71%] Building CXX object src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/base_units.cpp.obj
In file included from /usr/share/mingw-w64/include/intrin.h:41,
                 from /home/claude/opt/windows/posix/x86_64/include/Imath/half.h:183,
                 from /home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/../../lib/OpenEXRCore/internal_coding.h:24,
                 from /home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/base_units.cpp:24:
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2013:42: error: macro "__cpuid" requires 5 arguments, but only 2 given
 2013 | void __cpuid(int CPUInfo[4], int InfoType);
      |                                          ^
In file included from /home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/../../lib/OpenEXRCore/internal_cpuid.h:21,
                 from /home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/base_units.cpp:23:
/usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/cpuid.h:223: note: macro "__cpuid" defined here
  223 | #define __cpuid(level, a, b, c, d)                                      \
      | 
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:42: error: macro "__cpuid" requires 5 arguments, but only 2 given
 2016 | void __cpuid(int CPUInfo[4], int InfoType) {
      |                                          ^
/usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/cpuid.h:223: note: macro "__cpuid" defined here
  223 | #define __cpuid(level, a, b, c, d)                                      \
      | 
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2013:6: error: variable or field ‘__cpuid’ declared void
 2013 | void __cpuid(int CPUInfo[4], int InfoType);
      |      ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:6: error: variable or field ‘__cpuid’ declared void
 2016 | void __cpuid(int CPUInfo[4], int InfoType) {
      |      ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2017:4: error: expected primary-expression before ‘__asm__’
 2017 |    __asm__ __volatile__ (
      |    ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2017:4: error: expected ‘}’ before ‘__asm__’
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:44: note: to match this ‘{’
 2016 | void __cpuid(int CPUInfo[4], int InfoType) {
      |                                            ^
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2280:1: error: expected declaration before ‘}’ token
 2280 | }
      | ^
make[2]: *** [src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/build.make:77: src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/base_units.cpp.obj] Error 1
make[2]: Target 'src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/build' not remade because of errors.
make[1]: *** [CMakeFiles/Makefile2:1823: src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/all] Error 2
meshula commented 1 year ago

I notice this

/home/claude/opt/windows/posix/x86_64/include/Imath/half.h

It makes me wonder if you have a very old system installed half.h, and if using a new imath would fix the issue. Can you try reconfiguring your build, using the OPENEXR_FORCE_INTERNAL_IMATH flag?

claudeha commented 1 year ago

On 21/08/2023 21:42, Nick Porcino wrote:

I notice this

|/home/claude/opt/windows/posix/x86_64/include/Imath/half.h |

It makes me wonder if you have a very old system installed half.h, and if using a new imath would fix the issue.

I was using Imath 3.1.9 (I think this is the latest stable release?)

Can you try reconfiguring your build, using the |OPENEXR_FORCE_INTERNAL_IMATH| flag?

adding -DOPENEXR_FORCE_INTERNAL_IMATH=on breaks detection of my system installed libdeflate.h (configured using -DCMAKE_CXX_FLAGS="-I${prefix}/include") also adding -DOPENEXR_FORCE_INTERNAL_DEFLATE=on gives a similar __cpuid error as before:

[ 74%] Building CXX object 
src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/base_units.cpp.obj
In file included from /usr/share/mingw-w64/include/intrin.h:41,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/build/_deps/imath-src/src/Imath/half.h:183,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/../../lib/OpenEXRCore/internal_coding.h:24,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/base_units.cpp:24:
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2013:42: error: 
macro "__cpuid" requires 5 arguments, but only 2 given
  2013 | void __cpuid(int CPUInfo[4], int InfoType);
       |                                          ^
In file included from 
/home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/../../lib/OpenEXRCore/internal_cpuid.h:21,
                  from 
/home/claude/opt/windows/posix/x86_64/src/openexr-3.2.0-rc/src/test/OpenEXRCoreTest/base_units.cpp:23:
/usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/cpuid.h:223: note: 
macro "__cpuid" defined here
   223 | #define __cpuid(level, a, b, c, 
d)                                      \
       |
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:42: error: 
macro "__cpuid" requires 5 arguments, but only 2 given
  2016 | void __cpuid(int CPUInfo[4], int InfoType) {
       |                                          ^
/usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/cpuid.h:223: note: 
macro "__cpuid" defined here
   223 | #define __cpuid(level, a, b, c, 
d)                                      \
       |
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2013:6: error: 
variable or field ‘__cpuid’ declared void
  2013 | void __cpuid(int CPUInfo[4], int InfoType);
       |      ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:6: error: 
variable or field ‘__cpuid’ declared void
  2016 | void __cpuid(int CPUInfo[4], int InfoType) {
       |      ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2017:4: error: 
expected primary-expression before ‘__asm__’
  2017 |    __asm__ __volatile__ (
       |    ^~~~~~~
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2017:4: error: 
expected ‘}’ before ‘__asm__’
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2016:44: note: to 
match this ‘{’
  2016 | void __cpuid(int CPUInfo[4], int InfoType) {
       |                                            ^
/usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:2280:1: error: 
expected declaration before ‘}’ token
  2280 | }
       | ^
make[2]: *** 
[src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/build.make:77: 
src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/base_units.cpp.obj] 
Error 1
make[1]: *** [CMakeFiles/Makefile2:1914: 
src/test/OpenEXRCoreTest/CMakeFiles/OpenEXRCoreTest.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

This problem earlier occured in the main code too, but it was fixed there (but it seems not in the tests).