[PATCH] D105086: [NFC][RISCV] Add FrameSetup/FrameDestroy flag to prologue/epilog instructions.

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:320
   unsigned Opc = RISCV::ADD;
+  MachineInstr::MIFlag FrameFlag = MachineInstr::FrameDestroy;
   if (Amount < 0) {
----------------
This should be an explicit argument rather than inferred


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:528
         // move BP, SP
         TII->copyPhysReg(MBB, MBBI, DL, BPReg, SPReg, false);
       }
----------------
craig.topper wrote:
> HsiangKai wrote:
> > rogfer01 wrote:
> > > rogfer01 wrote:
> > > > 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`).
> > > > (I imagine we could traverse MBBI to the end of the MBB and invoke setFlag).
> > > Oh no, that wouldn't work. Please ignore me.
> > Yeah, there is no way to distinguish the caller of copyPhysReg() comes from prologue/epilogue or not.
> Can we just use BuildMI? We're not getting much from copyPhysReg here are we? It's just an ADDI with 0, I think.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105086/new/

https://reviews.llvm.org/D105086