[PATCH] [ARM] Implement eabi_attribute, cpu, and fpu directives.
Logan Chien
tzuhsiang.chien at gmail.com
Fri Oct 25 10:07:28 PDT 2013
================
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
----------------
Richard Barton wrote:
> 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?
>
>
I am removing most of the Tag_Advanced_SIMD_arch. However, the following case can't be mapped to a single .fpu directive.
; RUN: llc < %s -mtriple=armv8-linux-gnueabi -mattr=-fp-armv8,-crypto | FileCheck %s --check-prefix=V8-NEON
That's why you still can see a special case in ARMAsmPrinter.cpp (Line:663)
================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:663
@@ -870,6 +662,3 @@
if (Subtarget->hasV8Ops())
- AttrEmitter->EmitAttribute(ARMBuildAttrs::Advanced_SIMD_arch,
- ARMBuildAttrs::AllowedNeonV8);
- else
- AttrEmitter->EmitAttribute(ARMBuildAttrs::Advanced_SIMD_arch,
- ARMBuildAttrs::Allowed);
+ ATS.emitAttribute(ARMBuildAttrs::Advanced_SIMD_arch,
+ ARMBuildAttrs::AllowNeonARMv8);
----------------
The special case which we keep emitting Tag_Advanced_SIMD_arch.
================
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.
;
----------------
Richard Barton wrote:
> typo in comment
Thanks. Will be fixed in next revision.
================
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 \
----------------
Richard Barton wrote:
> This is a duplicate of line 9 above.
Thanks. Will be removed. I haven't notice this when I was copy the test command from 2010-09-29-mc-asm-header-test.ll.
================
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 \
----------------
Richard Barton wrote:
> 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.
Thanks. Test for Cortex-A15 will be added in next revision of this patch.
================
Comment at: test/CodeGen/ARM/2010-10-19-mc-elf-objheader.ll:31
@@ -7,2 +30,3 @@
+; RUN: | FileCheck %s --check-prefix=CORTEX-M0
----------------
Richard Barton wrote:
> 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.
Both cortex-m4 and cortex-a15 will be added to next revision of this patch. However, it seems that LLVM can't recognize cortex-r4 as the mcpu option. I will use cortex-r5 instead (p.s. According to the result, it seems that R5 is using VFPv3_D16 as well)
================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:669
@@ +668,3 @@
+ else if (Subtarget->hasVFP4())
+ ATS.emitFPU(Subtarget->isFPOnlySP() ? ARM::VFPV4_D16 : ARM::VFPV4);
+ else if (Subtarget->hasVFP3())
----------------
After further testing, it seems that we have to use Subtarget->hasD16() instead. However, I am not sure why was the old code using Subtarget->isFPOnlySP(). I will change this in next revision. Any thoughts?
http://llvm-reviews.chandlerc.com/D1987
More information about the llvm-commits
mailing list