Page MenuHomePhabricator

[RISCV] Lower calls through PLT
ClosedPublic

Authored by lewis-revill on Dec 4 2018, 5:15 PM.

Details

Summary

This patch adds support for generating calls through the procedure linkage table where required for a given ExternalSymbol or GlobalAddress callee.

Diff Detail

Repository
rL LLVM

Event Timeline

lewis-revill created this revision.Dec 4 2018, 5:15 PM
jrtc27 requested changes to this revision.Dec 4 2018, 7:49 PM
jrtc27 added inline comments.
lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
1108 ↗(On Diff #176742)

Do we want to be parsing the @plt for things other than call/tail? PseudoLLA also takes a bare_symbol but @plt isn't valid. Currently we parse bar@plt as a valid bare symbol; however, GNU as will reject it with illegal operands (as I understand it, this is because it doesn't treat @ as part of a valid identifier).

lib/Target/RISCV/RISCVSubtarget.cpp
51 ↗(On Diff #176742)

This can be inlined in LowerCall, especially once the fno-plt stuff is gone, as it's the only user.

56 ↗(On Diff #176742)

This seems to be X86-specific in LLVM; do we really need this in RISC-V? Clang's test/CodeGen/noplt.c gives a normal call foo@plt when compiled with riscv64-linux-gnu-gcc -fpic -fno-plt, so I would remove this and the related code.

This revision now requires changes to proceed.Dec 4 2018, 7:49 PM
lewis-revill marked 2 inline comments as done.Dec 5 2018, 9:22 AM
lewis-revill added inline comments.
lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
1108 ↗(On Diff #176742)

Hmm, probably a good idea. I guess I'd have to add yet another operand type in RISCVInstrInfo. Checking Operands[0] for "call" or "tail" seems a bit hacky to me, but maybe it's a reasonable approach?

lib/Target/RISCV/RISCVSubtarget.cpp
56 ↗(On Diff #176742)

OK, that makes things a lot simpler for LowerCall then. Is there no case where calls need to be lowered using GOT?

lewis-revill planned changes to this revision.Dec 5 2018, 10:11 AM

Splitting this patch into MC layer and Codegen.

lewis-revill marked 2 inline comments as not done.Dec 5 2018, 11:24 AM

Split into MC layer (patch D55335) and codegen (this patch).

lewis-revill retitled this revision from [RISCV, WIP] Lower calls through GOT and PLT to [RISCV, WIP] Lower calls through PLT.
lewis-revill edited the summary of this revision. (Show Details)

Removed capability of lowering through GOT since there is no apparent need to do so.

Rebased to use D55560 and with fewer dependencies

lewis-revill retitled this revision from [RISCV, WIP] Lower calls through PLT to [RISCV] Lower calls through PLT.

Rebased.

Rebased and added testing using -relocation-model=pic in 'calls.ll'. Also added a dso_local callee to 'calls.ll' since this affects whether PLT is used for PIC.

Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2019, 8:47 AM
Herald added subscribers: Jim, benna, psnobl. · View Herald Transcript
asb accepted this revision.Jun 18 2019, 2:55 AM

LGTM, thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Jun 18 2019, 7:26 AM
Closed by commit rL363686: [RISCV] Lower calls through PLT (authored by lewis-revill, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.