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

preames · web-flow · commit 49a7772be5bf · 2025-12-06T14:00:55.000-08:00
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.
diff --git 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
@@ -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
@@ -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;
+      // 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);
     }
   }
 
@@ -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
@@ -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
@@ -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
@@ -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,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())
diff --git 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
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.h b/llvm/lib/Target/RISCV/RISCVFrameLowering.h
