[PATCH] D28281: [ARM] Enable objdump to construct a useful triple for ARM

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D28281#642418, @samparker wrote:

> Thanks for the review. For attribute printing and parsing, I was hoping that the current readobj tests would be sufficient since hopefully I have not changed the functionality of that tool.


That is true, but now the library lives in LLVM, and just like Clang, we need to test both the tool and the library.

I'd recommend us having unit-tests for this, as we already have them for the Triple and TargetParser. It'd be much easier to test this with unit-tests than ASM files, though we could also have some of the latter, to make sure the whole cycle is complete.

> Also, how would you suggest building a triple from just the build attributes? Currently I use the existing triple to only decide whether it should be an arm or thumb triple and I don't believe build attributes can help here. Unless you mean for thumb only targets?

Sorry, I didn't mean to change the triple, but to change the supported features from the build attributes. Thumb/ARM, VFP/NEON and other architectural support need to be restricted if the build attributes mandate so. Those features are set by default from the triple, but if the build attribute overrides, then we have to error out when parsing the assembly file.

cheer,
--renato



================
Comment at: include/llvm/Support/ARMAttributeParser.h:24
+
+  std::map<unsigned, std::pair<unsigned, std::string>> Attributes;
 
----------------
samparker wrote:
> rengolin wrote:
> > Why map to a pair if you only use the `unsigned` value?
> This was so that if someone else wanted the string value from a string attribute, it would be available.
When that is needed, we add. No need to hold huge strings (compared to an int) in the map today.


https://reviews.llvm.org/D28281