This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add FrameSetup/FrameDestroy flag to prologue/epilog instructions.
ClosedPublic

Authored by HsiangKai on Jun 29 2021, 12:25 AM.

Diff Detail

Event Timeline

HsiangKai created this revision.Jun 29 2021, 12:25 AM
HsiangKai requested review of this revision.Jun 29 2021, 12:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2021, 12:25 AM

Remove redundant argument.

HsiangKai updated this revision to Diff 355144.Jun 29 2021, 1:38 AM

Fix build errors.

rogfer01 added inline comments.Jun 30 2021, 11:09 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
526–529

I think this might still add unflagged instructions.

Not sure what is the best way to flag them (I imagine we could traverse MBBI to the end of the MBB and invoke setFlag).

rogfer01 added inline comments.Jun 30 2021, 11:14 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
526–529

(I imagine we could traverse MBBI to the end of the MBB and invoke setFlag).

Oh no, that wouldn't work. Please ignore me.

HsiangKai added inline comments.Jul 1 2021, 6:17 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
526–529

Yeah, there is no way to distinguish the caller of copyPhysReg() comes from prologue/epilogue or not.

craig.topper added inline comments.Jul 1 2021, 6:59 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
526–529

Can we just use BuildMI? We're not getting much from copyPhysReg here are we? It's just an ADDI with 0, I think.

jrtc27 added inline comments.Jul 1 2021, 7:21 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
320

This should be an explicit argument rather than inferred

526–529

Downstream we'd have to add an if to use a capability move instruction for CHERI-RISC-V rather than getting to reuse the implementation in copyPhysReg, but that's hardly a huge burden. Ideally copyPhysReg would take an MIFlag but that's annoying to go and propagate everywhere just to avoid that.

HsiangKai updated this revision to Diff 356426.Jul 4 2021, 8:58 PM

Address comments.

asb accepted this revision.Jul 22 2021, 5:04 AM

This looks good to me, though it would be good to get a quick LGTM from someone who works more actively with the RVV code (e.g. Craig, Roger, or Fraser).

I think this one shouldn't be labeled as '[NFC]' as it will impact e.g. debug info generation.

This revision is now accepted and ready to land.Jul 22 2021, 5:04 AM
HsiangKai retitled this revision from [NFC][RISCV] Add FrameSetup/FrameDestroy flag to prologue/epilog instructions. to [RISCV] Add FrameSetup/FrameDestroy flag to prologue/epilog instructions..Jul 22 2021, 7:37 AM