ARMFrameLowering: Reserve emergency spill slot for large arguments

[MatzeB]

Summary

We need to reserve an emergency spill slot in cases with large argument
types that could overflow immediate offsets for FP relative address
calculations.

[MatzeB]

Rename BigFrameIndexes => BigFrameOffsets

[t.p.northover]

lib/Target/ARM/ARMFrameLowering.cpp

340 ↗

(On Diff #93996)

Should this be ArgumentStackSize too? I think ArgRegsSaveSize only accounts for when R0-R3 get pushed on function entry.

[MatzeB]

lib/Target/ARM/ARMFrameLowering.cpp

340 ↗

(On Diff #93996)

No, this is correct, but the getArgumentStackSize() above is an typo (an autocompleto).

The intention here is to capture the displacement between the location the stack pointer pointed to at function begin and the location the frame pointer points to.
The former is used as reference point for the offsets in MachineFrameInfo, the latter is what we actually have in the instructions and encode the immediate values against.

The stack arguments are already part of SP when the functions starts so they are not part of the displacement. It seems the new FP points to wherever the old FP was saved. This means we have to account for stuff before this place:

• Extra space to store register arguments on va_arg functions goes at the beginning.
• Some registers are saved first before saving the old FP. This is usually just LR, but depending on the calling convention may be more, hence this whole strange function giving you a maximum of how many other registers may be saved before.

[MatzeB]

Fix getArgRegsSaveSize() vs. getArgumentStackSize() mixup.

[MatzeB]

ping

[t.p.northover]

OK, sounds reasonable. LGTM.

llvm/trunk/lib/Target/ARM/ARMFrameLowering.cpp

if (!AFI->isThumbFunction()) {
     assert(CanUseBFC);
     BuildMI(MBB, MBBI, DL, TII.get(ARM::t2BFC), Reg)
         .addReg(Reg, RegState::Kill)
         .addImm(~AlignMask)
         .add(predOps(ARMCC::AL));
   }
 }
 
+/// We need the offset of the frame pointer relative to other MachineFrameInfo
+/// offsets which are encoded relative to SP at function begin.
+/// See also emitPrologue() for how the FP is set up.
+/// Unfortunately we cannot determine this value in determineCalleeSaves() yet
+/// as assignCalleeSavedSpillSlots() hasn't run at this point. Instead we use
+/// this to produce a conservative estimate that we check in an assert() later.
+static int getMaxFPOffset(const Function &F, const ARMFunctionInfo &AFI) {
+  // We may end up saving a lot of registers that are higher numbered than the
+  // r7 used for FP in thumb.
+  if (F.hasFnAttribute("interrupt") ||
+      F.getCallingConv() == CallingConv::CXX_FAST_TLS)
+    return -AFI.getArgRegsSaveSize() - (8 * 4);
+
+  // Usually it is just the link register and the FP itself (and in rare cases
+  // r12?) saved to reach FP.
+  return -AFI.getArgRegsSaveSize() - 12;
+}
+
 void ARMFrameLowering::emitPrologue(MachineFunction &MF,
                                     MachineBasicBlock &MBB) const {
   MachineBasicBlock::iterator MBBI = MBB.begin();
   MachineFrameInfo  &MFI = MF.getFrameInfo();
   ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
   MachineModuleInfo &MMI = MF.getMMI();
   MCContext &Context = MMI.getContext();
   const TargetMachine &TM = MF.getTarget();
void ARMFrameLowering::emitPrologue(MachineFunction &MF,
   // Determine starting offsets of spill areas.
   unsigned GPRCS1Offset = NumBytes - ArgRegsSaveSize - GPRCS1Size;
   unsigned GPRCS2Offset = GPRCS1Offset - GPRCS2Size;
   unsigned DPRAlign = DPRCSSize ? std::min(8U, Align) : 4U;
   unsigned DPRGapSize = (GPRCS1Size + GPRCS2Size + ArgRegsSaveSize) % DPRAlign;
   unsigned DPRCSOffset = GPRCS2Offset - DPRGapSize - DPRCSSize;
   int FramePtrOffsetInPush = 0;
   if (HasFP) {
-    FramePtrOffsetInPush =
-        MFI.getObjectOffset(FramePtrSpillFI) + ArgRegsSaveSize;
+    int FPOffset = MFI.getObjectOffset(FramePtrSpillFI);
+    assert(getMaxFPOffset(*MF.getFunction(), *AFI) <= FPOffset &&
+           "Max FP estimation is wrong");
+    FramePtrOffsetInPush = FPOffset + ArgRegsSaveSize;
     AFI->setFramePtrSpillOffset(MFI.getObjectOffset(FramePtrSpillFI) +
                                 NumBytes);
   }
   AFI->setGPRCalleeSavedArea1Offset(GPRCS1Offset);
   AFI->setGPRCalleeSavedArea2Offset(GPRCS2Offset);
   AFI->setDPRCalleeSavedAreaOffset(DPRCSOffset);
 
   // Move past area 2.
void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
   // the function adjusts the stack pointer during execution and the
   // adjustments aren't already part of our stack size estimate, our offset
   // calculations may be off, so be conservative.
   // FIXME: We could add logic to be more precise about negative offsets
   //        and which instructions will need a scratch register for them. Is it
   //        worth the effort and added fragility?
   unsigned EstimatedStackSize =
       MFI.estimateStackSize(MF) + 4 * (NumGPRSpills + NumFPRSpills);
+
+  // Determine biggest (positive) SP offset in MachineFrameInfo.
+  int MaxFixedOffset = 0;
+  for (int I = MFI.getObjectIndexBegin(); I < 0; ++I) {
+    int MaxObjectOffset = MFI.getObjectOffset(I) + MFI.getObjectSize(I);
+    MaxFixedOffset = std::max(MaxFixedOffset, MaxObjectOffset);
+  }
+
   bool HasFP = hasFP(MF);
   if (HasFP) {
     if (AFI->hasStackFrame())
       EstimatedStackSize += 4;
   } else {
     // If FP is not used, SP will be used to access arguments, so count the
     // size of arguments into the estimation.
-    EstimatedStackSize += AFI->getArgumentStackSize();
+    EstimatedStackSize += MaxFixedOffset;
   }
   EstimatedStackSize += 16; // For possible paddings.
 
-  bool BigStack = EstimatedStackSize >= estimateRSStackSizeLimit(MF, this) ||
-                  MFI.hasVarSizedObjects() ||
-                  (MFI.adjustsStack() && !canSimplifyCallFramePseudos(MF));
+  unsigned EstimatedRSStackSizeLimit = estimateRSStackSizeLimit(MF, this);
+  int MaxFPOffset = getMaxFPOffset(*MF.getFunction(), *AFI);
+  bool BigFrameOffsets = EstimatedStackSize >= EstimatedRSStackSizeLimit ||
+    (MFI.adjustsStack() && !canSimplifyCallFramePseudos(MF)) ||
+    // For large argument stacks fp relative addressed may overflow.
+    (HasFP && (MaxFixedOffset - MaxFPOffset) >= (int)EstimatedRSStackSizeLimit);
   bool ExtraCSSpill = false;
-  if (BigStack || !CanEliminateFrame || RegInfo->cannotEliminateFrame(MF)) {
+  if (BigFrameOffsets ||
+      !CanEliminateFrame || RegInfo->cannotEliminateFrame(MF)) {
     AFI->setHasStackFrame(true);
 
     if (HasFP) {
       SavedRegs.set(FramePtr);
       // If the frame pointer is required by the ABI, also spill LR so that we
       // emit a complete frame record.
       if (MF.getTarget().Options.DisableFramePointerElim(MF) && !LRSpilled) {
         SavedRegs.set(ARM::LR);
if (TargetAlign >= 8 && (NumGPRSpills & 1)) {
       }
     }
 
     // Estimate if we might need to scavenge a register at some point in order
     // to materialize a stack offset. If so, either spill one additional
     // callee-saved register or reserve a special spill slot to facilitate
     // register scavenging. Thumb1 needs a spill slot for stack pointer
     // adjustments also, even when the frame itself is small.
-    if (BigStack && !ExtraCSSpill) {
+    if (BigFrameOffsets && !ExtraCSSpill) {
       // If any non-reserved CS register isn't spilled, just spill one or two
       // extra. That should take care of it!
       unsigned NumExtras = TargetAlign / 4;
       SmallVector<unsigned, 2> Extras;
       while (NumExtras && !UnspilledCS1GPRs.empty()) {
         unsigned Reg = UnspilledCS1GPRs.back();
         UnspilledCS1GPRs.pop_back();
         if (!MRI.isReserved(Reg) &&

llvm/trunk/test/CodeGen/ARM/fpoffset_overflow.mir

+# RUN: llc -o - %s -mtriple=thumbv7-- -run-pass=stack-protector -run-pass=prologepilog | FileCheck %s
+---
+# This should trigger an emergency spill in the register scavenger because the
+# frame offset into the large argument is too large.
+# CHECK-LABEL: name: func0
+# CHECK: t2STRi12 killed %r7, %sp, 0, 14, _ :: (store 4 into %stack.0)
+# CHECK: %r7 = t2ADDri killed %sp, 4096, 14, _, _
+# CHECK: %r11 = t2LDRi12 killed %r7, 36, 14, _ :: (load 4)
+# CHECK: %r7 = t2LDRi12 %sp, 0, 14, _ :: (load 4 from %stack.0)
+name: func0
+tracksRegLiveness: true
+fixedStack:
+  - { id: 0, offset: 4084, size: 4, alignment: 4, isImmutable: true,
+      isAliased: false }
+  - { id: 1, offset: -12, size: 4096, alignment: 4, isImmutable: false,
+      isAliased: false }
+body: |
+  bb.0:
+    %r0 = IMPLICIT_DEF
+    %r1 = IMPLICIT_DEF
+    %r2 = IMPLICIT_DEF
+    %r3 = IMPLICIT_DEF
+    %r4 = IMPLICIT_DEF
+    %r5 = IMPLICIT_DEF
+    %r6 = IMPLICIT_DEF
+    %r8 = IMPLICIT_DEF
+    %r9 = IMPLICIT_DEF
+    %r10 = IMPLICIT_DEF
+    %r11 = IMPLICIT_DEF
+    %r12 = IMPLICIT_DEF
+    %lr = IMPLICIT_DEF
+
+    %r11 = t2LDRi12 %fixed-stack.0, 0, 14, _ :: (load 4)
+
+    KILL %r0
+    KILL %r1
+    KILL %r2
+    KILL %r3
+    KILL %r4
+    KILL %r5
+    KILL %r6
+    KILL %r8
+    KILL %r9
+    KILL %r10
+    KILL %r11
+    KILL %r12
+    KILL %lr
+...
+---
+# This should not trigger an emergency spill yet.
+# CHECK-LABEL: name: func1
+# CHECK-NOT: t2STRi12
+# CHECK-NOT: t2ADDri
+# CHECK: %r11 = t2LDRi12 %sp, 4092, 14, _ :: (load 4)
+# CHECK-NOT: t2LDRi12
+name: func1
+tracksRegLiveness: true
+fixedStack:
+  - { id: 0, offset: 4044, size: 4, alignment: 4, isImmutable: true,
+      isAliased: false }
+  - { id: 1, offset: -12, size: 4056, alignment: 4, isImmutable: false,
+      isAliased: false }
+body: |
+  bb.0:
+    %r0 = IMPLICIT_DEF
+    %r1 = IMPLICIT_DEF
+    %r2 = IMPLICIT_DEF
+    %r3 = IMPLICIT_DEF
+    %r4 = IMPLICIT_DEF
+    %r5 = IMPLICIT_DEF
+    %r6 = IMPLICIT_DEF
+    %r8 = IMPLICIT_DEF
+    %r9 = IMPLICIT_DEF
+    %r10 = IMPLICIT_DEF
+    %r11 = IMPLICIT_DEF
+    %r12 = IMPLICIT_DEF
+    %lr = IMPLICIT_DEF
+
+    %r11 = t2LDRi12 %fixed-stack.0, 0, 14, _ :: (load 4)
+
+    KILL %r0
+    KILL %r1
+    KILL %r2
+    KILL %r3
+    KILL %r4
+    KILL %r5
+    KILL %r6
+    KILL %r8
+    KILL %r9
+    KILL %r10
+    KILL %r11
+    KILL %r12
+    KILL %lr
+...