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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:230
+    llvm_unreachable("Unexpected Reg for Push/Pop Inst");
+  case /*s11*/ RISCV::X27:
+  case /*s10*/ RISCV::X26:
----------------
Put the register name comments at the end of the line instead of in the middle


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:276
+  case /*s10*/ RISCV::X26:
+    PushPopRegs = 13;
+    return RISCV::X27;
----------------
Can we count PushPopRegs as we go through the loop and only have a special case for RISCV::X26 after the loop?


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:277
+    PushPopRegs = 13;
+    return RISCV::X27;
+  case /*s9*/ RISCV::X25:
----------------
Can we replace this with

```
MaxPushPopReg = RISCV::X27;
break;
```

to avoid a return when every other case breaks.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:483
   const auto &CSI = MFI.getCalleeSavedInfo();
-
   // The frame pointer is callee-saved, and code has been generated for us to
----------------
Why is this blank line removed?


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1355
 
-  const char *SpillLibCall = getSpillLibCallName(*MF, CSI);
-  if (SpillLibCall) {
+  // Emmit CM.PUSH with base SPimm & evaluate Push stack
+  RISCVMachineFunctionInfo *RVFI = MF->getInfo<RISCVMachineFunctionInfo>();
----------------
Emmit -> Emit


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1442
+
+      static const Register AllPopRegs[] = {
+          RISCV::X1,  RISCV::X8,  RISCV::X9,  RISCV::X18, RISCV::X19,
----------------
This list is repeated in two places. Should it be a global variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134599