[llvm] [RISC-V] Fix incorrect epilogue_begin (PR #120623)

Venkata Ramanaiah Nalamothu via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 27 18:03:38 PST 2025


================
@@ -1088,7 +1092,8 @@ void RISCVFrameLowering::emitEpilogue(MachineFunction &MF,
   // Skip to before the restores of scalar callee-saved registers
   // FIXME: assumes exactly one instruction is used to restore each
   // callee-saved register.
-  auto LastFrameDestroy = std::prev(MBBI, getUnmanagedCSI(MF, CSI).size());
+  auto FirstScalarCSRRestoreInsn =
+      std::next(MBBI, getRVVCalleeSavedInfo(MF, CSI).size());
----------------
RamNalamothu wrote:

If we observe _RISCVFrameLowering::spillCalleeSavedRegisters_ and _RISCVFrameLowering::restoreCalleeSavedRegisters_, the frame layout is as follows:

```
spilling CSR regs:
               CM.PUSH/CSRSaveLibCall           ----- A
               ScalarCSRSaves                   ----- B
               VectorCSRSaves                   ----- C
restoring CSR regs:
               VectorCSRRestores                   ----- C
               ScalarCSRRestores                   ----- B
               CM.POP/CSRRestoreLibCall            ----- A

```
The _RISCVFrameLowering::emitPrologue_ and _RISCVFrameLowering::emitEpilogue_ need a reliable mechanism, because the CSR list is specific to each function and is not fixed, to identify beginning of **B** as per the below comment. The mechanism used is start at beginning (in emitPrologue) / ending (in emitEpilogue) of **MBB** and then try to find beginning of **B** using _FrameSetup/FrameDestroy_ flag and _getRVVCalleeSavedInfo(MF, CSI).size()_,                              _getUnmanagedCSI(MF, CSI).size()_.

   _// The frame pointer is callee-saved, and code has been generated for us to
  // save it to the stack. We need to skip over the storing of callee-saved
  // registers as the frame pointer must be modified after it has been saved
  // to the stack, not before_

Before my patch, we only generate _FrameSetup/FrameDestroy_ flag for **A** but now this patch generates those flags for all of **A**, **B** and **C** as that is needed to fix the _epilogue_begin_ identification. And this obviously requires adjusting the existing logic here which is trying to identify **B** based on _FrameSetup/FrameDestroy_ flags.

I just picked one way of doing it and if that's not the better one, I will give it a try to improve it.

Hope that gives more clarity and answers all the concerns raised.

https://github.com/llvm/llvm-project/pull/120623


More information about the llvm-commits mailing list