This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF][ARM] Implement ARM pc-relative relocations for adr and ldr
ClosedPublic

Authored by psmith on Feb 28 2020, 6:05 AM.

Details

Summary

The R_ARM_ALU_PC_G0 and R_ARM_LDR_PC_G0 relocations are used by the ADR and LDR pseudo instructions, and are the basis of the group relocations that can load an arbitrary constant via a series of add, sub and ldr instructions.

The relocations need to be obtained via the .reloc directive.

R_ARM_ALU_PC_G0 is much more complicated as the add/sub instruction uses a modified immediate encoding of an 8-bit immediate rotated right by an even 4-bit field. This means that the range of representable immediates is sparse. We extract the encoding and decoding functions for the modified immediate from llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h as this header file is not accessible from LLD. Duplication of code isn't ideal, but as these are well-defined mathematical functions they are unlikely to change.

Diff Detail

Event Timeline

psmith created this revision.Feb 28 2020, 6:05 AM

The assembly tests should be rewritten as yaml2obj because we will either resolve/reject such short-range fixups in MC.

The assembly tests should be rewritten as yaml2obj because we will either resolve/reject such short-range fixups in MC.

Thanks, I'll start working on that. May take a few days to work through them.

psmith updated this revision to Diff 255076.Apr 4 2020, 1:47 PM
psmith edited the summary of this revision. (Show Details)

Update diff to change tests to use .inst and .reloc. Updated description to remove the dependency on an LLVM-MC change. No other changes outside of tests.

Thanks for the update. I confess that I don't really understand the encoding but we should not waste the efforts making Arch/ARM.cpp complete. I mostly only get some nitpicking.

lld/ELF/Arch/ARM.cpp
418

From https://llvm.org/docs/CodingStandards.html

Don’t duplicate function or class name at the beginning of the comment.

lld/test/ELF/arm-adr-err.s
4

Do we need -n?

lld/test/ELF/arm-adr-long.s
9

Do we need -n ?

33

Beauty is in the eye of the beholder, but does it look better if CHECK: lines have sufficient spaces so that the contents align with contents on CHECK-LINE:? Additionally, some addresses can be omitted.

// CHECK:      10000000 <dat1>:
// CHECK-NEXT:  andeq r0, r0, r0

andeq is indented to make it clear that the instruction "belongs to" the <dat1> function.

psmith updated this revision to Diff 255656.Apr 7 2020, 6:05 AM

Thanks for the suggestions, I've updated the comments in ARM.cpp, I've also tried to align the CHECK so that it would be indented the same way as if we were looking at a raw llvm-objdump output. I've kept the addresses in for the moment as I've put an explanation in the comment that shows how the instruction matches the target. Happy to make further changes if less information is preferred.

MaskRay accepted this revision.Apr 7 2020, 1:39 PM

Thanks!

This revision is now accepted and ready to land.Apr 7 2020, 1:39 PM
This revision was automatically updated to reflect the committed changes.