This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Assemble `call foo` to R_RISCV_CALL_PLT
ClosedPublic

Authored by MaskRay on Aug 23 2022, 10:02 PM.

Details

Summary

R_RISCV_CALL/R_RISCV_CALL_PLT distinction isn't necessary. R_RISCV_CALL has been
deprecated as a resolution to
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/98 .

ld.lld and mold treat the two relocation types the same. GNU ld has a custom
handling for undefined weak functions which is unnecessary: calling an
unresolved undefined weak function is UB and GNU ld can handle the case without
a relocation error (such a function call is usually guarded by a zero value
check and should be allowed).

This patch assembles call foo to use R_RISCV_CALL_PLT instead of the
deprecated R_RISCV_CALL.

Note: the code generator still differentiates call foo and (maybe preemptible)
call foo@plt, but the difference is purely aesthetic.

Note: D105429 does not support R_RISCV_CALL_PLT correctly. Changed the test to
force R_RISCV_CALL for now.

Diff Detail

Event Timeline

MaskRay created this revision.Aug 23 2022, 10:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 23 2022, 10:02 PM
MaskRay requested review of this revision.Aug 23 2022, 10:02 PM
jrtc27 added inline comments.Aug 23 2022, 10:22 PM
llvm/test/CodeGen/RISCV/branch-relaxation.ll
38 ↗(On Diff #455072)

This seems dodgy

MaskRay marked an inline comment as done.Aug 23 2022, 10:35 PM
MaskRay added inline comments.
llvm/test/CodeGen/RISCV/branch-relaxation.ll
38 ↗(On Diff #455072)

This is fine. Linkers don't care whether the R_RISCV_CALL_PLT target is temporary label/local/non-preemptible.

MaskRay marked an inline comment as done.Aug 24 2022, 8:49 AM

An alternative is to (a) replace assembler to ignore @plt (b) unify riscv-call and riscv-plt. It will touch many more tests, and omitting @plt uses the deprecated R_RISCV_CALL with GNU assembler (not a big issue).

MaskRay updated this revision to Diff 457188.Sep 1 2022, 12:20 AM
MaskRay retitled this revision from [RISCV] Replace deprecated R_RISCV_CALL with R_RISCV_CALL_PLT to [RISCV] Assemble `call foo` to R_RISCV_CALL_PLT.
MaskRay edited the summary of this revision. (Show Details)

Change assembler instead.

We need to change binutils gas as well.

kito-cheng accepted this revision.Sep 13 2022, 8:08 AM

LGTM, thanks, I've talked to Nelson about that, so he might update that on binutils side soon.

This revision is now accepted and ready to land.Sep 13 2022, 8:08 AM
This revision was landed with ongoing or failed builds.Sep 13 2022, 6:48 PM
This revision was automatically updated to reflect the committed changes.