Stack frame on RISC-V with RVE32 and +save-restore

Code Generation RISCV
1

Compiling for RISC-V with options -target-feature +e -target-abi ilp32e -target-feature +save-restore creates function prologues like this:

TestFunction:
.cfi_startproc
call t0, __riscv_save_0
.cfi_def_cfa_offset 4
.cfi_offset ra, -4
addi a3, a0, 72
…

The function __riscv_save_0 actually pushes 3 words onto the stack, advancing the stack pointer by 12 bytes:

__riscv_save_2:
__riscv_save_1:
__riscv_save_0:
addi sp, sp, -12
sw s1, 0(sp)
sw s0, 4(sp)
sw ra, 8(sp)
jr t0

But the “.cfi_def_cfa_offset 4” doesn’t match this stack frame, which causes failures at stack unwinding.

What is wrong here? The calculation of cfi_def_cfa_offset or the function __riscv_save_0, which could push only one word to the stack?

2 
  if (int LibCallRegs = getLibCallID(MF, MFI.getCalleeSavedInfo()) + 1) {
    // Calculate the size of the frame managed by the libcall. The stack
    // alignment of these libcalls should be the same as how we set it in
    // getABIStackAlignment.
    unsigned LibCallFrameSize =
        alignTo((STI.getXLen() / 8) * LibCallRegs, getStackAlign());
    RVFI->setLibCallStackSize(LibCallFrameSize);
  }

This is the calculation used to determine that. Unfortunately the RVE libcalls don’t follow that pattern, they instead are all coalesced into one that always saves all 3 registers, even though stack alignment is such that it could split them up. Both compiler-rt and libgcc are the same here (and compiler-rt does the same for RV64E), but it’s not been formally written down in GitHub - riscv-non-isa/riscv-toolchain-conventions: Documenting the expected behaviour and supported command-line switches for GNU and LLVM based RISC-V toolchains, so I don’t know how much that was intended. GCC itself does know that RVE always saves 3 registers though, so I guess it is ABI at this point.

3

I saw this when the E save/restore stubs were added and was also confused. While I have written versions with the lower alignments, it’s not clear they’re better because it’s possible most functions will need to save at least three registers anyway.

I will post the patch I have soon, maybe tomorrow.

4

Thanks.

5

I’m confused about your claims regarding GCC - the cfi directives seem to match clang from what I can see here: Compiler Explorer - maybe it was fixed in a recent version?

6

I was just reading the source: gcc/gcc/config/riscv/riscv.cc at 3dac1049c1211e6d06c2536b86445a6334c3866d · gcc-mirror/gcc · GitHub

Maybe that doesn’t influence CFI and they have the same bug as us elsewhere.

7

Aha, no, they’re just keying off the ISA not the ABI.

8

Ok, I think this strikes me as two parts of the same bug in their system - I believe both their libcall implementations and this code should be keyed of the ABI, rather than the ISA.

9

The PR is here: [compiler-rt][RISC-V] Save-Restore for ILP32E/LP64E ABIs by lenary · Pull Request #95390 · llvm/llvm-project · GitHub - let’s work out which direction to go in there.