[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