[CodeGen] Replace (Min,Max)CSFrameIndex with flag on frame object [NFCI] #170905
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the tracking of the MinCSFrameIndex, and MaxCSFrameIndex markers, simplifying the target API. This brings the tracking for callee save spill slots in line with how we handle other properties of stack locations. A couple notes: 1) This requires doing scans of the entire object range, but we have other such instances in the code already, so I doubt this will matter in practice. 2) This removes the requirement that callee saved spill slots be contigious in the frame index identified space. I marked this as NFCI because if prior code violated the contingous range assumption - I can't find a case where we did - then this change might adjust frame layout in some edge cases. The motivation for this is mostly code readability, but I might use this as a primitive for something in an upcoming patch series around shrink wrapping. Haven't decided yet.
Member
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-backend-risc-v Author: Philip Reames (preames) ChangesThis removes the tracking of the MinCSFrameIndex, and MaxCSFrameIndex markers, simplifying the target API. This brings the tracking for callee save spill slots in line with how we handle other properties of stack locations. A couple notes:
I marked this as NFCI because if prior code violated the contiguous range assumption - I can't find a case where we did - then this change might adjust frame layout in some edge cases. The motivation for this is mostly code readability, but I might use this as a primitive for something in an upcoming patch series around shrink wrapping. Haven't decided yet. Patch is 21.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170905.diff 9 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 50ce93104ab53..c4852f84be352 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -153,6 +153,10 @@ class MachineFrameInfo {
/// register allocator.
bool isStatepointSpillSlot = false;
+ /// If true, this stack slot is used for spilling a callee saved register
+ /// in the calling convention of the containing function.
+ bool isCalleeSaved = false;
+
/// Identifier for stack memory type analagous to address space. If this is
/// non-0, the meaning is target defined. Offsets cannot be directly
/// compared between objects with different stack IDs. The object may not
@@ -762,6 +766,18 @@ class MachineFrameInfo {
return Objects[ObjectIdx+NumFixedObjects].isStatepointSpillSlot;
}
+ bool isCalleeSavedObjectIndex(int ObjectIdx) const {
+ assert(unsigned(ObjectIdx + NumFixedObjects) < Objects.size() &&
+ "Invalid Object Idx!");
+ return Objects[ObjectIdx + NumFixedObjects].isCalleeSaved;
+ }
+
+ void setIsCalleeSavedObjectIndex(int ObjectIdx, bool IsCalleeSaved) {
+ assert(unsigned(ObjectIdx + NumFixedObjects) < Objects.size() &&
+ "Invalid Object Idx!");
+ Objects[ObjectIdx + NumFixedObjects].isCalleeSaved = IsCalleeSaved;
+ }
+
/// \see StackID
uint8_t getStackID(int ObjectIdx) const {
return Objects[ObjectIdx+NumFixedObjects].StackID;
diff --git a/llvm/include/llvm/CodeGen/TargetFrameLowering.h b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
index 75696faf114cc..99034754e466b 100644
--- a/llvm/include/llvm/CodeGen/TargetFrameLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
@@ -159,14 +159,6 @@ class LLVM_ABI TargetFrameLowering {
/// returns false, spill slots will be assigned using generic implementation.
/// assignCalleeSavedSpillSlots() may add, delete or rearrange elements of
/// CSI.
- virtual bool assignCalleeSavedSpillSlots(MachineFunction &MF,
- const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
- return assignCalleeSavedSpillSlots(MF, TRI, CSI);
- }
-
virtual bool
assignCalleeSavedSpillSlots(MachineFunction &MF,
const TargetRegisterInfo *TRI,
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index 41efe622417c8..3cf885e744229 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -80,11 +80,6 @@ namespace {
class PEIImpl {
RegScavenger *RS = nullptr;
- // MinCSFrameIndex, MaxCSFrameIndex - Keeps the range of callee saved
- // stack frame indexes.
- unsigned MinCSFrameIndex = std::numeric_limits<unsigned>::max();
- unsigned MaxCSFrameIndex = 0;
-
// Save and Restore blocks of the current function. Typically there is a
// single save block, unless Windows EH funclets are involved.
MBBVector SaveBlocks;
@@ -456,9 +451,7 @@ void PEIImpl::calculateSaveRestoreBlocks(MachineFunction &MF) {
}
static void assignCalleeSavedSpillSlots(MachineFunction &F,
- const BitVector &SavedRegs,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) {
+ const BitVector &SavedRegs) {
if (SavedRegs.empty())
return;
@@ -490,8 +483,7 @@ static void assignCalleeSavedSpillSlots(MachineFunction &F,
const TargetFrameLowering *TFI = F.getSubtarget().getFrameLowering();
MachineFrameInfo &MFI = F.getFrameInfo();
- if (!TFI->assignCalleeSavedSpillSlots(F, RegInfo, CSI, MinCSFrameIndex,
- MaxCSFrameIndex)) {
+ if (!TFI->assignCalleeSavedSpillSlots(F, RegInfo, CSI)) {
// If target doesn't implement this, use generic code.
if (CSI.empty())
@@ -534,8 +526,7 @@ static void assignCalleeSavedSpillSlots(MachineFunction &F,
// min.
Alignment = std::min(Alignment, TFI->getStackAlign());
FrameIdx = MFI.CreateStackObject(Size, Alignment, true);
- if ((unsigned)FrameIdx < MinCSFrameIndex) MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex) MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
} else {
// Spill it to the stack where we must.
FrameIdx = MFI.CreateFixedSpillStackObject(Size, FixedSlot->Offset);
@@ -676,15 +667,13 @@ void PEIImpl::spillCalleeSavedRegs(MachineFunction &MF) {
const Function &F = MF.getFunction();
const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
MachineFrameInfo &MFI = MF.getFrameInfo();
- MinCSFrameIndex = std::numeric_limits<unsigned>::max();
- MaxCSFrameIndex = 0;
// Determine which of the registers in the callee save list should be saved.
BitVector SavedRegs;
TFI->determineCalleeSaves(MF, SavedRegs, RS);
// Assign stack slots for any callee-saved registers that must be spilled.
- assignCalleeSavedSpillSlots(MF, SavedRegs, MinCSFrameIndex, MaxCSFrameIndex);
+ assignCalleeSavedSpillSlots(MF, SavedRegs);
// Add the code to save and restore the callee saved registers.
if (!F.hasFnAttribute(Attribute::Naked)) {
@@ -752,10 +741,10 @@ static inline void AdjustStackOffset(MachineFrameInfo &MFI, int FrameIdx,
/// Compute which bytes of fixed and callee-save stack area are unused and keep
/// track of them in StackBytesFree.
-static inline void
-computeFreeStackSlots(MachineFrameInfo &MFI, bool StackGrowsDown,
- unsigned MinCSFrameIndex, unsigned MaxCSFrameIndex,
- int64_t FixedCSEnd, BitVector &StackBytesFree) {
+static inline void computeFreeStackSlots(MachineFrameInfo &MFI,
+ bool StackGrowsDown,
+ int64_t FixedCSEnd,
+ BitVector &StackBytesFree) {
// Avoid undefined int64_t -> int conversion below in extreme case.
if (FixedCSEnd > std::numeric_limits<int>::max())
return;
@@ -769,11 +758,10 @@ computeFreeStackSlots(MachineFrameInfo &MFI, bool StackGrowsDown,
if (MFI.getStackID(i) == TargetStackID::Default)
AllocatedFrameSlots.push_back(i);
// Add callee-save objects if there are any.
- if (MinCSFrameIndex <= MaxCSFrameIndex) {
- for (int i = MinCSFrameIndex; i <= (int)MaxCSFrameIndex; ++i)
- if (MFI.getStackID(i) == TargetStackID::Default)
- AllocatedFrameSlots.push_back(i);
- }
+ for (int i = MFI.getObjectIndexBegin(); i < MFI.getObjectIndexEnd(); i++)
+ if (MFI.isCalleeSavedObjectIndex(i) &&
+ MFI.getStackID(i) == TargetStackID::Default)
+ AllocatedFrameSlots.push_back(i);
for (int i : AllocatedFrameSlots) {
// These are converted from int64_t, but they should always fit in int
@@ -923,20 +911,28 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
Align MaxAlign = MFI.getMaxAlign();
// First assign frame offsets to stack objects that are used to spill
// callee saved registers.
- if (MaxCSFrameIndex >= MinCSFrameIndex) {
- for (unsigned i = 0; i <= MaxCSFrameIndex - MinCSFrameIndex; ++i) {
- unsigned FrameIndex =
- StackGrowsDown ? MinCSFrameIndex + i : MaxCSFrameIndex - i;
-
+ if (StackGrowsDown) {
+ for (int FI = MFI.getObjectIndexBegin(); FI < MFI.getObjectIndexEnd();
+ FI++) {
+ // Only allocate objects on the default stack.
+ if (!MFI.isCalleeSavedObjectIndex(FI) ||
+ MFI.getStackID(FI) != TargetStackID::Default)
+ continue;
+ AdjustStackOffset(MFI, FI, StackGrowsDown, Offset, MaxAlign);
+ }
+ } else {
+ for (int FI = MFI.getObjectIndexEnd() - 1; FI >= MFI.getObjectIndexBegin();
+ FI--) {
// Only allocate objects on the default stack.
- if (MFI.getStackID(FrameIndex) != TargetStackID::Default)
+ if (!MFI.isCalleeSavedObjectIndex(FI) ||
+ MFI.getStackID(FI) != TargetStackID::Default)
continue;
- // TODO: should this just be if (MFI.isDeadObjectIndex(FrameIndex))
- if (!StackGrowsDown && MFI.isDeadObjectIndex(FrameIndex))
+ // TODO: should this just be if (MFI.isDeadObjectIndex(FI))
+ if (MFI.isDeadObjectIndex(FI))
continue;
- AdjustStackOffset(MFI, FrameIndex, StackGrowsDown, Offset, MaxAlign);
+ AdjustStackOffset(MFI, FI, StackGrowsDown, Offset, MaxAlign);
}
}
@@ -1025,7 +1021,7 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
if (MFI.isObjectPreAllocated(i) && MFI.getUseLocalStackAllocationBlock())
continue;
- if (i >= MinCSFrameIndex && i <= MaxCSFrameIndex)
+ if (MFI.isCalleeSavedObjectIndex(i))
continue;
if (RS && RS->isScavengingFrameIndex((int)i))
continue;
@@ -1077,7 +1073,7 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
if (MFI.isObjectPreAllocated(i) && MFI.getUseLocalStackAllocationBlock())
continue;
- if (i >= MinCSFrameIndex && i <= MaxCSFrameIndex)
+ if (MFI.isCalleeSavedObjectIndex(i))
continue;
if (RS && RS->isScavengingFrameIndex((int)i))
continue;
@@ -1113,8 +1109,7 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
if (!ObjectsToAllocate.empty() &&
MF.getTarget().getOptLevel() != CodeGenOptLevel::None &&
MFI.getStackProtectorIndex() < 0 && TFI.enableStackSlotScavenging(MF))
- computeFreeStackSlots(MFI, StackGrowsDown, MinCSFrameIndex, MaxCSFrameIndex,
- FixedCSEnd, StackBytesFree);
+ computeFreeStackSlots(MFI, StackGrowsDown, FixedCSEnd, StackBytesFree);
// Now walk the objects and actually assign base offsets to them.
for (auto &Object : ObjectsToAllocate)
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 7290b3f67c2e3..c2f5c0368a782 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -2755,8 +2755,7 @@ void AArch64FrameLowering::determineCalleeSaves(MachineFunction &MF,
bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
MachineFunction &MF, const TargetRegisterInfo *RegInfo,
- std::vector<CalleeSavedInfo> &CSI, unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
+ std::vector<CalleeSavedInfo> &CSI) const {
bool NeedsWinCFI = needsWinCFI(MF);
unsigned StackHazardSize = getStackHazardSize(MF);
// To match the canonical windows frame layout, reverse the list of
@@ -2779,10 +2778,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
if (UsesWinAAPCS && hasFP(MF) && AFI->hasSwiftAsyncContext()) {
int FrameIdx = MFI.CreateStackObject(8, Align(16), true);
AFI->setSwiftAsyncContextFrameIdx(FrameIdx);
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
}
// Insert VG into the list of CSRs, immediately before LR if saved.
@@ -2812,31 +2808,21 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << HazardSlotIndex
<< "\n");
AFI->setStackHazardCSRSlotIndex(HazardSlotIndex);
- if ((unsigned)HazardSlotIndex < MinCSFrameIndex)
- MinCSFrameIndex = HazardSlotIndex;
- if ((unsigned)HazardSlotIndex > MaxCSFrameIndex)
- MaxCSFrameIndex = HazardSlotIndex;
+ MFI.setIsCalleeSavedObjectIndex(HazardSlotIndex, true);
}
unsigned Size = RegInfo->getSpillSize(*RC);
Align Alignment(RegInfo->getSpillAlign(*RC));
int FrameIdx = MFI.CreateStackObject(Size, Alignment, true);
CS.setFrameIdx(FrameIdx);
-
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
// Grab 8 bytes below FP for the extended asynchronous frame info.
if (hasFP(MF) && AFI->hasSwiftAsyncContext() && !UsesWinAAPCS &&
Reg == AArch64::FP) {
FrameIdx = MFI.CreateStackObject(8, Alignment, true);
AFI->setSwiftAsyncContextFrameIdx(FrameIdx);
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
}
LastReg = Reg;
}
@@ -2848,10 +2834,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << HazardSlotIndex
<< "\n");
AFI->setStackHazardCSRSlotIndex(HazardSlotIndex);
- if ((unsigned)HazardSlotIndex < MinCSFrameIndex)
- MinCSFrameIndex = HazardSlotIndex;
- if ((unsigned)HazardSlotIndex > MaxCSFrameIndex)
- MaxCSFrameIndex = HazardSlotIndex;
+ MFI.setIsCalleeSavedObjectIndex(HazardSlotIndex, true);
}
return true;
@@ -2968,9 +2951,8 @@ static SVEStackSizes determineSVEStackSizes(MachineFunction &MF,
}
for (int FI = 0, E = MFI.getObjectIndexEnd(); FI != E; ++FI) {
- if (FI == StackProtectorFI || MFI.isDeadObjectIndex(FI))
- continue;
- if (MaxCSFrameIndex >= FI && FI >= MinCSFrameIndex)
+ if (FI == StackProtectorFI || MFI.isDeadObjectIndex(FI) ||
+ MFI.isCalleeSavedObjectIndex(FI))
continue;
if (MFI.getStackID(FI) != TargetStackID::ScalableVector &&
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.h b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
index 32a9bd831989c..97db18dd30bef 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
@@ -88,11 +88,10 @@ class AArch64FrameLowering : public TargetFrameLowering {
bool hasReservedCallFrame(const MachineFunction &MF) const override;
- bool assignCalleeSavedSpillSlots(MachineFunction &MF,
- const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const override;
+ bool
+ assignCalleeSavedSpillSlots(MachineFunction &MF,
+ const TargetRegisterInfo *TRI,
+ std::vector<CalleeSavedInfo> &CSI) const override;
void determineCalleeSaves(MachineFunction &MF, BitVector &SavedRegs,
RegScavenger *RS) const override;
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index ffbb111d42221..ec3e720ef8887 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -1844,9 +1844,7 @@ void SIFrameLowering::determineCalleeSavesSGPR(MachineFunction &MF,
static void assignSlotsUsingVGPRBlocks(MachineFunction &MF,
const GCNSubtarget &ST,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) {
+ std::vector<CalleeSavedInfo> &CSI) {
SIMachineFunctionInfo *FuncInfo = MF.getInfo<SIMachineFunctionInfo>();
MachineFrameInfo &MFI = MF.getFrameInfo();
const SIRegisterInfo *TRI = ST.getRegisterInfo();
@@ -1915,10 +1913,7 @@ static void assignSlotsUsingVGPRBlocks(MachineFunction &MF,
int FrameIdx =
MFI.CreateStackObject(BlockSize, TRI->getSpillAlign(*BlockRegClass),
/*isSpillSlot=*/true);
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
CSIt->setFrameIdx(FrameIdx);
CSIt->setReg(RegBlock);
@@ -1928,8 +1923,7 @@ static void assignSlotsUsingVGPRBlocks(MachineFunction &MF,
bool SIFrameLowering::assignCalleeSavedSpillSlots(
MachineFunction &MF, const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI, unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
+ std::vector<CalleeSavedInfo> &CSI) const {
if (CSI.empty())
return true; // Early exit if no callee saved registers are modified!
@@ -1937,12 +1931,12 @@ bool SIFrameLowering::assignCalleeSavedSpillSlots(
bool UseVGPRBlocks = ST.useVGPRBlockOpsForCSR();
if (UseVGPRBlocks)
- assignSlotsUsingVGPRBlocks(MF, ST, CSI, MinCSFrameIndex, MaxCSFrameIndex);
+ assignSlotsUsingVGPRBlocks(MF, ST, CSI);
- return assignCalleeSavedSpillSlots(MF, TRI, CSI) || UseVGPRBlocks;
+ return assignCalleeSavedSpillSlotsImpl(MF, TRI, CSI) || UseVGPRBlocks;
}
-bool SIFrameLowering::assignCalleeSavedSpillSlots(
+bool SIFrameLowering::assignCalleeSavedSpillSlotsImpl(
MachineFunction &MF, const TargetRegisterInfo *TRI,
std::vector<CalleeSavedInfo> &CSI) const {
if (CSI.empty())
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.h b/llvm/lib/Target/AMDGPU/SIFrameLowering.h
index a72772987262e..4c1cf3c7f9526 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.h
@@ -49,11 +49,9 @@ class SIFrameLowering final : public AMDGPUFrameLowering {
const TargetRegisterInfo *TRI,
std::vector<CalleeSavedInfo> &CSI) const override;
- bool assignCalleeSavedSpillSlots(MachineFunction &MF,
- const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const override;
+ bool assignCalleeSavedSpillSlotsImpl(MachineFunction &MF,
+ const TargetRegisterInfo *TRI,
+ std::vector<CalleeSavedInfo> &CSI) const;
bool spillCalleeSavedRegisters(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MI,
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 75e7cf347e461..668bb84e72c3b 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -1992,17 +1992,17 @@ RISCVFrameLowering::getFirstSPAdjustAmount(const MachineFunction &MF) const {
bool RISCVFrameLowering::assignCalleeSavedSpillSlots(
MachineFunction &MF, const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI, unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
+ std::vector<CalleeSavedInfo> &CSI) const {
auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
+ MachineFrameInfo &MFI = MF.getFrameInfo();
+ const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
// Preemptible Interrupts have two additional Callee-save Frame Indexes,
// not tracked by `CSI`.
if (RVFI->isSiFivePreemptibleInterrupt(MF)) {
for (int I = 0; I < 2; ++I) {
int FI = RVFI->getInterruptCSRFrameIndex(I);
- MinCSFrameIndex = std::min<unsigned>(MinCSFrameIndex, FI);
- MaxCSFrameIndex = std::max<unsigned>(MaxCSFrameIndex, FI);
+ MFI.setIsCalleeSavedObjectIndex(FI, true);
}
}
@@ -2028,9 +2028,6 @@ bool RISCVFrameLowering::assignCalleeSavedSpillSlots(
}
}
- MachineFrameInfo &MFI = MF.getFrameInfo();
- const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
-
for (auto &CS : CSI) {
MCRegister Reg = CS.getReg();
const TargetRegisterClass *RC = RegInfo->getMinimalPhysReg...
[truncated]
|
Member
|
@llvm/pr-subscribers-backend-amdgpu Author: Philip Reames (preames) ChangesThis removes the tracking of the MinCSFrameIndex, and MaxCSFrameIndex markers, simplifying the target API. This brings the tracking for callee save spill slots in line with how we handle other properties of stack locations. A couple notes:
I marked this as NFCI because if prior code violated the contiguous range assumption - I can't find a case where we did - then this change might adjust frame layout in some edge cases. The motivation for this is mostly code readability, but I might use this as a primitive for something in an upcoming patch series around shrink wrapping. Haven't decided yet. Patch is 21.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170905.diff 9 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 50ce93104ab53..c4852f84be352 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -153,6 +153,10 @@ class MachineFrameInfo {
/// register allocator.
bool isStatepointSpillSlot = false;
+ /// If true, this stack slot is used for spilling a callee saved register
+ /// in the calling convention of the containing function.
+ bool isCalleeSaved = false;
+
/// Identifier for stack memory type analagous to address space. If this is
/// non-0, the meaning is target defined. Offsets cannot be directly
/// compared between objects with different stack IDs. The object may not
@@ -762,6 +766,18 @@ class MachineFrameInfo {
return Objects[ObjectIdx+NumFixedObjects].isStatepointSpillSlot;
}
+ bool isCalleeSavedObjectIndex(int ObjectIdx) const {
+ assert(unsigned(ObjectIdx + NumFixedObjects) < Objects.size() &&
+ "Invalid Object Idx!");
+ return Objects[ObjectIdx + NumFixedObjects].isCalleeSaved;
+ }
+
+ void setIsCalleeSavedObjectIndex(int ObjectIdx, bool IsCalleeSaved) {
+ assert(unsigned(ObjectIdx + NumFixedObjects) < Objects.size() &&
+ "Invalid Object Idx!");
+ Objects[ObjectIdx + NumFixedObjects].isCalleeSaved = IsCalleeSaved;
+ }
+
/// \see StackID
uint8_t getStackID(int ObjectIdx) const {
return Objects[ObjectIdx+NumFixedObjects].StackID;
diff --git a/llvm/include/llvm/CodeGen/TargetFrameLowering.h b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
index 75696faf114cc..99034754e466b 100644
--- a/llvm/include/llvm/CodeGen/TargetFrameLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
@@ -159,14 +159,6 @@ class LLVM_ABI TargetFrameLowering {
/// returns false, spill slots will be assigned using generic implementation.
/// assignCalleeSavedSpillSlots() may add, delete or rearrange elements of
/// CSI.
- virtual bool assignCalleeSavedSpillSlots(MachineFunction &MF,
- const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
- return assignCalleeSavedSpillSlots(MF, TRI, CSI);
- }
-
virtual bool
assignCalleeSavedSpillSlots(MachineFunction &MF,
const TargetRegisterInfo *TRI,
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index 41efe622417c8..3cf885e744229 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -80,11 +80,6 @@ namespace {
class PEIImpl {
RegScavenger *RS = nullptr;
- // MinCSFrameIndex, MaxCSFrameIndex - Keeps the range of callee saved
- // stack frame indexes.
- unsigned MinCSFrameIndex = std::numeric_limits<unsigned>::max();
- unsigned MaxCSFrameIndex = 0;
-
// Save and Restore blocks of the current function. Typically there is a
// single save block, unless Windows EH funclets are involved.
MBBVector SaveBlocks;
@@ -456,9 +451,7 @@ void PEIImpl::calculateSaveRestoreBlocks(MachineFunction &MF) {
}
static void assignCalleeSavedSpillSlots(MachineFunction &F,
- const BitVector &SavedRegs,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) {
+ const BitVector &SavedRegs) {
if (SavedRegs.empty())
return;
@@ -490,8 +483,7 @@ static void assignCalleeSavedSpillSlots(MachineFunction &F,
const TargetFrameLowering *TFI = F.getSubtarget().getFrameLowering();
MachineFrameInfo &MFI = F.getFrameInfo();
- if (!TFI->assignCalleeSavedSpillSlots(F, RegInfo, CSI, MinCSFrameIndex,
- MaxCSFrameIndex)) {
+ if (!TFI->assignCalleeSavedSpillSlots(F, RegInfo, CSI)) {
// If target doesn't implement this, use generic code.
if (CSI.empty())
@@ -534,8 +526,7 @@ static void assignCalleeSavedSpillSlots(MachineFunction &F,
// min.
Alignment = std::min(Alignment, TFI->getStackAlign());
FrameIdx = MFI.CreateStackObject(Size, Alignment, true);
- if ((unsigned)FrameIdx < MinCSFrameIndex) MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex) MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
} else {
// Spill it to the stack where we must.
FrameIdx = MFI.CreateFixedSpillStackObject(Size, FixedSlot->Offset);
@@ -676,15 +667,13 @@ void PEIImpl::spillCalleeSavedRegs(MachineFunction &MF) {
const Function &F = MF.getFunction();
const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
MachineFrameInfo &MFI = MF.getFrameInfo();
- MinCSFrameIndex = std::numeric_limits<unsigned>::max();
- MaxCSFrameIndex = 0;
// Determine which of the registers in the callee save list should be saved.
BitVector SavedRegs;
TFI->determineCalleeSaves(MF, SavedRegs, RS);
// Assign stack slots for any callee-saved registers that must be spilled.
- assignCalleeSavedSpillSlots(MF, SavedRegs, MinCSFrameIndex, MaxCSFrameIndex);
+ assignCalleeSavedSpillSlots(MF, SavedRegs);
// Add the code to save and restore the callee saved registers.
if (!F.hasFnAttribute(Attribute::Naked)) {
@@ -752,10 +741,10 @@ static inline void AdjustStackOffset(MachineFrameInfo &MFI, int FrameIdx,
/// Compute which bytes of fixed and callee-save stack area are unused and keep
/// track of them in StackBytesFree.
-static inline void
-computeFreeStackSlots(MachineFrameInfo &MFI, bool StackGrowsDown,
- unsigned MinCSFrameIndex, unsigned MaxCSFrameIndex,
- int64_t FixedCSEnd, BitVector &StackBytesFree) {
+static inline void computeFreeStackSlots(MachineFrameInfo &MFI,
+ bool StackGrowsDown,
+ int64_t FixedCSEnd,
+ BitVector &StackBytesFree) {
// Avoid undefined int64_t -> int conversion below in extreme case.
if (FixedCSEnd > std::numeric_limits<int>::max())
return;
@@ -769,11 +758,10 @@ computeFreeStackSlots(MachineFrameInfo &MFI, bool StackGrowsDown,
if (MFI.getStackID(i) == TargetStackID::Default)
AllocatedFrameSlots.push_back(i);
// Add callee-save objects if there are any.
- if (MinCSFrameIndex <= MaxCSFrameIndex) {
- for (int i = MinCSFrameIndex; i <= (int)MaxCSFrameIndex; ++i)
- if (MFI.getStackID(i) == TargetStackID::Default)
- AllocatedFrameSlots.push_back(i);
- }
+ for (int i = MFI.getObjectIndexBegin(); i < MFI.getObjectIndexEnd(); i++)
+ if (MFI.isCalleeSavedObjectIndex(i) &&
+ MFI.getStackID(i) == TargetStackID::Default)
+ AllocatedFrameSlots.push_back(i);
for (int i : AllocatedFrameSlots) {
// These are converted from int64_t, but they should always fit in int
@@ -923,20 +911,28 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
Align MaxAlign = MFI.getMaxAlign();
// First assign frame offsets to stack objects that are used to spill
// callee saved registers.
- if (MaxCSFrameIndex >= MinCSFrameIndex) {
- for (unsigned i = 0; i <= MaxCSFrameIndex - MinCSFrameIndex; ++i) {
- unsigned FrameIndex =
- StackGrowsDown ? MinCSFrameIndex + i : MaxCSFrameIndex - i;
-
+ if (StackGrowsDown) {
+ for (int FI = MFI.getObjectIndexBegin(); FI < MFI.getObjectIndexEnd();
+ FI++) {
+ // Only allocate objects on the default stack.
+ if (!MFI.isCalleeSavedObjectIndex(FI) ||
+ MFI.getStackID(FI) != TargetStackID::Default)
+ continue;
+ AdjustStackOffset(MFI, FI, StackGrowsDown, Offset, MaxAlign);
+ }
+ } else {
+ for (int FI = MFI.getObjectIndexEnd() - 1; FI >= MFI.getObjectIndexBegin();
+ FI--) {
// Only allocate objects on the default stack.
- if (MFI.getStackID(FrameIndex) != TargetStackID::Default)
+ if (!MFI.isCalleeSavedObjectIndex(FI) ||
+ MFI.getStackID(FI) != TargetStackID::Default)
continue;
- // TODO: should this just be if (MFI.isDeadObjectIndex(FrameIndex))
- if (!StackGrowsDown && MFI.isDeadObjectIndex(FrameIndex))
+ // TODO: should this just be if (MFI.isDeadObjectIndex(FI))
+ if (MFI.isDeadObjectIndex(FI))
continue;
- AdjustStackOffset(MFI, FrameIndex, StackGrowsDown, Offset, MaxAlign);
+ AdjustStackOffset(MFI, FI, StackGrowsDown, Offset, MaxAlign);
}
}
@@ -1025,7 +1021,7 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
if (MFI.isObjectPreAllocated(i) && MFI.getUseLocalStackAllocationBlock())
continue;
- if (i >= MinCSFrameIndex && i <= MaxCSFrameIndex)
+ if (MFI.isCalleeSavedObjectIndex(i))
continue;
if (RS && RS->isScavengingFrameIndex((int)i))
continue;
@@ -1077,7 +1073,7 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
if (MFI.isObjectPreAllocated(i) && MFI.getUseLocalStackAllocationBlock())
continue;
- if (i >= MinCSFrameIndex && i <= MaxCSFrameIndex)
+ if (MFI.isCalleeSavedObjectIndex(i))
continue;
if (RS && RS->isScavengingFrameIndex((int)i))
continue;
@@ -1113,8 +1109,7 @@ void PEIImpl::calculateFrameObjectOffsets(MachineFunction &MF) {
if (!ObjectsToAllocate.empty() &&
MF.getTarget().getOptLevel() != CodeGenOptLevel::None &&
MFI.getStackProtectorIndex() < 0 && TFI.enableStackSlotScavenging(MF))
- computeFreeStackSlots(MFI, StackGrowsDown, MinCSFrameIndex, MaxCSFrameIndex,
- FixedCSEnd, StackBytesFree);
+ computeFreeStackSlots(MFI, StackGrowsDown, FixedCSEnd, StackBytesFree);
// Now walk the objects and actually assign base offsets to them.
for (auto &Object : ObjectsToAllocate)
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 7290b3f67c2e3..c2f5c0368a782 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -2755,8 +2755,7 @@ void AArch64FrameLowering::determineCalleeSaves(MachineFunction &MF,
bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
MachineFunction &MF, const TargetRegisterInfo *RegInfo,
- std::vector<CalleeSavedInfo> &CSI, unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
+ std::vector<CalleeSavedInfo> &CSI) const {
bool NeedsWinCFI = needsWinCFI(MF);
unsigned StackHazardSize = getStackHazardSize(MF);
// To match the canonical windows frame layout, reverse the list of
@@ -2779,10 +2778,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
if (UsesWinAAPCS && hasFP(MF) && AFI->hasSwiftAsyncContext()) {
int FrameIdx = MFI.CreateStackObject(8, Align(16), true);
AFI->setSwiftAsyncContextFrameIdx(FrameIdx);
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
}
// Insert VG into the list of CSRs, immediately before LR if saved.
@@ -2812,31 +2808,21 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << HazardSlotIndex
<< "\n");
AFI->setStackHazardCSRSlotIndex(HazardSlotIndex);
- if ((unsigned)HazardSlotIndex < MinCSFrameIndex)
- MinCSFrameIndex = HazardSlotIndex;
- if ((unsigned)HazardSlotIndex > MaxCSFrameIndex)
- MaxCSFrameIndex = HazardSlotIndex;
+ MFI.setIsCalleeSavedObjectIndex(HazardSlotIndex, true);
}
unsigned Size = RegInfo->getSpillSize(*RC);
Align Alignment(RegInfo->getSpillAlign(*RC));
int FrameIdx = MFI.CreateStackObject(Size, Alignment, true);
CS.setFrameIdx(FrameIdx);
-
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
// Grab 8 bytes below FP for the extended asynchronous frame info.
if (hasFP(MF) && AFI->hasSwiftAsyncContext() && !UsesWinAAPCS &&
Reg == AArch64::FP) {
FrameIdx = MFI.CreateStackObject(8, Alignment, true);
AFI->setSwiftAsyncContextFrameIdx(FrameIdx);
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
}
LastReg = Reg;
}
@@ -2848,10 +2834,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << HazardSlotIndex
<< "\n");
AFI->setStackHazardCSRSlotIndex(HazardSlotIndex);
- if ((unsigned)HazardSlotIndex < MinCSFrameIndex)
- MinCSFrameIndex = HazardSlotIndex;
- if ((unsigned)HazardSlotIndex > MaxCSFrameIndex)
- MaxCSFrameIndex = HazardSlotIndex;
+ MFI.setIsCalleeSavedObjectIndex(HazardSlotIndex, true);
}
return true;
@@ -2968,9 +2951,8 @@ static SVEStackSizes determineSVEStackSizes(MachineFunction &MF,
}
for (int FI = 0, E = MFI.getObjectIndexEnd(); FI != E; ++FI) {
- if (FI == StackProtectorFI || MFI.isDeadObjectIndex(FI))
- continue;
- if (MaxCSFrameIndex >= FI && FI >= MinCSFrameIndex)
+ if (FI == StackProtectorFI || MFI.isDeadObjectIndex(FI) ||
+ MFI.isCalleeSavedObjectIndex(FI))
continue;
if (MFI.getStackID(FI) != TargetStackID::ScalableVector &&
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.h b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
index 32a9bd831989c..97db18dd30bef 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
@@ -88,11 +88,10 @@ class AArch64FrameLowering : public TargetFrameLowering {
bool hasReservedCallFrame(const MachineFunction &MF) const override;
- bool assignCalleeSavedSpillSlots(MachineFunction &MF,
- const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const override;
+ bool
+ assignCalleeSavedSpillSlots(MachineFunction &MF,
+ const TargetRegisterInfo *TRI,
+ std::vector<CalleeSavedInfo> &CSI) const override;
void determineCalleeSaves(MachineFunction &MF, BitVector &SavedRegs,
RegScavenger *RS) const override;
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index ffbb111d42221..ec3e720ef8887 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -1844,9 +1844,7 @@ void SIFrameLowering::determineCalleeSavesSGPR(MachineFunction &MF,
static void assignSlotsUsingVGPRBlocks(MachineFunction &MF,
const GCNSubtarget &ST,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) {
+ std::vector<CalleeSavedInfo> &CSI) {
SIMachineFunctionInfo *FuncInfo = MF.getInfo<SIMachineFunctionInfo>();
MachineFrameInfo &MFI = MF.getFrameInfo();
const SIRegisterInfo *TRI = ST.getRegisterInfo();
@@ -1915,10 +1913,7 @@ static void assignSlotsUsingVGPRBlocks(MachineFunction &MF,
int FrameIdx =
MFI.CreateStackObject(BlockSize, TRI->getSpillAlign(*BlockRegClass),
/*isSpillSlot=*/true);
- if ((unsigned)FrameIdx < MinCSFrameIndex)
- MinCSFrameIndex = FrameIdx;
- if ((unsigned)FrameIdx > MaxCSFrameIndex)
- MaxCSFrameIndex = FrameIdx;
+ MFI.setIsCalleeSavedObjectIndex(FrameIdx, true);
CSIt->setFrameIdx(FrameIdx);
CSIt->setReg(RegBlock);
@@ -1928,8 +1923,7 @@ static void assignSlotsUsingVGPRBlocks(MachineFunction &MF,
bool SIFrameLowering::assignCalleeSavedSpillSlots(
MachineFunction &MF, const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI, unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
+ std::vector<CalleeSavedInfo> &CSI) const {
if (CSI.empty())
return true; // Early exit if no callee saved registers are modified!
@@ -1937,12 +1931,12 @@ bool SIFrameLowering::assignCalleeSavedSpillSlots(
bool UseVGPRBlocks = ST.useVGPRBlockOpsForCSR();
if (UseVGPRBlocks)
- assignSlotsUsingVGPRBlocks(MF, ST, CSI, MinCSFrameIndex, MaxCSFrameIndex);
+ assignSlotsUsingVGPRBlocks(MF, ST, CSI);
- return assignCalleeSavedSpillSlots(MF, TRI, CSI) || UseVGPRBlocks;
+ return assignCalleeSavedSpillSlotsImpl(MF, TRI, CSI) || UseVGPRBlocks;
}
-bool SIFrameLowering::assignCalleeSavedSpillSlots(
+bool SIFrameLowering::assignCalleeSavedSpillSlotsImpl(
MachineFunction &MF, const TargetRegisterInfo *TRI,
std::vector<CalleeSavedInfo> &CSI) const {
if (CSI.empty())
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.h b/llvm/lib/Target/AMDGPU/SIFrameLowering.h
index a72772987262e..4c1cf3c7f9526 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.h
@@ -49,11 +49,9 @@ class SIFrameLowering final : public AMDGPUFrameLowering {
const TargetRegisterInfo *TRI,
std::vector<CalleeSavedInfo> &CSI) const override;
- bool assignCalleeSavedSpillSlots(MachineFunction &MF,
- const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI,
- unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const override;
+ bool assignCalleeSavedSpillSlotsImpl(MachineFunction &MF,
+ const TargetRegisterInfo *TRI,
+ std::vector<CalleeSavedInfo> &CSI) const;
bool spillCalleeSavedRegisters(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MI,
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 75e7cf347e461..668bb84e72c3b 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -1992,17 +1992,17 @@ RISCVFrameLowering::getFirstSPAdjustAmount(const MachineFunction &MF) const {
bool RISCVFrameLowering::assignCalleeSavedSpillSlots(
MachineFunction &MF, const TargetRegisterInfo *TRI,
- std::vector<CalleeSavedInfo> &CSI, unsigned &MinCSFrameIndex,
- unsigned &MaxCSFrameIndex) const {
+ std::vector<CalleeSavedInfo> &CSI) const {
auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
+ MachineFrameInfo &MFI = MF.getFrameInfo();
+ const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
// Preemptible Interrupts have two additional Callee-save Frame Indexes,
// not tracked by `CSI`.
if (RVFI->isSiFivePreemptibleInterrupt(MF)) {
for (int I = 0; I < 2; ++I) {
int FI = RVFI->getInterruptCSRFrameIndex(I);
- MinCSFrameIndex = std::min<unsigned>(MinCSFrameIndex, FI);
- MaxCSFrameIndex = std::max<unsigned>(MaxCSFrameIndex, FI);
+ MFI.setIsCalleeSavedObjectIndex(FI, true);
}
}
@@ -2028,9 +2028,6 @@ bool RISCVFrameLowering::assignCalleeSavedSpillSlots(
}
}
- MachineFrameInfo &MFI = MF.getFrameInfo();
- const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
-
for (auto &CS : CSI) {
MCRegister Reg = CS.getReg();
const TargetRegisterClass *RC = RegInfo->getMinimalPhysReg...
[truncated]
|
lenary
reviewed
Dec 5, 2025
Comment on lines
931
to
932
| // TODO: should this just be if (MFI.isDeadObjectIndex(FI)) | ||
| if (MFI.isDeadObjectIndex(FI)) |
Member
There was a problem hiding this comment.
This TODO should probably move, as it looks done, but it's actually not.
Collaborator
Author
There was a problem hiding this comment.
Where should it move to? I wasn't clear on what the TODO was actually saying, so I kept it with the code it'd been near.
Member
There was a problem hiding this comment.
I think you might want to rewrite the comment a little and put it where a MFI.isDeadObjectIndex(FI) call would be in the loop in the if (StackGrowsDown) then-block right?
I don't know why we only check for dead objects when the stack grows a specific direction, but I presume there's some boundary condition bug with the logic.
lenary
approved these changes
Dec 5, 2025
Member
lenary
left a comment
lenary
left a comment
There was a problem hiding this comment.
LGTM. This is a nice cleanup that I think makes most of the code clearer. One Nit which i meant to put into this review but posted as a single comment.
MacDue
reviewed
Dec 5, 2025
MacDue
reviewed
Dec 5, 2025
| unsigned FrameIndex = | ||
| StackGrowsDown ? MinCSFrameIndex + i : MaxCSFrameIndex - i; | ||
|
|
||
| if (StackGrowsDown) { |
Member
There was a problem hiding this comment.
Feel free to reject this, but I think you could do something like:
auto AllFIsSeq = seq(MFI.getObjectIndexBegin(), MFI.getObjectIndexEnd());
for (auto [FILo, FIHi] : zip_equal(AllFIsSeq, reverse(AllFIsSeq))) {
int FI = StackGrowsDown ? FILo : FIHi;
// Only allocate objects on the default stack.
if (!MFI.isCalleeSavedObjectIndex(FI) ||
MFI.getStackID(FI) != TargetStackID::Default)
continue;
// TODO: should this just be if (MFI.isDeadObjectIndex(FI))
if (!StackGrowsDown && MFI.isDeadObjectIndex(FI))
continue;
AdjustStackOffset(MFI, FI, StackGrowsDown, Offset, MaxAlign);
}Rather than duplicate the loop for forwards/reverse (IIUC).
Collaborator
Author
There was a problem hiding this comment.
Personally, I'd find that much harder to reason about. The idea of using a sequence is interesting though, maybe we could conditional reverse a sequence instead? I'll play with this a bit and may tag you for review on a followup.
Member
There was a problem hiding this comment.
No worries 👍 You could wrap this trick up as:
template <typename ContainerTy>
[[nodiscard]] auto conditionally_reverse(ContainerTy &&C, bool Reverse) {
return map_range(zip_equal(reverse(C), C), [Reverse](auto I) {
return Reverse ? std::get<0>(I) : std::get<1>(I);
});
}(Probably would fit in llvm/ADT/STLExtras.h)
Which allows for:
auto AllFIs = seq(MFI.getObjectIndexBegin(), MFI.getObjectIndexEnd());
for (auto FI : conditionally_reverse(AllFIs, /*Reverse=*/!StackGrowsDown)) {
// ...
}
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/28218 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/17763 Here is the relevant piece of the build log for the reference |
honeygoyal
pushed a commit
to honeygoyal/llvm-project
that referenced
this pull request
Dec 9, 2025
…CI] (llvm#170905) This removes the tracking of the MinCSFrameIndex, and MaxCSFrameIndex markers, simplifying the target API. This brings the tracking for callee save spill slots in line with how we handle other properties of stack locations. A couple notes: 1) This requires doing scans of the entire object range, but we have other such instances in the code already, so I doubt this will matter in practice. 2) This removes the requirement that callee saved spill slots be contiguous in the frame index identified space. I marked this as NFCI because if prior code violated the contiguous range assumption - I can't find a case where we did - then this change might adjust frame layout in some edge cases. The motivation for this is mostly code readability, but I might use this as a primitive for something in an upcoming patch series around shrink wrapping. Haven't decided yet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the tracking of the MinCSFrameIndex, and MaxCSFrameIndex markers, simplifying the target API. This brings the tracking for callee save spill slots in line with how we handle other properties of stack locations.
A couple notes:
other such instances in the code already, so I doubt this will
matter in practice.
contiguous in the frame index identified space.
I marked this as NFCI because if prior code violated the contiguous range assumption - I can't find a case where we did - then this change might adjust frame layout in some edge cases.
The motivation for this is mostly code readability, but I might use this as a primitive for something in an upcoming patch series around shrink wrapping. Haven't decided yet.