[PATCH] tools: add support for decoding ARM attributes

Richard Barton richard.barton at arm.com
Mon Jan 27 11:25:25 PST 2014


  Hi Saleem

  A couple more nitpicks on the wording of some of the attributes. I'm happy to back down on using the full ABI descriptions for the attributes so long as the short descriptions are not ambiguous.

  I think this patch is fine to commit.

  Thanks,
  Rich


================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:275
@@ +274,3 @@
+                                         uint32_t &Offset) {
+  static const char *Strings[] = { "Unused", "IEEE-754", "Sign Only" };
+
----------------
I'm going to quibble over wording again:

"Unused" here does not mean the same as "Unused" above (line 217) The attribute value 0 means that denormals might not be supported. We can't use "Unknown" as it is used above to mean attribute value with no meaning (line 256), I think "Unsupported" would be better.

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:335
@@ +334,3 @@
+  static const char *Strings[] = {
+    "Not Required", "8-byte alignment", "8-byte data and code alignment",
+    "Reserved"
----------------
Perhaps "8-byte data alignment" for value 1 here to disambiguate.

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:413
@@ +412,3 @@
+    "None", "Speed", "Aggressive Speed", "Size", "Aggressive Size", "Accuracy",
+    "Aggressive Accuracy"
+  };
----------------
ABI has "Best Accuracy" here; "aggressive accuracy" sounds scary! ;-)


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



More information about the llvm-commits mailing list