From 96099a79a8ec250624071c75148c7855f87d073d Mon Sep 17 00:00:00 2001 From: James Zern Date: Thu, 19 Nov 2020 17:05:46 -0800 Subject: [PATCH] cmake: fix per-file assembly flags Since a376e7b9 Fix compilation on windows and clang-cl+ninja. the highest level assembly flag (/arch:AVX / -msse4.1) would be applied to all assembly files. This could result in higher level instructions being used which would defeat the runtime cpu detection check and could result in a crash. With this change -m style flags are used with clang-cl as it supports them and allows the correct level to be set. With at least Visual Studio 12 (2013)+ /arch is not necessary to build SSE2 or SSE4 code unless a lesser /arch is forced so these flags are avoided. This matches the nmake build. For emscripten only sse2 and sse4.1 are tested (NEON will succeed in being enabled, but fail to build). This is consistent with the current behavior added in: commit 36c81ff6a940f45b492e137b27eae7ff268c3cd8 WASM-SIMD: port 2 patches from rreverser@'s tree * Stop on first found SIMD version * Imply lower SSE version when higher is found Bug: webp:488 Change-Id: I34d01274e5204a477b6b9f35ed566048a62b4c57 Tested: msvc 2013-2019 debug win32/x64 builds; clang-cl under 2019 --- cmake/cpu.cmake | 63 ++++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/cmake/cpu.cmake b/cmake/cpu.cmake index 940e14c2..b01e4666 100644 --- a/cmake/cpu.cmake +++ b/cmake/cpu.cmake @@ -31,9 +31,17 @@ endfunction() set(WEBP_SIMD_FLAGS "SSE41;SSE2;MIPS32;MIPS_DSP_R2;NEON;MSA") set(WEBP_SIMD_FILE_EXTENSIONS "_sse41.c;_sse2.c;_mips32.c;_mips_dsp_r2.c;_neon.c;_msa.c") -if(MSVC) - # MSVC does not have a SSE4 flag but AVX support implies SSE4 support. - set(SIMD_ENABLE_FLAGS "/arch:AVX;/arch:SSE2;;;;") +if(MSVC AND CMAKE_C_COMPILER_ID STREQUAL "MSVC") + # With at least Visual Studio 12 (2013)+ /arch is not necessary to build SSE2 + # or SSE4 code unless a lesser /arch is forced. MSVC does not have a SSE4 + # flag, but an AVX one. Using that with SSE4 code risks generating illegal + # instructions when used on machines with SSE4 only. The flags are left for + # older (untested) versions to avoid any potential compatibility issues. + if(MSVC_VERSION GREATER_EQUAL 1800 AND NOT CMAKE_C_FLAGS MATCHES "/arch:") + set(SIMD_ENABLE_FLAGS) + else() + set(SIMD_ENABLE_FLAGS "/arch:AVX;/arch:SSE2;;;;") + endif() set(SIMD_DISABLE_FLAGS) else() set(SIMD_ENABLE_FLAGS @@ -57,9 +65,14 @@ endif() list(LENGTH WEBP_SIMD_FLAGS WEBP_SIMD_FLAGS_LENGTH) math(EXPR WEBP_SIMD_FLAGS_RANGE "${WEBP_SIMD_FLAGS_LENGTH} - 1") -unset(HIGHEST_SSE_FLAG) foreach(I_SIMD RANGE ${WEBP_SIMD_FLAGS_RANGE}) + # With Emscripten 2.0.9 -msimd128 -mfpu=neon will enable NEON, but the + # source will fail to compile. + if(EMSCRIPTEN AND ${I_SIMD} GREATER_EQUAL 2) + break() + endif() + list(GET WEBP_SIMD_FLAGS ${I_SIMD} WEBP_SIMD_FLAG) # First try with no extra flag added as the compiler might have default flags @@ -67,43 +80,33 @@ foreach(I_SIMD RANGE ${WEBP_SIMD_FLAGS_RANGE}) unset(WEBP_HAVE_${WEBP_SIMD_FLAG} CACHE) cmake_push_check_state() set(CMAKE_REQUIRED_FLAGS) - if(NOT simd_found) - webp_check_compiler_flag(${WEBP_SIMD_FLAG} ${WEBP_ENABLE_SIMD}) - if(NOT WEBP_HAVE_${WEBP_SIMD_FLAG}) - list(GET SIMD_ENABLE_FLAGS ${I_SIMD} SIMD_COMPILE_FLAG) - if(EMSCRIPTEN) - set(SIMD_COMPILE_FLAG "-msimd128 ${SIMD_COMPILE_FLAG}") - endif() - set(CMAKE_REQUIRED_FLAGS ${SIMD_COMPILE_FLAG}) - webp_check_compiler_flag(${WEBP_SIMD_FLAG} ${WEBP_ENABLE_SIMD}) - else() - if(MSVC) - list(GET SIMD_ENABLE_FLAGS ${I_SIMD} SIMD_COMPILE_FLAG) - else() - set(SIMD_COMPILE_FLAG " ") - endif() + webp_check_compiler_flag(${WEBP_SIMD_FLAG} ${WEBP_ENABLE_SIMD}) + if(NOT WEBP_HAVE_${WEBP_SIMD_FLAG}) + list(GET SIMD_ENABLE_FLAGS ${I_SIMD} SIMD_COMPILE_FLAG) + if(EMSCRIPTEN) + set(SIMD_COMPILE_FLAG "-msimd128 ${SIMD_COMPILE_FLAG}") + endif() + set(CMAKE_REQUIRED_FLAGS ${SIMD_COMPILE_FLAG}) + webp_check_compiler_flag(${WEBP_SIMD_FLAG} ${WEBP_ENABLE_SIMD}) + else() + if(MSVC AND SIMD_ENABLE_FLAGS) + # The detection for SSE2/SSE4 support under MSVC is based on the compiler + # version so e.g., clang-cl will require flags to enable the assembly. + list(GET SIMD_ENABLE_FLAGS ${I_SIMD} SIMD_COMPILE_FLAG) + else() + set(SIMD_COMPILE_FLAG " ") endif() - elseif(${I_SIMD} LESS 2) - set(WEBP_HAVE_${WEBP_SIMD_FLAG} 1) endif() # Check which files we should include or not. list(GET WEBP_SIMD_FILE_EXTENSIONS ${I_SIMD} WEBP_SIMD_FILE_EXTENSION) file(GLOB SIMD_FILES "${CMAKE_CURRENT_LIST_DIR}/../" "src/dsp/*${WEBP_SIMD_FILE_EXTENSION}") if(WEBP_HAVE_${WEBP_SIMD_FLAG}) - if(${I_SIMD} LESS 2 AND NOT HIGHEST_SSE_FLAG) - set(HIGHEST_SSE_FLAG ${SIMD_COMPILE_FLAG}) - endif() # Memorize the file and flags. foreach(FILE ${SIMD_FILES}) list(APPEND WEBP_SIMD_FILES_TO_INCLUDE ${FILE}) - if(${I_SIMD} LESS 2) - list(APPEND WEBP_SIMD_FLAGS_TO_INCLUDE ${HIGHEST_SSE_FLAG}) - else() - list(APPEND WEBP_SIMD_FLAGS_TO_INCLUDE ${SIMD_COMPILE_FLAG}) - endif() + list(APPEND WEBP_SIMD_FLAGS_TO_INCLUDE ${SIMD_COMPILE_FLAG}) endforeach() - set(simd_found 1) else() # Remove the file from the list. foreach(FILE ${SIMD_FILES})