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

[luxufan via Phabricator via llvm-commits]

StephenFan added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:693
+    // | scalar local variables   | | <----'
+    // |--------------------------| -- <-- BP (if present)
+    // | VarSize objects          | |
----------------
Does `if present` means if the BP exists? But If both realignment and VarSize objects exist, then `BP` must be present.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:718
     if (hasFP(MF)) {
+      assert(FrameReg == getFPReg(STI) && "Frame register not FP");
       Offset += StackOffset::getFixed(RVFI->getVarArgsSaveSize());
----------------
I also don't like this assertion and IMO we need to avoid this trivial assertion. I call it a trivial assertion since `getFrameRegister` would return the frame register according to the result of `hasFP`, this assertion would always succeed unless the compiler generates the wrong code. 


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

https://reviews.llvm.org/D126088