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

[Peter Smith via Phabricator via llvm-commits]

peter.smith updated this revision to Diff 124376.
peter.smith added a comment.
Herald added a subscriber: arichardson.

Updated review diff incorporating review comments, I'll commit tomorrow if there are no more changes needed.

To answer a couple of questions in more detail:

>> +// The ARM support in lld makes some use of instructions that are not available
>>  +// on all ARM architectures. Namely:
>>  +// - Use of BLX instruction for interworking between ARM and Thumb state.
>>  +// - Use of the extended Thumb branch encoding in relocation.
>>  +// - Use of the MOVT/MOVW instructions in Thumb Thunks.
>>  +// The ARM Attributes section contains information about the architecture chosen
>>  +// at compile time. We follow the convention that if at least one input object
>>  +// is compiled with an architecture that supports these features then lld is
>>  +// permitted to use them.
>> 
>> That is the same heuristic used by bfd and gold?

In a simplified form yes. Both bfd and gold have additional logic to detect incompatibilities between attributes, but both will choose the highest architecture if the two are compatible. In the absence of a linker -mcpu option the only other option is to fault inconsistencies (how do we know which of the two the target system is?), which I think would be frustrating.

The basic rules for combining build attributes are defined from the ABI (http://infocenter.arm.com/help/topic/com.arm.doc.ihi0045e/IHI0045E_ABI_addenda.pdf) search for "Combining Attribute Values". The intention is that the values for each Build Attributes tag such as Tag_CPU_arch form a partial order in terms of the demands that they make of the environment. It is up to the tool to determine what it wants to do with that ordering information.

>> +  if (Config->EMachine == EM_ARM) {
>>  +    if (InX::ARMAttributes == nullptr) {
>>  +      // No ARM attributes section present (assembler without .eabi_attribute
>>  +      // directives, like lld tests), assume support for all features
>>  +      Config->ARMHasBlx = true;
>>  +      Config->ARMHasMovtMovw = true;
>>  +      Config->ARMJ1J2BranchEncoding = true;
>>  +    }
> 
> The tests don't fail because of a warning, right? So you could probably
>  just remove this if.

I've removed it in the updated diff. This has no effect right now. I've got some follow up patches to post that add support for v5 and v6 Arm, when these are added I'll need to add a .eabi_attribute 6, 10    @ Tag_CPU_arch for v7; for the existing branch range tests to make sure they use the v7 branch range.


https://reviews.llvm.org/D36823

Files:
  ELF/Config.h
  ELF/Driver.cpp
  ELF/InputFiles.cpp
  test/ELF/arm-bl-v6.s
  test/ELF/arm-blx-v4t.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36823.124376.patch
Type: text/x-patch
Size: 8068 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171127/b8e310bb/attachment.bin>