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

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 14:16:05 PDT 2022


reames 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());
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126088



More information about the llvm-commits mailing list