Skip to content

[CodeGen] Replace (Min,Max)CSFrameIndex with flag on frame object [NFCI] #170905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
preames merged 3 commits into from
Dec 6, 2025
Merged

[CodeGen] Replace (Min,Max)CSFrameIndex with flag on frame object [NFCI] #170905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac1dc21
[CodeGen] Replace (Min,Max)CSFrameIndex with flag on frame object [NFCI]
preames Dec 4, 2025
2fefa58
Move a comment
preames Dec 5, 2025
28b4171
Merge branch 'main' into pr-callee-saved-frame-object-tracking
preames Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions llvm/include/llvm/CodeGen/MachineFrameInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions llvm/include/llvm/CodeGen/TargetFrameLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
69 changes: 32 additions & 37 deletions llvm/lib/CodeGen/PrologEpilogInserter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Copy link
Member

@MacDue MacDue Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator Author

@preames preames Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@MacDue MacDue Dec 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)) {
  // ...
}

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;
// TODO: should we be using MFI.isDeadObjectIndex(FI) here?
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))
if (MFI.isDeadObjectIndex(FI))
continue;

AdjustStackOffset(MFI, FrameIndex, StackGrowsDown, Offset, MaxAlign);
AdjustStackOffset(MFI, FI, StackGrowsDown, Offset, MaxAlign);
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
34 changes: 8 additions & 26 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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 &&
Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 6 additions & 12 deletions llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -1928,21 +1923,20 @@ 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!

const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
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())
Expand Down
8 changes: 3 additions & 5 deletions llvm/lib/Target/AMDGPU/SIFrameLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading