[PATCH] [arm] Parse tag names for eabi_attribute directive.
Logan Chien
tzuhsiang.chien at gmail.com
Wed Dec 18 09:49:42 PST 2013
Thanks. These comments will be addressed in next diff.
================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8026
@@ -8025,3 +8025,3 @@
/// parseDirectiveEabiAttr
/// ::= .eabi_attribute int, int
bool ARMAsmParser::parseDirectiveEabiAttr(SMLoc L) {
----------------
Richard Barton wrote:
> Comment is out of date now.
Done.
================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8074
@@ +8073,3 @@
+ CASE_EABI_ATTR_TAG(nodefaults)
+ CASE_EABI_ATTR_TAG(conformance)
+ CASE_EABI_ATTR_TAG(also_compatible_with)
----------------
Richard Barton wrote:
> Conformance should come after T2EE
Done.
================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8079
@@ +8078,3 @@
+#undef CASE_EABI_ATTR_TAG
+ .Case("Tag_FP_arch", ARMBuildAttrs::VFP_arch)
+ .Default(-1);
----------------
Richard Barton wrote:
> Tag_FP_arch is the new name for this attribute. I would prefer the enum member be renamed, and Tag_VFP_arch be the synonym.
Done.
================
Comment at: test/MC/ARM/directive-eabi_attribute-str.s:17
@@ +16,3 @@
+
+ .eabi_attribute Tag_CPU_arch, 0
+@ CHECK: .eabi_attribute 6, 0
----------------
Richard Barton wrote:
> I think this test would be more meaningful if we could use some real values in for the attribute. An AEABI build attribute with value 0 is normally semantically the same as not being present at all, so a valid implementation of the llvm-mc disassembly would be not to emit any .eabi_attribute at all. I suggest adding a meaningful value for each of the numeric ones as you have for the ones that take strings.
OK. I have changed most of them to non-zero value except Tag_ABI_HardFP_use and Tag_nodefaults.
http://llvm-reviews.chandlerc.com/D2424
More information about the llvm-commits
mailing list