This is an archive of the discontinued LLVM Phabricator instance.

[lld][LoongArch] Support the R_LARCH_PCREL20_S2 relocation type
ClosedPublic

Authored by SixWeining on Jul 31 2023, 9:06 PM.

Details

Summary

R_LARCH_PCREL20_S2 is a new added relocation type in LoongArch ELF
psABI v2.10 [1] which is not corvered by D138135 except R_LARCH_64_PCREL.

A motivation to support R_LARCH_PCREL20_S2 in lld is to build the
runtime of .NET core (a.k.a CoreCLR) in which strict PC-relative
semantics need to be guaranteed [2]. The normal pcalau12i + addi.d
approach doesn't work because the code will be copied to other places
with different "page" and offsets. To achieve this, we can use pcaddi
with explicit R_LARCH_PCREL20_S2 reloc to address +-2MB PC-relative
range with 4-bytes aligned.

[1]: https://github.com/loongson/la-abi-specs/releases/tag/v2.10
[2]: https://github.com/dotnet/runtime/blob/release/7.0/src/coreclr/vm/loongarch64/asmhelpers.S#L307

Diff Detail

Event Timeline

SixWeining created this revision.Jul 31 2023, 9:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2023, 9:06 PM
SixWeining requested review of this revision.Jul 31 2023, 9:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2023, 9:06 PM
xen0n added a comment.Jul 31 2023, 9:32 PM

Thanks for the patch, actually I had this one in my local repo for a while but hadn't submitted just yet ;-) The code change is trivially correct and I only have a minor suggestion for the test case. Let's wait for @MaskRay's opinions though...

lld/test/ELF/loongarch-pcrel20-s2.s
13–17

I remember MaskRay once mentioned the desire to keep lld invocations to a minimum? To me this looks like two pcaddi's pointing to two separate sections (one above .text and one below), then only one run for the positive cases would be enough. Let's see what he thinks.

MaskRay added inline comments.Aug 1 2023, 8:02 AM
lld/test/ELF/loongarch-pcrel20-s2.s
13–17

Yes, we should reduce the number of invocations. If a linker script can test multiple edge cases, we should use that.

SixWeining updated this revision to Diff 546481.Aug 2 2023, 8:08 AM

Remove redundant lld invocations in tests.

xen0n accepted this revision.Aug 8 2023, 3:33 AM

LGTM (LoongArch-wise); @MaskRay please take another look (we'll wait for your comment before landing of course), thanks!

This revision is now accepted and ready to land.Aug 8 2023, 3:33 AM
MaskRay accepted this revision.Aug 8 2023, 5:50 PM