[PATCH] D22272: ARM: define __ARM_VFPV5__ when present.

[Renato Golin via cfe-commits]

rengolin accepted this revision.
rengolin added a comment.
This revision is now accepted and ready to land.

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


https://reviews.llvm.org/D22272



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22272: ARM: define __ARM_VFPV5__ when present.

[Richard Barton via cfe-commits]

richard.barton.arm added a subscriber: richard.barton.arm.
richard.barton.arm added a comment.

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.


http://reviews.llvm.org/D22272



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22272: ARM: define __ARM_VFPV5__ when present.

[Renato Golin via cfe-commits]

rengolin added a reviewer: kristof.beyls.
rengolin added a comment.

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?


http://reviews.llvm.org/D22272



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22272: ARM: define __ARM_VFPV5__ when present.

[Tim Northover via cfe-commits]

t.p.northover created this revision.
t.p.northover added reviewers: rengolin, jmolloy.
t.p.northover added a subscriber: cfe-commits.
Herald added subscribers: mcrosier, rengolin, aemerson.

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.

http://reviews.llvm.org/D22272

Files:
  lib/Basic/Targets.cpp
  test/Preprocessor/arm-target-features.c

Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -380,6 +380,7 @@
 // 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
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5219,6 +5219,8 @@
 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


Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -380,6 +380,7 @@
 // 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
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5219,6 +5219,8 @@
 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits