This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add support for static chain
ClosedPublic

Authored by melonedo on Jul 4 2022, 11:42 PM.

Details

Summary

The static chain parameter is a special parameter that is not passed in the usual argument registers or stack space. For example, in x64 System V ABI it is always passed in R10. Although the ABI of RISCV does not assign a register for this purpose, GCC had support for it on RISC-V a long time ago, and it is exposed via __builtin_call_with_static_chain intrinsic, and assign t2 for static chain parameters. This patch also chose t2 for compatibility.

In LLVM, static chain parameters are handled by the nest attribute of an argument to a function (D6332), so tests are added to ensure nest arguments are handled correctly.

Diff Detail

Event Timeline

melonedo created this revision.Jul 4 2022, 11:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2022, 11:42 PM
melonedo requested review of this revision.Jul 4 2022, 11:42 PM
melonedo updated this revision to Diff 442195.Jul 5 2022, 1:05 AM

Rebase the commit on master.

I guess this patch should update the other conventions as well? E.g. error out if nest is used with the GHC CC. There's also the FastCC.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
10206–10207 ↗(On Diff #442195)

Nit: "as did" -> "as done"

llvm/test/CodeGen/RISCV/nest-register.ll
1–9

Checking RV32IM and RV64IM is not needed.

jrtc27 added a comment.Jul 5 2022, 7:00 AM

This doesn’t work with dynamic linking (unless you use the recently-added STO_RISCV_VARIANT_CC) as t2 can be clobbered by the resolver.

melonedo updated this revision to Diff 453893.Aug 18 2022, 11:19 PM

Adopt suggestions

This doesn’t work with dynamic linking (unless you use the recently-added STO_RISCV_VARIANT_CC) as t2 can be clobbered by the resolver.

Sorry for the delay, I opened an issue in GCC and they feel OK with a scratch register.

I guess this patch should update the other conventions as well? E.g. error out if nest is used with the GHC CC. There's also the FastCC.

I added an error check in GHC CC. If not mistaken, the FastCC is internal, so as long as the frontend emit consistent function attrubutes, it does not make a difference whether an argument is attributed 'nest' or not. Ignoring 'nest' in the FastCC should be fine.

Could you re-upload your patch again, the diff seems is only contain fixes for review commets?

melonedo updated this revision to Diff 458636.Sep 7 2022, 8:59 PM
melonedo marked 2 inline comments as done.

Rebase and upload changes again

This revision is now accepted and ready to land.Oct 26 2022, 11:34 PM
MaskRay requested changes to this revision.Oct 27 2022, 5:58 PM
MaskRay added inline comments.
llvm/test/CodeGen/RISCV/nest-register.ll
10

i8* => ptr for opaque pointers

This revision now requires changes to proceed.Oct 27 2022, 5:58 PM
melonedo updated this revision to Diff 471367.Oct 27 2022, 7:16 PM

Use ptr for opaque pointers

melonedo marked an inline comment as done.Oct 27 2022, 7:17 PM
melonedo updated this revision to Diff 471368.Oct 27 2022, 7:19 PM

Use opaque pointers

melonedo updated this revision to Diff 471369.Oct 27 2022, 7:21 PM

Also use opaque pointers in ghccc-nest.ll

jrtc27 added inline comments.Oct 27 2022, 7:25 PM
llvm/test/CodeGen/RISCV/ghccc-nest.ll
1 ↗(On Diff #471369)

Double negative, though feel this is redundant with the CHECK line

melonedo updated this revision to Diff 471373.Oct 27 2022, 7:29 PM
melonedo marked an inline comment as done.

Revise comments

MaskRay added inline comments.Oct 27 2022, 7:35 PM
llvm/test/CodeGen/RISCV/ghccc-nest.ll
2 ↗(On Diff #471373)

-filetype=null if the output is not used

melonedo updated this revision to Diff 471382.Oct 27 2022, 8:11 PM

Specify -filetype=null

melonedo marked an inline comment as done.Oct 27 2022, 8:11 PM
MaskRay accepted this revision.Nov 3 2022, 11:05 PM
This revision is now accepted and ready to land.Nov 3 2022, 11:05 PM
This revision was automatically updated to reflect the committed changes.