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

Renato Golin renato.golin at linaro.org
Tue Oct 22 05:32:46 PDT 2013


  Overall, it looks like a good move. Apart from my other comments, I think it's good. I'd love to get Richard Barton to review this code, as he's an ace in spotting architecture mismatches. I'll include him in the reviewers' list.


================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:125
@@ +124,3 @@
+void ARMTargetAsmStreamer::emitTextAttribute(unsigned Attribute,
+                                             StringRef String) {
+  switch (Attribute) {
----------------
A better name for this would be CPU

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:166
@@ +165,3 @@
+      Value >>= 7;
+      Size += sizeof(int8_t); // Is this really necessary?
+    } while (Value);
----------------
I don't think so.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:472
@@ +471,3 @@
+  setAttributeItem(Attribute, Value);
+}
+void ARMTargetELFStreamer::emitFPU(StringRef FPU) {
----------------
These two functions look redundant. Is it just in ARM's case?

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:474
@@ +473,3 @@
+void ARMTargetELFStreamer::emitFPU(StringRef FPU) {
+  if (FPU == "vfp" || FPU == "vfpv2") {
+    setAttributeItem(ARMBuildAttrs::VFP_arch,
----------------
It'd be really good to get rid of string parsing at this level...


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



More information about the llvm-commits mailing list