[PATCH] D126088: [RISCV] Add clarifying asserts to getFrameIndexReference [NFC]

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:718
     if (hasFP(MF)) {
+      assert(FrameReg == RISCV::X8 && "Frame register not FP");
       Offset += StackOffset::getFixed(RVFI->getVarArgsSaveSize());
----------------
reames wrote:
> jrtc27 wrote:
> > The whole point of getFrameRegister is to avoid hard-coding, and this would be a pain for us downstream in CHERI LLVM where pure-capability code uses capability register C8 not the X8 integer sub-register. I mean, I'd just delete the assert downstream rather than repeat the condition... so don't like this assert.
> Would having a RISCVABI::getFPReg() and using that in the assert make life easier for you?  It would still give me the property I want to assert (e.g. we are using FP if hasFP is true), but maybe be a bit easier to merge?
We already have getFP/SPReg functions in this file that take the subtarget. Using those for this would be fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126088