This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Attach VK_RISCV_CALL to symbols upon creation

Authored by lewis-revill on Dec 11 2018, 9:02 AM.



This patch replaces the addition of VK_RISCV_CALL in RISCVMCCodeEmitter by creating the RISCVMCExpr when tail/call are parsed, or in the codegen case when the callee symbols are created.

This required adding a new CallSymbol operand to allow only adding VK_RISCV_CALL to tail/call instructions.

This patch will allow further expansion of parsing and codegen to easily include PLT symbols which must generate the R_RISCV_CALL_PLT relocation.

Diff Detail


Event Timeline

lewis-revill created this revision.Dec 11 2018, 9:02 AM
jrtc27 added inline comments.Dec 11 2018, 9:07 AM
280 ↗(On Diff #177725)

I don't think we want to allow VK_RISCV_None, as expandFunctionCall won't add a VK_RISCV_CALL wrapper so we won't get an R_RISCV_CALL?

lewis-revill added inline comments.Dec 11 2018, 12:54 PM
280 ↗(On Diff #177725)

Good catch, I'll fix that.

Do not allow VK_RISCV_None on call symbols

lewis-revill marked an inline comment as done.Dec 11 2018, 1:01 PM
asb added a comment.Jan 10 2019, 7:44 AM

I know this isn't a behaviour change vs the previous bare_symbol, but I note that GNU as currently accepts call with constants, e.g. call 1234. Should we be doing the same?

Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2019, 9:21 AM
jrtc27 added a comment.Feb 5 2019, 9:22 AM

(Added dependency due to both adding to RISCVBaseInfo.h)

jrtc27 added a comment.Feb 5 2019, 9:32 AM

Hi Lewis, please rebase this after rL351823 (the Call node is now a riscv_call node in PseudoCll's pattern.

asb accepted this revision.Apr 1 2019, 7:49 AM

Thanks Lewis, this looks good to me. We should later revisit whether we should accept call 1234 like GNU as does.

This revision is now accepted and ready to land.Apr 1 2019, 7:49 AM
This revision was automatically updated to reflect the committed changes.