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

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

I'm expecting some tests to make sure that the new attributes are correctly parsed, printed, and that objects without a triple, but with attributes, can pick the right triple from them.



================
Comment at: include/llvm/Support/ARMAttributeParser.h:24
+
+  std::map<unsigned, std::pair<unsigned, std::string>> Attributes;
 
----------------
Why map to a pair if you only use the `unsigned` value?


================
Comment at: lib/Object/ELFObjectFile.cpp:125
 
+void ELFObjectFileBase::setARMSubArch(Triple &TheTriple) const {
+  if (TheTriple.getSubArch() != Triple::NoSubArch)
----------------
I don't like this function being here. It should be encoded from some table gen description or from the target parser, but that shouldn't be done in this commit.

Please add a FIXME to that effect.


================
Comment at: lib/Object/ELFObjectFile.cpp:140
+
+  switch(Attributes.getAttributeValue(ARMBuildAttrs::CPU_arch)) {
+  case ARMBuildAttrs::v4:
----------------
This list doesn't have all possible values documented in the ABI.


https://reviews.llvm.org/D28281