[compiler-rt][builtins] Skip building (b)float16 support on i386-freebsd

[dim]

Summary

Since bfloat16 and float16 support is not available for i386-freebsd,
the truncdfbf2.c and truncsfbf2.c builtin sources should be skipped
when targeting that platform, and COMPILER_RT_HAS_FLOAT16 should not
be defined.

However, the CMake configuration stage runs its tests with the default
target, which normally is amd64-freebsd, so it will detect both bfloat16
and float16 support.

Move adding of the COMPILER_RT_HAS_FLOAT16 define to the foreach()
loop where all the supported architectures are handled, and do not
enable it when targeting i386-freebsd.

Also remove the bfloat16 sources from the i386_SOURCES list, when
targeting i386-freebsd.

[dim]

@brooks btw didn't you encounter this problem during llvm-devel port builds ? E.g. it dies with:

In file included from /share/dim/src/llvm/release-15/compiler-rt/lib/builtins/truncsfbf2.c:11:
In file included from /share/dim/src/llvm/release-15/compiler-rt/lib/builtins/fp_trunc_impl.inc:39:
/share/dim/src/llvm/release-15/compiler-rt/lib/builtins/fp_trunc.h:63:9: error: __bf16 is not supported on this target
typedef __bf16 dst_t;
        ^

and similar errors for the float16 support.

[dim]

Another note to clarify: the problem is that the builtins are built *twice*, once for x86_64 and once for i386, the latter using the -m32 compiler flag. In the CMake configuration phase however, the feature detection is always done with the default compiler flags, i.e. without -m32, and therefore it will find features that aren't supported for 32-bit targets.

(I'm not sure how this problem could be solved in a more general manner, except by re-running the whole CMake configuration logic for each and every separate target architecture, using the compiler flags to enable that architecture. You would also have to have per-target config.h files, and the like...)

[arichardson]

What is being done for things like 64bit atomics and int128 support? Surely that will have the same problem?

It sounds to me like the configure check should be redone with -m32 instead of adding an os-specific workaround

[dim]

In D136044#3861187, @arichardson wrote:

What is being done for things like 64bit atomics and int128 support? Surely that will have the same problem?

These are simply done inline at compile time, similar to what I tried to do in rGce6d40f5c23923a807388c58b82b4c343eced0ce (but that was unfortunately reverted).

For 64 bit atomics, compiler-rt/lib/builtins/atomic.c has:

/// Macros for determining whether a size is lock free.
#define ATOMIC_ALWAYS_LOCK_FREE_OR_ALIGNED_LOCK_FREE(size, p)                  \
  (__atomic_always_lock_free(size, p) ||                                       \
   (__atomic_always_lock_free(size, 0) && ((uintptr_t)p % size) == 0))
#define IS_LOCK_FREE_1(p) ATOMIC_ALWAYS_LOCK_FREE_OR_ALIGNED_LOCK_FREE(1, p)
#define IS_LOCK_FREE_2(p) ATOMIC_ALWAYS_LOCK_FREE_OR_ALIGNED_LOCK_FREE(2, p)
#define IS_LOCK_FREE_4(p) ATOMIC_ALWAYS_LOCK_FREE_OR_ALIGNED_LOCK_FREE(4, p)
#define IS_LOCK_FREE_8(p) ATOMIC_ALWAYS_LOCK_FREE_OR_ALIGNED_LOCK_FREE(8, p)
#define IS_LOCK_FREE_16(p) ATOMIC_ALWAYS_LOCK_FREE_OR_ALIGNED_LOCK_FREE(16, p)

and then specific functions are enabled or not depending on the value of the various IS_LOCK_FREE macros.

For int128 support there is in compiler-rt/lib/builtins/int_types.h:

#if defined(__LP64__) || defined(__wasm__) || defined(__mips64) ||             \
    defined(__riscv) || defined(_WIN64)
#define CRT_HAS_128BIT
#endif

and all instances of int128 support are fenced by CRT_HAS_128BIT. (Something similar is done for 128 bit long doubles.)

It sounds to me like the configure check should be redone with -m32 instead of adding an os-specific workaround

I don't understand enough about compiler-rt's build system (and CMake in general) to do that.

[arichardson]

D114473 removed support for -m32 for the other runtimes - I feel like compiler-rt should probably do the same thing as well since fixing all of these assumptions that "-m32 supports the same features" would require prefixing every check.

[pengfei]

In D136044#3933375, @arichardson wrote:

D114473 removed support for -m32 for the other runtimes - I feel like compiler-rt should probably do the same thing as well since fixing all of these assumptions that "-m32 supports the same features" would require prefixing every check.

Sounds good.

[dim]

Btw these changes are independent of whether -m32 is used or not, they only check ${arch}. So if compiler-rt is ever changed to go over all support arches with e.g. -target x-y-z, this change would still work as-is. I'm mostly interested in getting the out-of-the-box build going on FreeBSD again, as it has been broken since 15.0.0.

Note also that the changes in D114473 seem to only have an effect on libc++, so they won't affect this change either.

If there are no more objections, I will go and and commit this now.

[dim]

Rebase onto main.

[dim]

Ugh, this is really a messy situation. COMPILER_RT_HAS_FLOAT16 (as a CMake feature) is also used in compiler-rt/test/builtins/CMakeLists.txt, where it is unconditionally used for each arch. I received some failures in buildbots, e.g.:

• https://lab.llvm.org/buildbot/#/builders/197/builds/3839
• https://lab.llvm.org/buildbot/#/builders/179/builds/5395
• https://lab.llvm.org/buildbot/#/builders/178/builds/3783
• https://lab.llvm.org/buildbot/#/builders/185/builds/2856

And these fail either with something similar to:

******************** TEST 'Builtins-aarch64-linux :: trunctfhf2_test.c' FAILED ********************
Script:
--
: 'RUN: at line 1';       /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/./bin/clang   -gline-tables-only   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta  -DCOMPILER_RT_HAS_FLOAT16  -fno-builtin -I /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/lib/builtins -nodefaultlibs /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/builtins/Unit/trunctfhf2_test.c /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/./lib/clang/16/lib/aarch64-unknown-linux-gnu/libclang_rt.builtins.a -lc -lm -o /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/projects/compiler-rt/test/builtins/Unit/AARCH64LinuxConfig/Output/trunctfhf2_test.c.tmp &&  /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/projects/compiler-rt/test/builtins/Unit/AARCH64LinuxConfig/Output/trunctfhf2_test.c.tmp
--
Exit Code: 1

Command Output (stderr):
--
/usr/bin/ld: /usr/bin/ld: DWARF error: invalid or unhandled FORM value: 0x25
/tmp/lit-tmp-o2gvd3a7/trunctfhf2_test-59b745.o: in function `test__trunctfhf2':
trunctfhf2_test.c:(.text+0x18): undefined reference to `__trunctfhf2'
clang-16: error: linker command failed with exit code 1 (use -v to see invocation)

--

********************

or with an unexpected value:

******************** TEST 'Builtins-armhf-linux :: truncdfhf2_test.c' FAILED ********************
Script:
--
: 'RUN: at line 1';       C:/buildbot/as-builder-1/x-armv7l/build/./bin/clang.exe   -gline-tables-only -gcodeview -gcolumn-info   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta  -fomit-frame-pointer -DCOMPILER_RT_ARMHF_TARGET -DCOMPILER_RT_HAS_FLOAT16  -fno-builtin -I C:/buildbot/as-builder-1/x-armv7l/llvm-project/compiler-rt\lib\builtins -nodefaultlibs C:\buildbot\as-builder-1\x-armv7l\llvm-project\compiler-rt\test\builtins\Unit\truncdfhf2_test.c C:/buildbot/as-builder-1/x-armv7l/build/./lib/clang/16/lib/armv7-unknown-linux-gnueabihf\libclang_rt.builtins.a -lc -lm -o C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\compiler-rt\test\builtins\Unit\ARMHFLinuxConfig\Output\truncdfhf2_test.c.tmp && "C:/Python310/python.exe" "C:/buildbot/as-builder-1/x-armv7l/llvm-project/llvm/utils/remote-exec.py" --host=[e-mail] C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\compiler-rt\test\builtins\Unit\ARMHFLinuxConfig\Output\truncdfhf2_test.c.tmp
--
Exit Code: 1
Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "C:/buildbot/as-builder-1/x-armv7l/build/./bin/clang.exe" "-gline-tables-only" "-gcodeview" "-gcolumn-info" "-Wthread-safety" "-Wthread-safety-reference" "-Wthread-safety-beta" "-fomit-frame-pointer" "-DCOMPILER_RT_ARMHF_TARGET" "-DCOMPILER_RT_HAS_FLOAT16" "-fno-builtin" "-I" "C:/buildbot/as-builder-1/x-armv7l/llvm-project/compiler-rt\lib\builtins" "-nodefaultlibs" "C:\buildbot\as-builder-1\x-armv7l\llvm-project\compiler-rt\test\builtins\Unit\truncdfhf2_test.c" "C:/buildbot/as-builder-1/x-armv7l/build/./lib/clang/16/lib/armv7-unknown-linux-gnueabihf\libclang_rt.builtins.a" "-lc" "-lm" "-o" "C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\compiler-rt\test\builtins\Unit\ARMHFLinuxConfig\Output\truncdfhf2_test.c.tmp"
$ "C:/Python310/python.exe" "C:/buildbot/as-builder-1/x-armv7l/llvm-project/llvm/utils/remote-exec.py" "--host=[e-mail]" "C:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-armv7-unknown-linux-gnueabihf-bins\compiler-rt\test\builtins\Unit\ARMHFLinuxConfig\Output\truncdfhf2_test.c.tmp"
# command output:
error in test__truncdfhf2(nan) = 0000, expected 0x7e00
error: command failed with exit status: 1
--
********************

Unfortunately I cannot reproduce them. I tried with a Ubuntu 20.04 on aarch64 VM, but it succeeds with all tests. I have no idea what is different in the buildbot configuration.

If anybody has any clue about this, please let me know. Otherwise I'll probably put yet another kludge on top of the existing kludge, to avoid changing behavior for any platform other than FreeBSD.

[pengfei]

compiler-rt/lib/builtins/CMakeLists.txt

804

Should move the removal of GENERIC_BF_SOURCES in this loop like this (Just wild guess. I'm neither familiar with compiler-rt nor cmake)

[arichardson]

I've managed to reproduce this error on Linux, it turns out the real problem is that these checks are being done once even though the library is built multiple times with differing flags. I think the only way to fix this issue is to actually start using ExternalProject_Add (or adding an ARCH prefix for all the checks).

cmake -GNinja -S compiler-rt/lib/builtins -B cmake-build-builtins -DLLVM_CONFIG_PATH=NOTFOUND -DCMAKE_DISABLE_FIND_PACKAGE_LLVM=TRUE && ninja -C cmake-build-builtins

[arichardson]

I believe D145237 fixes the issue.

compiler-rt/lib/builtins/CMakeLists.txt

set(GENERIC_SOURCES
   udivti3.c
   umoddi3.c
   umodsi3.c
   umodti3.c
 )
 
 # Build BF16 files only when "__bf16" is available.
 if(COMPILER_RT_HAS_BFLOAT16 AND NOT APPLE)
-  set(GENERIC_SOURCES
-    ${GENERIC_SOURCES}
+  set(GENERIC_BF_SOURCES
     truncdfbf2.c
     truncsfbf2.c
   )
+  set(GENERIC_SOURCES
+    ${GENERIC_SOURCES}
+    ${GENERIC_BF_SOURCES}
+  )
 endif()
 
 # TODO: Several "tf" files (and divtc3.c, but not multc3.c) are in
 # GENERIC_SOURCES instead of here.
 set(GENERIC_TF_SOURCES
   addtf3.c
   comparetf2.c
   divtc3.c
if (NOT MSVC)
 
   if (WIN32)
     set(i386_SOURCES
       ${i386_SOURCES}
       i386/chkstk.S
       i386/chkstk2.S
     )
   endif()
+
+  if(${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")
+    # Do not build bfloat16 sources i386-freebsd, as it is not supported.
+    list(REMOVE_ITEM i386_SOURCES ${GENERIC_BF_SOURCES})
+  endif()
 else () # MSVC
   # Use C versions of functions when building on MSVC
   # MSVC's assembler takes Intel syntax, not AT&T syntax.
   # Also use only MSVC compilable builtin implementations.
   set(x86_64_SOURCES
     ${GENERIC_SOURCES}
     ${x86_ARCH_SOURCES}
     x86_64/floatdidf.c

 else ()
   set(BUILTIN_CFLAGS "")
   add_security_warnings(BUILTIN_CFLAGS 0)
 
   if (COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
     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
   # cmake step. Add them manually if this is a standalone build.
   if(COMPILER_RT_STANDALONE_BUILD)
     if(COMPILER_RT_BUILTINS_ENABLE_PIC)
       append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC BUILTIN_CFLAGS)
     endif()
if (CAN_TARGET_${arch})
         else()
           # Exclude any double-precision builtins if VFP is single-precision-only
           try_compile_only(COMPILER_RT_HAS_${arch}_VFP_DP
                            SOURCE "#if !(__ARM_FP & 0x8)
                                    #error No double-precision support!
                                    #endif
                                    int main(void) { return 0; }")
           if(NOT COMPILER_RT_HAS_${arch}_VFP_DP)
             list(REMOVE_ITEM ${arch}_SOURCES ${arm_Thumb1_VFPv2_DP_SOURCES})
           endif()
         endif()
       endif()
 
       # Remove a generic C builtin when an arch-specific builtin is specified.
       filter_builtin_sources(${arch}_SOURCES ${arch})
 
       # Needed for clear_cache on debug mode, due to r7's usage in inline asm.
if (CAN_TARGET_${arch})
           lse_builtin_symlinks
           BYPRODUCTS ${lse_builtins}
           ${arm64_lse_commands}
         )
 
         set(deps_aarch64 lse_builtin_symlinks)
       endif()
 
+      # Do not enable float16 on i386-freebsd, as it is not supported.
+      if(NOT (${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD" AND ${arch} STREQUAL "i386"))
+        append_list_if(COMPILER_RT_HAS_FLOAT16 -DCOMPILER_RT_HAS_FLOAT16 BUILTIN_CFLAGS)
+      endif()
+
       add_compiler_rt_runtime(clang_rt.builtins
                               STATIC
                               ARCHS ${arch}
                               DEPS ${deps_${arch}}
                               SOURCES ${${arch}_SOURCES}
                               DEFS ${BUILTIN_DEFS}
                               CFLAGS ${BUILTIN_CFLAGS_${arch}}
                               PARENT_TARGET builtins)