[PATCH] tools: add support for decoding ARM attributes

Saleem Abdulrasool compnerd at compnerd.org
Wed Jan 29 20:52:23 PST 2014



================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:275
@@ +274,3 @@
+                                         uint32_t &Offset) {
+  static const char *Strings[] = { "Unused", "IEEE-754", "Sign Only" };
+
----------------
Richard Barton wrote:
> 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.
No worries :-).  Changed to unsupported.

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:101
@@ +100,3 @@
+                                         uint32_t &Offset) {
+  StringRef TagName = ARMBuildAttrs::AttrTypeAsString(Tag, /*TagPrefix*/false);
+
----------------
Renato Golin wrote:
> Same name may introduce conflicts. Please rename the list below:
> 
> ​static const EnumEntry<unsigned> TagNames[]
Oh didnt even realise I did that!  That is indeed confusing, changed.

================
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"
----------------
Richard Barton wrote:
> Perhaps "8-byte data alignment" for value 1 here to disambiguate.
That does clearer, changed.

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


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



More information about the llvm-commits mailing list