[llvm-branch-commits] [compiler-rt] e0ffaab - [builtins] Only build float16/bfloat16 code if actually supported

[Tom Stellard via llvm-branch-commits]

Author: Alex Richardson
Date: 2023-03-09T06:34:51-08:00
New Revision: e0ffaabd2f75af7f96fea0878e587314777bae39

URL: https://github.com/llvm/llvm-project/commit/e0ffaabd2f75af7f96fea0878e587314777bae39
DIFF: https://github.com/llvm/llvm-project/commit/e0ffaabd2f75af7f96fea0878e587314777bae39.diff

LOG: [builtins] Only build float16/bfloat16 code if actually supported

When building compiler-rt builtins for x86_64 they library will by default
also be built for i386. We unconditionally add the Float16 compile flags
since the check for Float16 support will be done using x86_64 compiler
flags, but i386 does not actually support it. Fix this by moving the
COMPILER_RT_HAS_FLOAT16 and COMPILER_RT_HAS_FLOAT16 checks to a
per-target-architecture check inside the loop (using
`check_c_source_compiles` and `cmake_{push,pop}_check_state`).

Many of the checks in the builtin-config-ix file should probably also be
changed to per-target-arch checks, but so far only the Float16 one has
caused issues. This is an alternative to D136044 which added a special case
for i386 FreeBSD.

Fixes: https://github.com/llvm/llvm-project/issues/57224
Differential Revision: https://reviews.llvm.org/D145237

(cherry picked from commit 489bda6a9c0ec9d2644b7bb0c230294d38f7296e)

Added: 
    

Modified: 
    compiler-rt/cmake/builtin-config-ix.cmake
    compiler-rt/lib/builtins/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/compiler-rt/cmake/builtin-config-ix.cmake b/compiler-rt/cmake/builtin-config-ix.cmake
index 9f818499a076..e045c81a0f74 100644
--- a/compiler-rt/cmake/builtin-config-ix.cmake
+++ b/compiler-rt/cmake/builtin-config-ix.cmake
@@ -22,22 +22,6 @@ int foo(int x, int y) {
 }
 ")
 
-builtin_check_c_compiler_source(COMPILER_RT_HAS_FLOAT16
-"
-_Float16 foo(_Float16 x) {
- return x;
-}
-"
-)
-
-builtin_check_c_compiler_source(COMPILER_RT_HAS_BFLOAT16
-"
-__bf16 foo(__bf16 x) {
- return x;
-}
-"
-)
-
 builtin_check_c_compiler_source(COMPILER_RT_HAS_ASM_LSE
 "
 asm(\".arch armv8-a+lse\");

diff  --git a/compiler-rt/lib/builtins/CMakeLists.txt b/compiler-rt/lib/builtins/CMakeLists.txt
index 5e2274d52256..2fc70522895f 100644
--- a/compiler-rt/lib/builtins/CMakeLists.txt
+++ b/compiler-rt/lib/builtins/CMakeLists.txt
@@ -58,6 +58,7 @@ if (COMPILER_RT_STANDALONE_BUILD)
 endif()
 
 include(builtin-config-ix)
+include(CMakePushCheckState)
 
 if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
   include(CompilerRTAIXUtils)
@@ -190,14 +191,11 @@ set(GENERIC_SOURCES
   umodti3.c
 )
 
-# Build BF16 files only when "__bf16" is available.
-if(COMPILER_RT_HAS_BFLOAT16 AND NOT APPLE)
-  set(GENERIC_SOURCES
-    ${GENERIC_SOURCES}
+# We only build BF16 files when "__bf16" is available.
+set(BF16_SOURCES
     truncdfbf2.c
     truncsfbf2.c
-  )
-endif()
+)
 
 # TODO: Several "tf" files (and divtc3.c, but not multc3.c) are in
 # GENERIC_SOURCES instead of here.
@@ -747,8 +745,6 @@ else ()
     append_list_if(COMPILER_RT_ENABLE_CET -fcf-protection=full BUILTIN_CFLAGS)
   endif()
 
-  append_list_if(COMPILER_RT_HAS_FLOAT16 -DCOMPILER_RT_HAS_FLOAT16 BUILTIN_CFLAGS)
-
   append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 BUILTIN_CFLAGS)
 
   # These flags would normally be added to CMAKE_C_FLAGS by the llvm
@@ -780,7 +776,11 @@ else ()
 
   foreach (arch ${BUILTIN_SUPPORTED_ARCH})
     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}})
       # 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}")
@@ -799,6 +799,15 @@ else ()
           endif()
         endif()
       endif()
+      check_c_source_compiles("_Float16 foo(_Float16 x) { return x; }"
+                              COMPILER_RT_HAS_${arch}_FLOAT16)
+      append_list_if(COMPILER_RT_HAS_${arch}_FLOAT16 -DCOMPILER_RT_HAS_FLOAT16 BUILTIN_CFLAGS_${arch})
+      check_c_source_compiles("__bf16 foo(__bf16 x) { return x; }"
+                              COMPILER_RT_HAS_${arch}_BFLOAT16)
+      # Build BF16 files only when "__bf16" is available.
+      if(COMPILER_RT_HAS_${arch}_BFLOAT16)
+        list(APPEND ${arch}_SOURCES ${BF16_SOURCES})
+      endif()
 
       # Remove a generic C builtin when an arch-specific builtin is specified.
       filter_builtin_sources(${arch}_SOURCES ${arch})
@@ -833,6 +842,7 @@ else ()
                               DEFS ${BUILTIN_DEFS}
                               CFLAGS ${BUILTIN_CFLAGS_${arch}}
                               PARENT_TARGET builtins)
+      cmake_pop_check_state()
     endif ()
   endforeach ()
 endif ()