[PATCH] D134599: [RISCV] Readjusting the framestack for Zcmp

[Xinlong Wu via Phabricator via llvm-commits]

VincentWu added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1377
+    }
+    RVFI->setRVPushStackSize(PoshPopRegs * (IsRV64 ? 8 : 4));
+
----------------
fakepaper56 wrote:
> IMHO, is it possible the callee save register needed spilled are not successive, like {ra, s0, s3}? 
I think it should not support this syntax.
In spec, it is valid only in the following cases:

{F27468105}


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1474-1475
 
-  const char *RestoreLibCall = getRestoreLibCallName(*MF, CSI);
-  if (RestoreLibCall) {
-    // Add restore libcall via tail call.
-    MachineBasicBlock::iterator NewMI =
-        BuildMI(MBB, MI, DL, TII.get(RISCV::PseudoTAIL))
-            .addExternalSymbol(RestoreLibCall, RISCVII::MO_CALL)
-            .setMIFlag(MachineInstr::FrameDestroy);
-
-    // Remove trailing returns, since the terminator is now a tail call to the
-    // restore function.
-    if (MI != MBB.end() && MI->getOpcode() == RISCV::PseudoRET) {
-      NewMI->copyImplicitOps(*MF, *MI);
-      MI->eraseFromParent();
+    MachineInstrBuilder PopBuilder =
+        BuildMI(MBB, MI, DL, TII.get(RISCV::CM_POP));
+    // Use encoded number to represent registers to restore.
----------------
KYG wrote:
> VincentWu wrote:
> > KYG wrote:
> > > Need to notify CM_POP kill (def) callee-saved registers, otherwise MIScheduler could not see the dependency, but I'm not sure how to do this (addDef to each poped register?).
> > I think I have got what you mean, but can you tell me which function I should use to kill the callee-saved register? 
> > Or where the LW/LD instruction does？
> Normally we should add those registers to the "Defs" list in .td, but I don't know how to do this since the rlist is a compile time variable.
> So I just add implicit def to the MI as a convenient workaround.
> ```
> for (int x = 0; RegList[x] <= MaxReg && RegList[x] > 0; ++x)
>   PopBuilder.addDef(RegList[x], RegState::Implicit);
> ```
is it `RegState::Implicit`? did you mean `RegState::Kill` ?

or I have add `.setMIFlag(MachineInstr::FrameDestroy)`, does it useful for this point?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134599