afritz1 / OpenTESArena

Open-source re-implementation of The Elder Scrolls: Arena.
MIT License
915 stars 68 forks source link

Compilation error in Windows (Msys2): inlining failed in call to always_inline 'int _mm_extract_epi32(__m128i, int) #163

Closed abelsromero closed 4 years ago

abelsromero commented 4 years ago

I wanted to compile the latest version with Msys2 and I stumbled upon this error.

This appears several times but it's always the same error.

[  9%] Building CXX object OpenTESArena/CMakeFiles/TESArena.dir/src/Rendering/SoftwareRenderer.cpp.obj
In file included from I:/bin/msys64/mingw64/lib/gcc/x86_64-w64-mingw32/9.2.0/include/immintrin.h:37,
                 from I:/bin/msys64/home/abelsr/games/OpenTESArena/OpenTESArena/src/Rendering/SoftwareRenderer.cpp:4:
I:/bin/msys64/mingw64/lib/gcc/x86_64-w64-mingw32/9.2.0/include/smmintrin.h: In static member function 'static void SoftwareRenderer::drawDistantPixelsSSE(int, const SoftwareRenderer::DrawRange&, double, double, double, const SoftwareRenderer::SkyTexture&, bool, const SoftwareRenderer::ShadingInfo&, const SoftwareRenderer::FrameView&)':
I:/bin/msys64/mingw64/lib/gcc/x86_64-w64-mingw32/9.2.0/include/smmintrin.h:447:1: error: inlining failed in call to always_inline 'int _mm_extract_epi32(__m128i, int)': target specific option mismatch
  447 | _mm_extract_epi32 (__m128i __X, const int __N)
      | ^~~~~~~~~~~~~~~~~
I:/bin/msys64/home/abelsr/games/OpenTESArena/OpenTESArena/src/Rendering/SoftwareRenderer.cpp:3632:41: note: called from here
 3632 |   const bool opaque1 = _mm_extract_epi32(opaques, 0) != 0;
      |                        ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
afritz1 commented 4 years ago

I recently started experimenting with SSE instructions in the software renderer to get the ball rolling with performance-related things.

_mm_extract_epi32() and _mm_blendv_pd() are SSE4.1 instructions. I believe SSE2 is enabled by default in MSVC and GCC these days but an additional compile option is needed for SSE4.1 in GCC (and apparently MSYS2).

I added it in CMake but only for GCC: https://github.com/afritz1/OpenTESArena/commit/2451a34fdfc1fae454a877723d14f7d0598b1dcf

The option needs to be added for MSYS2. Probably just add some condition in CMakeLists.txt for it similar to the existing -msse4.1 one?

afritz1 commented 4 years ago

It's possible this would work. I don't know if MSYS2 satisfies the CMAKE_COMPILER_IS_GNUCXX condition in any way, though.

IF (CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
    # To use std::thread and friends, we must pass -lpthread and -pthread to the compiler and Linker for GCC
    IF (NOT WIN32 AND NOT APPLE)
        SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread")
        SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")
        SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lpthread -pthread")
    ENDIF ()

    # Optimizations.
    SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -msse4.1")
ENDIF ()
afritz1 commented 4 years ago

Ping! Have you had a chance to test out the changes?

abelsromero commented 4 years ago

Adding the -mss4.1 as you pointed worked perfectly, I could compile and run the game without issues.

It seems MSYS2 does honor CMAKE_COMPILER_IS_GNUCXX, I used https://cmake.org/cmake/help/v3.0/command/message.html to show some message from the if section before the optimitzation and it showed :+1:.

afritz1 commented 4 years ago

Great, I will add the flags as depicted above.

afritz1 commented 4 years ago

Commit 805aa65be57eaede439dc82e987676b0356852ce