[PATCH] D98101: [RISCV] Enable the LocalStackSlotAllocation pass support

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:322
+    if (!ReservedRegs.test(Reg) &&
+        !MF.getSubtarget<RISCVSubtarget>().isRegisterReservedByUser(Reg))
+      CalleeSavedSize += getSpillSize(*getMinimalPhysRegClass(Reg));
----------------
Is ReservedByUser not already cover by getReservedRegs?


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:332
+
+  // Assume 128 bytes spill slots size to estimate the maximum possible 
+  // offset relative to the stack pointer.
----------------
StephenFan wrote:
> jrtc27 wrote:
> > Why 128? A hard-coded constant common across all ISAs seems fishy.
> This value should be an experimental target-dependent value. But I don't know yet which value is appropriate for riscv.
Can we copy the comments from ARM and AArch64 here including the FIXME for 128?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98101