[Clang][ARM] Define __VFP_FP__ macro unconditionally

[vhscampos]

Summary

Clang only defines VFP_FP when the FPU is enabled. However, gcc
defines it unconditionally.

This patch aligns Clang with gcc.

[peter.smith]

I think this is the right thing to do. GCC changed to unconditionally set the macro with https://gcc.gnu.org/legacy-ml/gcc-patches/2016-10/msg01025.html my understanding is that the macro means that floating point representation uses the VFP format and not the FPA format (predecessor to VFP, not supported by clang, found in very old systems), it does not mean that there is a VFP unit present.

see https://wiki.debian.org/ArmEabiPort for the mutually exclusive __MAVERICK__ whcih refers to https://wiki.debian.org/ArmEabiMaverickCrunch the processor that had the FPA FPU.

I've set LGTM, and added a few more non-Arm people involved in the area for visibility, will be good to give them a chance to comment before committing.

clang/lib/Basic/Targets/ARM.cpp

758

Worth a comment like "VFP_FP means that the floating point format is VFP, not that a hardware FPU is present. The VFP format is the only one supported by clang."

[vhscampos]

Add a clarifying comment.

[vhscampos]

Thanks Peter. Since one week has passed, I plan to commit these changes by the end of the day if nothing surfaces.

[rengolin]

It's a weird flag, for sure, but if that's the semantics of it, than this change LGTM. Thanks!

clang/lib/Basic/Targets/ARM.cpp

if (((HWDiv & HWDivThumb) && isThumb()) ||
       ((HWDiv & HWDivARM) && !isThumb())) {
     Builder.defineMacro("__ARM_FEATURE_IDIV", "1");
     Builder.defineMacro("__ARM_ARCH_EXT_IDIV__", "1");
   }
 
   // Note, this is always on in gcc, even though it doesn't make sense.
   Builder.defineMacro("__APCS_32__");
 
-  if (FPUModeIsVFP((FPUMode)FPU)) {
-    Builder.defineMacro("__VFP_FP__");
+  // __VFP_FP__ means that the floating-point format is VFP, not that a hardware
+  // FPU is present. Moreover, the VFP format is the only one supported by
+  // clang. For these reasons, this macro is always defined.
+
+  if (FPUModeIsVFP((FPUMode)FPU)) {
     if (FPU & VFP2FPU)
       Builder.defineMacro("__ARM_VFPV2__");
     if (FPU & VFP3FPU)
       Builder.defineMacro("__ARM_VFPV3__");
     if (FPU & VFP4FPU)
       Builder.defineMacro("__ARM_VFPV4__");
     if (FPU & FPARMV8)
       Builder.defineMacro("__ARM_FPV5__");

clang/test/Preprocessor/arm-target-features.c

 // CHECK-V7S: #define __ARMEL__ 1
 // CHECK-V7S: #define __ARM_ARCH 7
 // CHECK-V7S: #define __ARM_ARCH_7S__ 1
 // CHECK-V7S-NOT: __ARM_FEATURE_CRC32
 // CHECK-V7S-NOT: __ARM_FEATURE_NUMERIC_MAXMIN
 // CHECK-V7S-NOT: __ARM_FEATURE_DIRECTED_ROUNDING
 // CHECK-V7S: #define __ARM_FP 0xe
 
+// RUN: %clang -target arm-arm-none-eabi -march=armv7-m -mfloat-abi=soft -x c -E -dM %s | FileCheck -match-full-lines --check-prefix=CHECK-VFP-FP %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv7-m -mfloat-abi=softfp -x c -E -dM %s | FileCheck -match-full-lines --check-prefix=CHECK-VFP-FP %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv7-m -mfloat-abi=hard -x c -E -dM %s | FileCheck -match-full-lines --check-prefix=CHECK-VFP-FP %s
+// CHECK-VFP-FP: #define __VFP_FP__ 1
+
 // RUN: %clang -target armv8a -mfloat-abi=hard -x c -E -dM %s | FileCheck -match-full-lines --check-prefix=CHECK-V8-BAREHF %s
 // CHECK-V8-BAREHF: #define __ARMEL__ 1
 // CHECK-V8-BAREHF: #define __ARM_ARCH 8
 // CHECK-V8-BAREHF: #define __ARM_ARCH_8A__ 1
 // CHECK-V8-BAREHF: #define __ARM_FEATURE_CRC32 1
 // CHECK-V8-BAREHF: #define __ARM_FEATURE_DIRECTED_ROUNDING 1
 // CHECK-V8-BAREHF: #define __ARM_FEATURE_NUMERIC_MAXMIN 1
 // CHECK-V8-BAREHP: #define __ARM_FP 0xe