[PATCH] [ARM] Implement eabi_attribute, cpu, and fpu directives.

[Richard Barton]

  Hi Logan

  I have a few reservations about the behaviour of the .fpu vs .eabi directives generation which are in the comments.

  Also would like to see a few more tests added, just to get full coverage. If you can show these cores/architecture configurations are covered by other testing elsewhere then that would be fine too.

  Otherwise, looks great. Thanks for the patch.

  Rich


================
Comment at: test/CodeGen/ARM/2010-09-29-mc-asm-header-test.ll:81
@@ -80,3 +78,2 @@
-; V8-FPARMv8-NEON: .eabi_attribute 10, 7
 ; V8-FPARMv8-NEON: .eabi_attribute 12, 3
 
----------------
I understand that we are choosing not to emit the .eabi_attribute directive for Tag_FP_arch in favour of the .fpu directive because that's what GNU does, but we are also emitting the .eabi_attribute directive for Tag_Advanced_SIMD_arch at the same time. Is this just to be compatible with GNU as well?



================
Comment at: test/CodeGen/ARM/2010-09-29-mc-asm-header-test.ll:96
@@ -95,3 +91,1 @@
-; CORTEX-A9:  .eabi_attribute 10, 3
-; CORTEX-A9:  .eabi_attribute 12, 1
 ; CORTEX-A9:  .eabi_attribute 20, 1
----------------
This is inconsistent with above when the .eabi_attribute directive for Tag_Advanced_SIMD_arch was left in. Which is right? Or is this just being compatible with GNU again?

================
Comment at: test/CodeGen/ARM/2010-10-19-mc-elf-objheader.ll:33
@@ -8,3 +32,3 @@
 
 ; This tests that the extpected ARM attributes are emitted.
 ;
----------------
typo in comment

================
Comment at: test/CodeGen/ARM/2010-10-19-mc-elf-objheader.ll:22
@@ +21,3 @@
+; RUN:   | llvm-readobj -s -sd | FileCheck %s --check-prefix=V8-FPARMv8-NEON
+; RUN: llc < %s -mtriple=armv8-linux-gnueabi -filetype=obj \
+; RUN:   | llvm-readobj -s -sd \
----------------
This is a duplicate of line 9 above.

================
Comment at: test/CodeGen/ARM/2010-10-19-mc-elf-objheader.ll:25
@@ +24,3 @@
+; RUN:   | FileCheck %s --check-prefix=V8-FPARMv8-NEON-CRYPTO
+; RUN: llc < %s -mtriple=armv7-linux-gnueabi -mcpu=cortex-a9 -filetype=obj \
+; RUN:   | llvm-readobj -s -sd \
----------------
This test generates identical attributes to line 7 above. It would be nice to see a test for a v7 core with VFPv4, so Cortex-A15.

================
Comment at: test/CodeGen/ARM/2010-10-19-mc-elf-objheader.ll:31
@@ -7,2 +30,3 @@
+; RUN:   | FileCheck %s --check-prefix=CORTEX-M0
 
 
----------------
It would be nice to get full coverage of the FPU options that you have touched, so:
 * R4 - VFPv3_D16
 * M4F AKA M4 - VFPv4_D16 (and also v7E-M)
 * A15 - VFPv4+NEONv2 (and also without neon)

For the half-precision only variants, there is another build attribute that needs setting: Tag_VFP_HP_extension, but I accept that is beyond the original scope of this patch, so the test strings are ok without it for now.


http://llvm-reviews.chandlerc.com/D1987