From 8b57579fd8f336a344948ca28495964139f37ae9 Mon Sep 17 00:00:00 2001 From: Alexander Richardson Date: Thu, 28 Nov 2024 10:42:47 -0800 Subject: [PATCH 1/2] [compiler-rt] Fix detecting _Float16 support for secondary targets (#117813) It turns out we were not passing -m32 to the check_c_source_compiles() invocation since CMAKE_REQUIRE_FLAGS needs to be string separated list and we were passing a ;-separated CMake list which appears to be parsed by CMake as 'ignore all arguments beyond the first'. Fix this by transforming the list to a command line first. With this change, Clang 17 no longer claims to support _Float16 for i386. (cherry picked from commit a4c8ef0f401d86040594cc6f01bcdad9392e8ee2) --- compiler-rt/lib/builtins/CMakeLists.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler-rt/lib/builtins/CMakeLists.txt b/compiler-rt/lib/builtins/CMakeLists.txt index 49e463e97f9ac..aced1c3e6c54c 100644 --- a/compiler-rt/lib/builtins/CMakeLists.txt +++ b/compiler-rt/lib/builtins/CMakeLists.txt @@ -848,9 +848,12 @@ else () if (CAN_TARGET_${arch}) cmake_push_check_state() # TODO: we should probably make most of the checks in builtin-config depend on the target flags. - message(STATUS "Performing additional configure checks with target flags: ${TARGET_${arch}_CFLAGS}") set(BUILTIN_CFLAGS_${arch} ${BUILTIN_CFLAGS}) - list(APPEND CMAKE_REQUIRED_FLAGS ${TARGET_${arch}_CFLAGS} ${BUILTIN_CFLAGS_${arch}}) + # CMAKE_REQUIRED_FLAGS must be a space separated string but unlike TARGET_${arch}_CFLAGS, + # BUILTIN_CFLAGS_${arch} is a CMake list, so we have to join it to create a valid command line. + list(JOIN BUILTIN_CFLAGS " " CMAKE_REQUIRED_FLAGS) + set(CMAKE_REQUIRED_FLAGS "${TARGET_${arch}_CFLAGS} ${BUILTIN_CFLAGS_${arch}}") + message(STATUS "Performing additional configure checks with target flags: ${CMAKE_REQUIRED_FLAGS}") # For ARM archs, exclude any VFP builtins if VFP is not supported if (${arch} MATCHES "^(arm|armhf|armv7|armv7s|armv7k|armv7m|armv7em|armv8m.main|armv8.1m.main)$") string(REPLACE ";" " " _TARGET_${arch}_CFLAGS "${TARGET_${arch}_CFLAGS}") From e958952b650dcd4e36fc95b36177a9f2072e81de Mon Sep 17 00:00:00 2001 From: Evan Wilde Date: Fri, 4 Apr 2025 09:02:24 -0700 Subject: [PATCH 2/2] [compiler-rt][CMake] Pass all flags to _Float16 try-compile (#133952) The try-compile mechanism requires that `CMAKE_REQUIRED_FLAGS` is a space-separated string instead of a list of flags. The original code expanded `BUILTIN_FLAGS` into `CMAKE_REQUIRED_FLAGS` as a space-separated string and then would overwrite `CMAKE_REQUIRED_FLAGS` with `TARGET_${arch}_CFLAGS` prepended to the unexpanded `BUILTIN_CFLAGS_${arch}`. This resulted in the first two arguments being passed into the try-compile invocation, but dropping the other arguments listed in `BUILTIN_CFLAGS_${arch}`. This patch appends `TARGET_${arch}_CFLAGS` and `BUILTIN_CFLAGS_${arch}` to `CMAKE_REQUIRED_FLAGS` before expanding CMAKE_REQUIRED_FLAGS as a space-separated string. This passes any pre-set required flags, in addition to all of the builtin and target flags to the Float16 detection. (cherry picked from commit 0d3f5ec0da064d2314098644e78d29d3c84e179c) --- compiler-rt/lib/builtins/CMakeLists.txt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/compiler-rt/lib/builtins/CMakeLists.txt b/compiler-rt/lib/builtins/CMakeLists.txt index aced1c3e6c54c..df5455bc6e560 100644 --- a/compiler-rt/lib/builtins/CMakeLists.txt +++ b/compiler-rt/lib/builtins/CMakeLists.txt @@ -849,15 +849,17 @@ else () cmake_push_check_state() # TODO: we should probably make most of the checks in builtin-config depend on the target flags. set(BUILTIN_CFLAGS_${arch} ${BUILTIN_CFLAGS}) - # CMAKE_REQUIRED_FLAGS must be a space separated string but unlike TARGET_${arch}_CFLAGS, - # BUILTIN_CFLAGS_${arch} is a CMake list, so we have to join it to create a valid command line. - list(JOIN BUILTIN_CFLAGS " " CMAKE_REQUIRED_FLAGS) - set(CMAKE_REQUIRED_FLAGS "${TARGET_${arch}_CFLAGS} ${BUILTIN_CFLAGS_${arch}}") + # CMAKE_REQUIRED_FLAGS must be a space separated string + # Join BUILTIN_CFLAGS_${arch} and TARGET_${arch}_CFLAGS as a + # space-separated string. + list(APPEND CMAKE_REQUIRED_FLAGS + ${BUILTIN_CFLAGS_${arch}} + ${TARGET_${arch}_CFLAGS}) + list(JOIN CMAKE_REQUIRED_FLAGS " " CMAKE_REQUIRED_FLAGS) message(STATUS "Performing additional configure checks with target flags: ${CMAKE_REQUIRED_FLAGS}") # For ARM archs, exclude any VFP builtins if VFP is not supported if (${arch} MATCHES "^(arm|armhf|armv7|armv7s|armv7k|armv7m|armv7em|armv8m.main|armv8.1m.main)$") - string(REPLACE ";" " " _TARGET_${arch}_CFLAGS "${TARGET_${arch}_CFLAGS}") - check_compile_definition(__ARM_FP "${CMAKE_C_FLAGS} ${_TARGET_${arch}_CFLAGS}" COMPILER_RT_HAS_${arch}_VFP) + check_compile_definition(__ARM_FP "${CMAKE_C_FLAGS}" COMPILER_RT_HAS_${arch}_VFP) if(NOT COMPILER_RT_HAS_${arch}_VFP) list(REMOVE_ITEM ${arch}_SOURCES ${arm_Thumb1_VFPv2_DP_SOURCES} ${arm_Thumb1_VFPv2_SP_SOURCES} ${arm_Thumb1_SjLj_EH_SOURCES}) else()