[PATCH] tools: add support for decoding ARM attributes

[Richard Barton]

  Hi Saleem

  My main suggestion would be that the reader uses the verbatim description strings from the ABI addenda document. The strings have been designed precisely to match user intentions so using the exact wording would eliminate confusion.

  I'm also concerned that we have information about ARM Build attributes in three separate places in LLVM for the integrated assembler, the MC assembler and now for llvm-readelf. It would be great to be able to lift this code out into one place.


================
Comment at: test/tools/llvm-readobj/ARM/attributes.s:72
@@ +71,3 @@
+@ CHECK:       Tag_ABI_FP_user_exceptions: IEEE-754
+@ CHECK:       Tag_ABI_FP_number_model: IEEE-754
+@ CHECK:       Tag_ABI_align_needed: 8-byte alignment
----------------
Agree with Amara's comment here - It is vital to see the attribute value as well as the description.

It would be even better to see the verbatim description from the ABI addenda here too.

================
Comment at: test/tools/llvm-readobj/ARM/attributes.s:55
@@ +54,3 @@
+@ CHECK:       Tag_CPU_name: CORTEX-A9
+@ CHECK:       Tag_CPU_arch: v7
+@ CHECK:       Tag_CPU_arch_profile: Application
----------------
In the ABI document these are represented with a preceeding "ARM"

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:117
@@ +116,3 @@
+  case 'M': Profile = "Microcontroller"; break;
+  case 'S': Profile = "System"; break;
+  case '0': Profile = "None"; break;
----------------
Renato Golin wrote:
> This is more like A/R than System, and the ABI describes it as "classic programmer's model". Not sure what short description to use here... system may be ok, though. Richard?
Agreed - I think the strings from the ABI document would be the most suitable thing to have here. "System" is an extension on the ARMv6M architecture and is the 'S' part in v6S-M and does not mean the same thing as it does in v7-S unfortunately.

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:321
@@ +320,3 @@
+                           uint32_t &Offset) {
+  static const char *Strings[] = { "Unused", "Small", "int", "forced to int" };
+  uint64_t Value = ParseInteger(Data, Offset);
----------------
Amara Emerson wrote:
> Renato Golin wrote:
> > This is more like: { "Forbidden", "Pack", "Int32", "Visible Int32" }
> Yep, although Rich should clarify.
I think it would be best for the ABI descriptions to be used here.

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:331
@@ +330,3 @@
+  static const char *Strings[] = {
+    "Tag_FP_arch", "Single-Precision", "Reserved", "Tag_FP_arch (deprecated)"
+  };
----------------
Amara Emerson wrote:
> Renato Golin wrote:
> > My copy of the ABI might be a bit old, but it says:
> > 
> > Tag_ABI_HardFP_use, (=27), uleb128
> > 0 The user intended that VFP use should be implied by Tag_FP_arch
> > 1 The user permitted this entity to use only SP VFP instructions
> > 2 The user permitted this entity to use only DP VFP instructions
> > 3 The user permitted this entity to use both SP and DP VFP instructions
> > (Note: This is effectively an explicit version of the default encoded by 0)
> > 
> > Richard, did that change with newer releases of the document?
> Yes, r2.09 changed this.
The latest ABI documents are available online at http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ihi0045d/index.html

================
Comment at: tools/llvm-readobj/ARMAttributeParser.cpp:428
@@ +427,3 @@
+  static const char *Strings[] = {
+    "If Available", "Not Permitted", "Permitted"
+  };
----------------
Renato Golin wrote:
> This is more like { "Thumb-v7R/M", "Not Permitted", "v7A" }
Nope - the original patch is more accurate to the ABI. Again, using the official wording would remove all confusion.


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