From 5ad59c9e594dba82f8ef630ae56d68dc8d47834b Mon Sep 17 00:00:00 2001 From: Alex Bradbury Date: Tue, 2 Aug 2022 09:43:32 +0100 Subject: [PATCH] [RISCV][NFCI] Set TransientStackAlignment and rely on it rather than RVV-specific logic on RVV-less functions * TargetFrameLowering has a TransientStackAlignment field that "returns the number of bytes to which the stack pointer must be aligned at all times, even between calls. * As explained in the [RISC-V calling convention](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc), the stack pointer must remain fully aligned throughout execution for compliant code. This is important for embedded targets that might avoid realigning the stack pointer for interrupt service routines. Systems running full OSes may always realign the stack anyway. * TransientStackAlignment is used in estimateStackSize in MachineFrameInfo and in PEI::calculateFrameObjectOffsets. * estimateStackSize is only used in the RISC-V backend for scavenging slots. It may be possible to craft a function where the difference is observable, but it wouldn't be a meaningful test. * calculateFrameObjectOffsets makes use of TransientStackAlignment, but then sets the stack alignment to the max of that alignment and MaxAlign, which is unconditionally set to 16 in RISCVFrameLowering::processFunctionBeforeFrameFinalized * I've changed this logic to only set MaxAlign if there are RVV frame objects. There should be no functional change here for either RVV targets (MaxAlign is set as before) or non-RVV targets (TransientStackAlign is now 16 anyway). Differential Revision: https://reviews.llvm.org/D130068 --- llvm/lib/Target/RISCV/RISCVFrameLowering.cpp | 48 +++++++++++++++------------- llvm/lib/Target/RISCV/RISCVFrameLowering.h | 3 +- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp index a7286b2..cd60a59 100644 --- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp @@ -996,6 +996,25 @@ static unsigned getScavSlotsNumForRVV(MachineFunction &MF) { return MaxScavSlotsNum; } +static bool hasRVVFrameObject(const MachineFunction &MF) { + // Originally, the function will scan all the stack objects to check whether + // if there is any scalable vector object on the stack or not. However, it + // causes errors in the register allocator. In issue 53016, it returns false + // before RA because there is no RVV stack objects. After RA, it returns true + // because there are spilling slots for RVV values during RA. It will not + // reserve BP during register allocation and generate BP access in the PEI + // pass due to the inconsistent behavior of the function. + // + // The function is changed to use hasVInstructions() as the return value. It + // is not precise, but it can make the register allocation correct. + // + // FIXME: Find a better way to make the decision or revisit the solution in + // D103622. + // + // Refer to https://github.com/llvm/llvm-project/issues/53016. + return MF.getSubtarget().hasVInstructions(); +} + void RISCVFrameLowering::processFunctionBeforeFrameFinalized( MachineFunction &MF, RegScavenger *RS) const { const RISCVRegisterInfo *RegInfo = @@ -1011,10 +1030,12 @@ void RISCVFrameLowering::processFunctionBeforeFrameFinalized( RVFI->setRVVStackSize(RVVStackSize); RVFI->setRVVStackAlign(RVVStackAlign); - // Ensure the entire stack is aligned to at least the RVV requirement: some - // scalable-vector object alignments are not considered by the - // target-independent code. - MFI.ensureMaxAlignment(RVVStackAlign); + if (hasRVVFrameObject(MF)) { + // Ensure the entire stack is aligned to at least the RVV requirement: some + // scalable-vector object alignments are not considered by the + // target-independent code. + MFI.ensureMaxAlignment(RVVStackAlign); + } // estimateStackSize has been observed to under-estimate the final stack // size, so give ourselves wiggle-room by checking for stack size @@ -1051,25 +1072,6 @@ void RISCVFrameLowering::processFunctionBeforeFrameFinalized( RVFI->setCalleeSavedStackSize(Size); } -static bool hasRVVFrameObject(const MachineFunction &MF) { - // Originally, the function will scan all the stack objects to check whether - // if there is any scalable vector object on the stack or not. However, it - // causes errors in the register allocator. In issue 53016, it returns false - // before RA because there is no RVV stack objects. After RA, it returns true - // because there are spilling slots for RVV values during RA. It will not - // reserve BP during register allocation and generate BP access in the PEI - // pass due to the inconsistent behavior of the function. - // - // The function is changed to use hasVInstructions() as the return value. It - // is not precise, but it can make the register allocation correct. - // - // FIXME: Find a better way to make the decision or revisit the solution in - // D103622. - // - // Refer to https://github.com/llvm/llvm-project/issues/53016. - return MF.getSubtarget().hasVInstructions(); -} - // Not preserve stack space within prologue for outgoing variables when the // function contains variable size objects or there are vector objects accessed // by the frame pointer. diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.h b/llvm/lib/Target/RISCV/RISCVFrameLowering.h index a5cf68a..8b13eb0 100644 --- a/llvm/lib/Target/RISCV/RISCVFrameLowering.h +++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.h @@ -24,7 +24,8 @@ public: explicit RISCVFrameLowering(const RISCVSubtarget &STI) : TargetFrameLowering(StackGrowsDown, /*StackAlignment=*/Align(16), - /*LocalAreaOffset=*/0), + /*LocalAreaOffset=*/0, + /*TransientStackAlignment=*/Align(16)), STI(STI) {} void emitPrologue(MachineFunction &MF, MachineBasicBlock &MBB) const override; -- 2.7.4