Skip to content

Commit

Permalink
[builtins] Only build float16/bfloat16 code if actually supported
Browse files Browse the repository at this point in the history
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: #57224
Differential Revision: https://reviews.llvm.org/D145237

(cherry picked from commit 489bda6)
  • Loading branch information
arichardson authored and tstellar committed Mar 9, 2023
1 parent c1949c6 commit e0ffaab
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 24 deletions.
16 changes: 0 additions & 16 deletions compiler-rt/cmake/builtin-config-ix.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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\");
Expand Down
26 changes: 18 additions & 8 deletions compiler-rt/lib/builtins/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ if (COMPILER_RT_STANDALONE_BUILD)
endif()

include(builtin-config-ix)
include(CMakePushCheckState)

if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
include(CompilerRTAIXUtils)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}")
Expand All @@ -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})
Expand Down Expand Up @@ -833,6 +842,7 @@ else ()
DEFS ${BUILTIN_DEFS}
CFLAGS ${BUILTIN_CFLAGS_${arch}}
PARENT_TARGET builtins)
cmake_pop_check_state()
endif ()
endforeach ()
endif ()
Expand Down

0 comments on commit e0ffaab

Please sign in to comment.