This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Relax R_RISCV_CALL and R_RISCV_CALL_PLT
ClosedPublic

Authored by MaskRay on Jun 12 2022, 11:05 PM.

Details

Summary

A pair of auipc+jalr relocated by R_RISCV_CALL or R_RISCV_CALL_PLT can be
converted to c.j, c.jal, or jal.

  • c.j: RVC and displacement is representable as an int12
  • c.jal: RV32C and displacement is representable as an int12
  • jal: displacement is representable as an int21

Use the D127581 relaxation framework to implement the relaxation. If a shorter
sequence is satisfied, we record the new relocation type in relocTypes and
saves the new instruction into writes. Finally let riscvFinalizeRelax rewrite the
instruction by setting skip.

Diff Detail

Event Timeline

MaskRay created this revision.Jun 12 2022, 11:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2022, 11:05 PM
MaskRay requested review of this revision.Jun 12 2022, 11:05 PM
MaskRay updated this revision to Diff 436689.Jun 14 2022, 1:09 AM
MaskRay edited the summary of this revision. (Show Details)

rebase

MaskRay updated this revision to Diff 438898.Jun 21 2022, 8:17 PM

improve tests

MaskRay accepted this revision.Jun 23 2022, 11:54 PM

Will push this next week.

This revision is now accepted and ready to land.Jun 23 2022, 11:54 PM
luismarques accepted this revision.Jul 3 2022, 7:25 AM

My tests finally finished running. No issues detected. LGTM.

lld/test/ELF/riscv-relax-call.s
122

Nit: "32c" is confusing. "RV32C" is the usual name.

135

Nit: just write "RV32C" instead of "32c (RVC32)"?

jrtc27 added inline comments.Jul 3 2022, 9:46 AM
lld/ELF/Arch/RISCV.cpp
479

RelType

485

Same comment as for relocDeltas in the other patch, really just want a pointer not a dynamically-resizable SmallVector

523

Shouldn't we know from the relocation's expr which we picked? Repeating a version of that logic seems fragile.

582

R_RISCV_NONE

687

RelType (and maybe check != R_RISCV_NONE explicitly?)

716

(maybe != R_RISCV_NONE?)

luismarques requested changes to this revision.Jul 3 2022, 11:09 AM

Sorry, my previous approval comment was meant for D127581.

This revision now requires changes to proceed.Jul 3 2022, 11:09 AM
luismarques resigned from this revision.Jul 3 2022, 11:10 AM
This revision is now accepted and ready to land.Jul 3 2022, 11:10 AM
MaskRay updated this revision to Diff 441972.Jul 3 2022, 12:09 PM
MaskRay marked 8 inline comments as done.

(in a trip, so i didn't spend too much time yet)
but all comments should be addressed

lld/test/ELF/riscv-relax-call.s
122

32c is the executable name.

Run this patch with LLVM testsuite and internal testsuite, no regression.

MaskRay updated this revision to Diff 442449.Jul 5 2022, 11:45 PM

rebase
improve test

This revision was landed with ongoing or failed builds.Jul 7 2022, 10:18 AM
This revision was automatically updated to reflect the committed changes.