Skip to content

Commit

Permalink
[RISCV] Align stack size down to a multiple of 16 before using cm.pus…
Browse files Browse the repository at this point in the history
…h/pop. (#86073)

This an alternative to #84935 to fix the miscompile, but not be optimal.

The immediate for cm.push/pop must be a multiple of 16. For RVE, it
might not be. It's not easy to increase the stack size without messing
up cfa directives and maybe other things.

This patch rounds the stack size down to a multiple of 16 before
clamping it to 48. This causes an extra addi to be emitted to handle the
remainder.

Once this commited, I can commit #84989 to add verification for these
instructions being generated with valid offsets.
  • Loading branch information
topperc committed Mar 27, 2024
1 parent ecf6bb2 commit 8a9c170
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
12 changes: 8 additions & 4 deletions llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,10 @@ void RISCVFrameLowering::emitPrologue(MachineFunction &MF,
if (RVFI->isPushable(MF) && FirstFrameSetup != MBB.end() &&
FirstFrameSetup->getOpcode() == RISCV::CM_PUSH) {
// Use available stack adjustment in push instruction to allocate additional
// stack space.
uint64_t Spimm = std::min(StackSize, (uint64_t)48);
// stack space. Align the stack size down to a multiple of 16. This is
// needed for RVE.
// FIXME: Can we increase the stack size to a multiple of 16 instead?
uint64_t Spimm = std::min(alignDown(StackSize, 16), (uint64_t)48);
FirstFrameSetup->getOperand(1).setImm(Spimm);
StackSize -= Spimm;
}
Expand Down Expand Up @@ -776,8 +778,10 @@ void RISCVFrameLowering::emitEpilogue(MachineFunction &MF,
if (RVFI->isPushable(MF) && MBBI != MBB.end() &&
MBBI->getOpcode() == RISCV::CM_POP) {
// Use available stack adjustment in pop instruction to deallocate stack
// space.
uint64_t Spimm = std::min(StackSize, (uint64_t)48);
// space. Align the stack size down to a multiple of 16. This is needed for
// RVE.
// FIXME: Can we increase the stack size to a multiple of 16 instead?
uint64_t Spimm = std::min(alignDown(StackSize, 16), (uint64_t)48);
MBBI->getOperand(1).setImm(Spimm);
StackSize -= Spimm;
}
Expand Down
6 changes: 4 additions & 2 deletions llvm/test/CodeGen/RISCV/zcmp-additional-stack.ll
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
define ptr @func(ptr %s, i32 %_c, ptr %incdec.ptr, i1 %0, i8 %conv14) #0 {
; RV32-LABEL: func:
; RV32: # %bb.0: # %entry
; RV32-NEXT: cm.push {ra, s0-s1}, -24
; RV32-NEXT: cm.push {ra, s0-s1}, -16
; RV32-NEXT: addi sp, sp, -8
; RV32-NEXT: .cfi_def_cfa_offset 24
; RV32-NEXT: .cfi_offset ra, -12
; RV32-NEXT: .cfi_offset s0, -8
Expand Down Expand Up @@ -31,7 +32,8 @@ define ptr @func(ptr %s, i32 %_c, ptr %incdec.ptr, i1 %0, i8 %conv14) #0 {
; RV32-NEXT: lw a0, 4(sp) # 4-byte Folded Reload
; RV32-NEXT: sb a0, 0(s0)
; RV32-NEXT: mv a0, s1
; RV32-NEXT: cm.popret {ra, s0-s1}, 24
; RV32-NEXT: addi sp, sp, 8
; RV32-NEXT: cm.popret {ra, s0-s1}, 16
entry:
br label %while.body

Expand Down

0 comments on commit 8a9c170

Please sign in to comment.