[PATCH] D36823: [LLD][ELF] Read ARM BuildAttributes section to determine supported features.

[Peter Smith via Phabricator via llvm-commits]

peter.smith marked an inline comment as done.
peter.smith added a comment.

Thanks very much for the comments.

I initially thought that putting the Arm specific parts of the implementation into Arch/ARM, however it looks like all of the Targets in Arch are just an implementation of Target.h and aren't directly accessed from the rest of lld, for example there is no ARM.h to include so it looked like I was breaking someone's abstraction. I'm happy to move them if it is the consensus that it is worth separating them.

I'm also happy to move the command line options into ArmFeatures, although I noted that the Mips specific features weren't isolated either, again if there is a consensus that this is the right thing to do I'll do it.



================
Comment at: ELF/InputFiles.cpp:372
+    case ARMBuildAttrs::v6KZ:
+    case ARMBuildAttrs::v6K:
+      Config->ARMHasBlx = true;
----------------
grimar wrote:
> Is `v6T2` intentionally missing from this list ?
Yes it is. The ARMBuildAttrs v6T2 should be covered by the default: case. I've attempted to explain this with the comment:

```
      // Architectures used in pre-Cortex processors do not support
      // The J1 = 1 J2 = 1 Thumb branch range extension, with the exception
      // of Architecture v6T2 (arm1156t2-s and arm1156t2f-s) that do.
```
If that isn't clear? I can reword it.
 




https://reviews.llvm.org/D36823