[PATCH] D39849: [RISCV] Implement prolog and epilog insertion

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/RISCVFrameLowering.cpp:130
+  DebugLoc DL = MBBI->getDebugLoc();
+  unsigned FPReg = RISCV::X8;
+  unsigned SPReg = RISCV::X2;
----------------
reames wrote:
> You've got these in a couple of places, time for an alias in the enum?
I've added some helper functions to return FPReg and SPReg. They take RISCVSubtargetInfo to allow future extensibility (e.g. different ABIs).


================
Comment at: test/CodeGen/RISCV/addc-adde-sube-subc.ll:10
 ; RV32I:       # BB#0:
+; RV32I-NEXT:    addi sp, sp, -16
+; RV32I-NEXT:    sw ra, 12(sp)
----------------
reames wrote:
> The test churn in this patch is really unfortunate.  Is there a small change you can add to not need all the spill code for most functions?
Frame pointer elimination isn't a particularly large change (see [here](https://github.com/lowRISC/riscv-llvm/blob/16adc08a754609a02fb1cf5bbd11663be6de5b82/0043-RISCV-Implement-frame-pointer-elimination.patch)). Although verbose generated code has disadvantages, I think there are benefits to "completing" codegen before supporting extra optimisations, even fairly trivial ones. Although ultimately a straight-forward change, a large percentage of the bugs I've encountered while developing the backend have actually been related to frame handling (whether as part of argument passing or otherwise).


https://reviews.llvm.org/D39849