This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Simplify BP initialisation
ClosedPublic

Authored by StephenFan on Jan 22 2021, 6:15 AM.

Details

Summary

We can re-use copyPhysReg rather than writing a specialised copy.

Diff Detail

Event Timeline

StephenFan created this revision.Jan 22 2021, 6:15 AM
StephenFan requested review of this revision.Jan 22 2021, 6:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2021, 6:15 AM
StephenFan retitled this revision from DRY for copy physical register to [RISCV]DRY for copy physical register.Jan 22 2021, 6:53 AM
jrtc27 retitled this revision from [RISCV]DRY for copy physical register to [RISCV] Simplify BP initialisation.Jan 22 2021, 7:01 AM
jrtc27 edited the summary of this revision. (Show Details)
craig.topper added inline comments.Jan 22 2021, 4:51 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
484

While they aren't necessary, leave the curly braces since the comment makes more than one line below the if.

Address craig.topper's comment.

StephenFan marked an inline comment as done.Jan 24 2021, 6:11 PM

This is certainly cleaner, in the sense of the code operating at a higher level of abstraction. I wonder if it's worth the indirection cost.
I'll let others bikeshed this :-)

jrtc27 accepted this revision.Jan 25 2021, 2:22 PM

This is certainly cleaner, in the sense of the code operating at a higher level of abstraction. I wonder if it's worth the indirection cost.
I'll let others bikeshed this :-)

Fewer places we have to teach to use a cmove cbp, csp dowstream in CHERI-LLVM :) (although we've already paid that cost, but still, diff reducing is good)

This revision is now accepted and ready to land.Jan 25 2021, 2:22 PM
asb accepted this revision.Jan 28 2021, 5:31 AM

LGTM.

This revision was landed with ongoing or failed builds.Feb 17 2021, 4:34 AM
This revision was automatically updated to reflect the committed changes.