[cmake] [ARM] Exclude any VFP builtins if VFP is not supported

[azharudd]

Summary

rL325492 disables FPU features when using soft floating point
(-mfloat-abi=soft), which is used internally when building for arm. This causes
errors with builtins that utililize VFP instructions.

With this change we check if VFP is enabled (by checking if the preprocessor
macro VFP_FP is defined), and exclude such builtins if it is not enabled.

[peter.smith]

I think that this is a sensible thing to do and it also helps out when trying to build compiler-rt for armv7-m as compiler-rt currently assumes that you have a target with a VFP when many armv7-m devices do not.

lib/builtins/CMakeLists.txt

404

Would it be worth reordering so that we don't stat the test with a NOT. For example:

if (COMPILER_RT_HAS_ARM_VFP)
  set(arm_Thumb1_SOURCES
    ${arm_Thumb1_JT_SOURCES}
    ${arm_Thumb1_SjLj_EH_SOURCES}
    ${arm_Thumb1_VFPv2_SOURCES}
    ${arm_Thumb1_icache_SOURCES})
else()
  set(arm_Thumb1_SOURCES
  ${arm_Thumb1_JT_SOURCES}
  ${arm_Thumb1_icache_SOURCES})
endif()

[peter.smith]

Thanks for the update. That looks good to me.

[azharudd]

@peter.smith Thanks for the review.

[azharudd]

I had to revert this last time as it was causing the armhf buildbot to fail. This should take care of that.

[peter.smith]

Can you elaborate on why the armhf buildbot failed and why you have excluded armhf from the MATCHES expression above. A simple test of a source file containing

__VFP_FP__

clang --target=arm-linux-gnueabihf -E vfp.c gives me

# 1 "vfp.c"
# 1 "<built-in>" 1
# 1 "<built-in>" 3
# 361 "<built-in>" 3
# 1 "<command line>" 1
# 1 "<built-in>" 2
# 1 "vfp.c" 2
1

Unless I'm missing something a target of arm-linux-gnueabihf would define the macro unless the fpu were disabled with -mfpu=none so it should come through the test with the right answer.

[azharudd]

In D47217#1165279, @peter.smith wrote:

Can you elaborate on why the armhf buildbot failed and why you have excluded armhf from the MATCHES expression above. A simple test of a source file containing

__VFP_FP__

clang --target=arm-linux-gnueabihf -E vfp.c gives me

# 1 "vfp.c"
# 1 "<built-in>" 1
# 1 "<built-in>" 3
# 361 "<built-in>" 3
# 1 "<command line>" 1
# 1 "<built-in>" 2
# 1 "vfp.c" 2
1

Unless I'm missing something a target of arm-linux-gnueabihf would define the macro unless the fpu were disabled with -mfpu=none so it should come through the test with the right answer.

Regarding the buildbot failure: the "armhf" sources list is populated from the "arm" sources list, and is the same as that. In the previous change, the VFP builtin sources were being excluded from the "arm" sources list earlier, resulting in them being excluded from the following "armhf" sources list as well. This was causing the tests related to those VFP sources to fail on the "armhf" builder.

why you have excluded armhf from the MATCHES expression above

It shouldn't actually be excluded. When writing that if condition I assumed the VFP sources can always be included for armhf, which is wrong, as yes one can still disable fpu through -mfpu=none as part of the flags. I'll update the patch.

[azharudd]

Updated to check if VFP is enabled for armhf too.

[azharudd]

Ping for review please.

[peter.smith]

Apologies for the delay. This looks good to me. I've checked that I can build v7-m without floating point support and I've checked that I can build and run the tests for armv7-linux-gnueabihf.

[azharudd]

@peter.smith Thanks for the review.

lib/builtins/CMakeLists.txt

set(arm_Thumb1_icache_SOURCES
   arm/sync_synchronize.S)
 set(arm_Thumb1_SOURCES
   ${arm_Thumb1_JT_SOURCES}
   ${arm_Thumb1_SjLj_EH_SOURCES}
   ${arm_Thumb1_VFPv2_SOURCES}
   ${arm_Thumb1_icache_SOURCES})
 
 if(MINGW)
   set(arm_SOURCES
       arm/aeabi_idivmod.S
       arm/aeabi_ldivmod.S
       arm/aeabi_uidivmod.S
       arm/aeabi_uldivmod.S
       divmoddi4.c
       divmodsi4.c
       divdi3.c
       divsi3.c
if (CAN_TARGET_${arch})
       # we catch them all.
       set(_arch ${arch})
       if("${arch}" STREQUAL "armv6m")
         set(_arch "arm|armv6m")
       elseif("${arch}" MATCHES "^(armhf|armv7|armv7s|armv7k|armv7m|armv7em)$")
         set(_arch "arm")
       endif()
 
+      # For ARM archs, exclude any VFP builtins if VFP is not supported
+      if (${arch} MATCHES "^(arm|armhf|armv7|armv7s|armv7k|armv7m|armv7em)$")
+        string(REPLACE ";" " " _TARGET_${arch}_CFLAGS "${TARGET_${arch}_CFLAGS}")
+        check_compile_definition(__VFP_FP__ "${CMAKE_C_FLAGS} ${_TARGET_${arch}_CFLAGS}" COMPILER_RT_HAS_${arch}_VFP)
+        if(NOT COMPILER_RT_HAS_${arch}_VFP)
+          list(REMOVE_ITEM ${arch}_SOURCES ${arm_Thumb1_VFPv2_SOURCES} ${arm_Thumb1_SjLj_EH_SOURCES})
+        endif()
+      endif()
+
       # Filter out generic versions of routines that are re-implemented in
       # architecture specific manner.  This prevents multiple definitions of the
       # same symbols, making the symbol selection non-deterministic.
       foreach (_file ${${arch}_SOURCES})
         if (${_file} MATCHES ${_arch}/*)
           get_filename_component(_name ${_file} NAME)
           string(REPLACE ".S" ".c" _cname "${_name}")
           list(REMOVE_ITEM ${arch}_SOURCES ${_cname})