ARM: define __ARM_VFPV5__ when present.

[t.p.northover]

Summary

We've had embedded developers requesting we extend the ARM_VFPVn series to support Cortex-M7, which sounds reasonable apart from the name bikeshedding.

The features added are equivalent to a v8 FPU and the LLVM codebase is pretty split on whether it's fp-armv8 or vfpv5. Since this is user-facing, I thought I'd ask for opinions. The obvious choices are:

• __ARM_VFPV5__ everywhere (including Cortex-A57 for example). This matches Cortex-M7 naming from ARM, but probably not Cortex-A57. It also matches our historical #defines.
• __ARM_FPV8__ (or similar) everywhere. Reverse problem from above.
• Both, depending on whether the CPU really is v8. No naming mismatch, but two #defines for what's essentially the same thing. It's difficult to imagine code actually wanting to distinguish the two.

I've got a mild preference for the first, hence this patch. Any objections or other suggestions?

Tim.

[rengolin]

I don't mind either way, but would be good to be compatible with gcc in that respect. Least surprise and all.

ARM was naming it fpv8 for the command line options and other things, might also be good to keep the consistency.

Would it be too bad to have both?

[richard.barton.arm]

Hi all

At first glance I thought this would be something that is in the ACLE, so was going to give an opinion. On second look, this is not actually an ACLE macro, and seems to be a clang special - my arm-none-eabi-gcc 5.4.2 does not emit it. I think it would be best to use the right names in all circumstances, although I appreciate the situation has not been made easy by ARM giving two names to the same thing. If that approach is not practical, then Tim's patch is probably ok.

The thinking behind not having this in the ACLE by the way is that ACLE now tries to define feature test macros per architectural feature rather than generationally. The one exception is the __ARM_ARCH macro and it would not be practical to describe all the changes between e.g. ARMv7 and ARMv8 via individual macros. Given that, the meaning of this macro can be expressed by combining the __ARM_ARCH, __ARM_FP and __ARM_FEATURE_FMA macros to pick between VFPv2,VFPv3,...,FP-ARMv8.

[rengolin]

Hi Tim,

This patch seems stale. I think neither me nor Richard have any strong feelings against this, so I'll just approve and let you decide on which solution is best. I have a mild preference for both.

cheers,
--renato

lib/Basic/Targets.cpp

void getTargetDefines(const LangOptions &Opts,
     if (FPUModeIsVFP((FPUMode) FPU)) {
       Builder.defineMacro("__VFP_FP__");
       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_VFPV5__");
     }
 
     // This only gets set when Neon instructions are actually available, unlike
     // the VFP define, hence the soft float and arch check. This is subtly
     // different from gcc, we follow the intent which was that it should be set
     // when Neon instructions are actually available.
     if ((FPU & NeonFPU) && !SoftFloat && ArchVersion >= 7) {
       Builder.defineMacro("__ARM_NEON", "1");

test/Preprocessor/arm-target-features.c

 // M4-THUMB:#define __ARM_FEATURE_DSP 1
 // M4-THUMB:#define __ARM_FP 0x6
 
 // Test whether predefines are as expected when targeting cortex-m7.
 // RUN: %clang -target armv7 -mthumb -mcpu=cortex-m7 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=M7-THUMB %s
 // M7-THUMB:#define __ARM_ARCH_EXT_IDIV__ 1
 // M7-THUMB:#define __ARM_FEATURE_DSP 1
 // M7-THUMB:#define __ARM_FP 0xE
+// M7-THUMB:#define __ARM_VFPV5__ 1
 
 // Test whether predefines are as expected when targeting krait.
 // RUN: %clang -target armv7 -mcpu=krait -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=KRAIT %s
 // RUN: %clang -target armv7 -mthumb -mcpu=krait -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=KRAIT %s
 // KRAIT:#define __ARM_ARCH_EXT_IDIV__ 1
 // KRAIT:#define __ARM_FEATURE_DSP 1
 // KRAIT:#define  __ARM_VFPV4__ 1