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

[Ed Maste via Phabricator via llvm-commits]

emaste added a comment.

Thank you, this will be quite useful for FreeBSD and the change seems reasonable to me.

In FreeBSD we target the original RPi, and currently use `TARGET_ARCH=armv6` across the majority of our supported 32-bit platforms (but are in the process of introducing `TARGET_ARCH=armv7`). We do support a small number of Armv4/v5 devices as well.



================
Comment at: ELF/Config.h:112-114
+  bool ARMHasBlx = false;
+  bool ARMHasMovtMovw = false;
+  bool ARMJ1J2BranchEncoding = false;
----------------
Should these now be `ArmHas...` etc.? (i.e., new capitalization as mentioned on llvm-weekly)


================
Comment at: ELF/Driver.cpp:1073-1075
+      Config->ARMHasBlx = true;
+      Config->ARMHasMovtMovw = true;
+      Config->ARMJ1J2BranchEncoding = true;
----------------
As a general principle I'd think these should really default to false, but until lld can generate proper output for the false cases I'd say of course this is preferred. (Or, is it the case that we expect tool chains to always produce an attributes section?)

Once it can generate proper output (and tests cover both cases) and the warnings below disappear perhaps this case for lack of attribute sections can become a warning?




https://reviews.llvm.org/D36823