Page MenuHomePhabricator

[LLD][ELF][ARM] Add support for Armv5 and Armv6 compatible Thunks
ClosedPublic

Authored by peter.smith on Jul 31 2018, 9:23 AM.

Details

Summary

Older Arm architectures such as Armv5 and Armv6 do not support the MOVT and MOVW instructions used in the Arm range extension thunks. When we detect that all input objects are pre-Armv7 we use an alternative sequence of instructions that does not use these that either loads the absolute address of the destination into the PC, or loads an offset to the destination that is added to the PC.

With this patch and D50076 it should be possible to use LLD to target a first edition Raspberry Pi (Armv6). These thunks are not compatible with Armv4t (arm7tdmi) which I don't plan on supporting.

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith created this revision.Jul 31 2018, 9:23 AM
efriedma added inline comments.
ELF/Thunks.cpp
485 ↗(On Diff #158297)

I think add pc, pc, ip isn't an interworking branch on pre-v7 targets, so you can't use this sequence if the destination is a Thumb function.

ruiu added inline comments.Jul 31 2018, 1:58 PM
ELF/Driver.cpp
1463 ↗(On Diff #158297)

Currently, target is essentially a read-only object which only its constructor can initialize its members. So, if this needs to be a variable, can you define a function that returns an appropriate value, instead of precompute a value and mutate the Target member?

grimar added a comment.Aug 1 2018, 1:41 AM

I am not an expert in ARM, so have only a style comments/questions.

ELF/Thunks.cpp
622 ↗(On Diff #158297)

Does it make sense to include the file name and/or at least relocation value in this error message?

646 ↗(On Diff #158297)

Is it possible to test this error?
Also, Thunks should be lower case (and perhaps Architecture too).

Thanks for the comments, I'll post another patch shortly.

ELF/Driver.cpp
1463 ↗(On Diff #158297)

Yes, will do.

ELF/Thunks.cpp
485 ↗(On Diff #158297)

Yes you are correct. I've checked in the Arm ARM under interworking and add only comes up from v7 onwards. I'll update with with add ip, pc, ip ; bx ip.

622 ↗(On Diff #158297)

I'll improve it to add relocation type and symbol name. We don't have access to the filename at this point and given that this error shouldn't occur if all the input files have the right build attributes it is probably not worth threading it through.

646 ↗(On Diff #158297)

Yes, will correct and add tests for the error messages.

Updated patch. Changes:

  • Correct instruction sequence in PI thunk to avoid use of Armv7 instruction
  • Update and add tests for error messages
  • Moved ThunkSectionSpacing into a function

I have no other comments, thanks!

Ping, I don't think that there are any outstanding comments that need addressing.

niosHD added a subscriber: niosHD.Aug 14 2018, 4:19 AM
ruiu accepted this revision.Aug 20 2018, 1:35 AM

LGTM

This revision is now accepted and ready to land.Aug 20 2018, 1:35 AM
This revision was automatically updated to reflect the committed changes.