[ARM] Fix for MVE narrow load/store stack accesses

[dmgreen]

Summary

Certain MVE instructions, such as the narrowing loads, can be created with frame indices even though they do not accept sp as a register. This makes sure that if we have situations like that, we have an emergency register to use as the frame base, to ensure that we do not run out of registers.

[samparker]

So with D67489, what situations could still generate these addressing modes?

[dmgreen]

The AddrModeT2_i7, etc can still be generated from the non-widening forms of the MVE loads/stores (as they accept a GPR not a tGPR they can take SP too). I don't think the narrowing loads (the ones that cannot accept a stack pointer) will be generated any more, but I'd prefer to fix these cases nonetheless. They may change in the future and these failures are silent until the function gets complex enough to trigger them, so can be difficult to test for.

[samparker]

So would it not be worth adding a register class like GPRnosp too?

[dmgreen]

The widening/narrowing MVE loads and stores, like MVE_VLDRBU32 (the B with the 32, in this example) can only take "thumb" registers as they only have 3 bits for the Rn operand. Which is what I meant by "can't take SP". This is not something that any other stack load/store has had to deal with in the past, and hence RegClass->contains(ARM::SP) is added here to check for such cases. Similar to https://reviews.llvm.org/D66285.

The other MVE loads/stores, like a MVE_VLDRBU8 do have the full 4 bits needed for "Thumb2" register, so do not hit the same problem. They can accept a SP and it will only be a problem for frame indices if the offset is out of range of the instruction.

I've added another test for that second case.

[samparker]

Gotcha, thanks for explaining.

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

static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
   return FnSize;
 }
 
 /// estimateRSStackSizeLimit - Look at each instruction that references stack
 /// frames and return the stack size limit beyond which some of these
 /// instructions will require a scratch register during their expansion later.
 // FIXME: Move to TII?
 static unsigned estimateRSStackSizeLimit(MachineFunction &MF,
-                                         const TargetFrameLowering *TFI) {
+                                         const TargetFrameLowering *TFI,
+                                         bool &HasNonSPFrameIndex) {
   const ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
+  const ARMBaseInstrInfo &TII =
+      *static_cast<const ARMBaseInstrInfo *>(MF.getSubtarget().getInstrInfo());
+  const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
   unsigned Limit = (1 << 12) - 1;
   for (auto &MBB : MF) {
     for (auto &MI : MBB) {
       for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
         if (!MI.getOperand(i).isFI())
           continue;
 
         // When using ADDri to get the address of a stack object, 255 is the
         // largest offset guaranteed to fit in the immediate offset.
         if (MI.getOpcode() == ARM::ADDri) {
           Limit = std::min(Limit, (1U << 8) - 1);
           break;
         }
 
+        const MCInstrDesc &MCID = MI.getDesc();
+        const TargetRegisterClass *RegClass = TII.getRegClass(MCID, i, TRI, MF);
+        if (RegClass && !RegClass->contains(ARM::SP))
+          HasNonSPFrameIndex = true;
+
         // Otherwise check the addressing mode.
         switch (MI.getDesc().TSFlags & ARMII::AddrModeMask) {
         case ARMII::AddrMode3:
         case ARMII::AddrModeT2_i8:
           Limit = std::min(Limit, (1U << 8) - 1);
           break;
         case ARMII::AddrMode5:
         case ARMII::AddrModeT2_i8s4:
         case ARMII::AddrModeT2_ldrex:
           Limit = std::min(Limit, ((1U << 8) - 1) * 4);
           break;
         case ARMII::AddrModeT2_i12:
           // i12 supports only positive offset so these will be converted to
           // i8 opcodes. See llvm::rewriteT2FrameIndex.
           if (TFI->hasFP(MF) && AFI->hasStackFrame())
             Limit = std::min(Limit, (1U << 8) - 1);
           break;
         case ARMII::AddrMode4:
         case ARMII::AddrMode6:
           // Addressing modes 4 & 6 (load/store) instructions can't encode an
           // immediate offset for stack references.
           return 0;
+        case ARMII::AddrModeT2_i7:
+          Limit = std::min(Limit, ((1U << 7) - 1) * 1);
+          break;
+        case ARMII::AddrModeT2_i7s2:
+          Limit = std::min(Limit, ((1U << 7) - 1) * 2);
+          break;
+        case ARMII::AddrModeT2_i7s4:
+          Limit = std::min(Limit, ((1U << 7) - 1) * 4);
+          break;
         default:
           break;
         }
         break; // At most one FI per instruction
       }
     }
   }
 
void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
   } else {
     // If FP is not used, SP will be used to access arguments, so count the
     // size of arguments into the estimation.
     EstimatedStackSize += MaxFixedOffset;
   }
   EstimatedStackSize += 16; // For possible paddings.
 
   unsigned EstimatedRSStackSizeLimit, EstimatedRSFixedSizeLimit;
+  bool HasNonSPFrameIndex = false;
   if (AFI->isThumb1OnlyFunction()) {
     // For Thumb1, don't bother to iterate over the function. The only
     // instruction that requires an emergency spill slot is a store to a
     // frame index.
     //
     // tSTRspi, which is used for sp-relative accesses, has an 8-bit unsigned
     // immediate. tSTRi, which is used for bp- and fp-relative accesses, has
     // a 5-bit unsigned immediate.
     //
     // We could try to check if the function actually contains a tSTRspi
     // that might need the spill slot, but it's not really important.
     // Functions with VLAs or extremely large call frames are rare, and
     // if a function is allocating more than 1KB of stack, an extra 4-byte
     // slot probably isn't relevant.
     if (RegInfo->hasBasePointer(MF))
       EstimatedRSStackSizeLimit = (1U << 5) * 4;
     else
       EstimatedRSStackSizeLimit = (1U << 8) * 4;
     EstimatedRSFixedSizeLimit = (1U << 5) * 4;
   } else {
-    EstimatedRSStackSizeLimit = estimateRSStackSizeLimit(MF, this);
+    EstimatedRSStackSizeLimit =
+        estimateRSStackSizeLimit(MF, this, HasNonSPFrameIndex);
     EstimatedRSFixedSizeLimit = EstimatedRSStackSizeLimit;
   }
   // Final estimate of whether sp or bp-relative accesses might require
   // scavenging.
   bool HasLargeStack = EstimatedStackSize > EstimatedRSStackSizeLimit;
 
   // If the stack pointer moves and we don't have a base pointer, the
   // estimate logic doesn't work. The actual offsets might be larger when
void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
   //
   // We could do slightly better on Thumb1; in some cases, an sp-relative
   // offset would be legal even though an fp-relative offset is not.
   int MaxFPOffset = getMaxFPOffset(MF.getFunction(), *AFI);
   bool HasLargeArgumentList =
       HasFP && (MaxFixedOffset - MaxFPOffset) > (int)EstimatedRSFixedSizeLimit;
 
   bool BigFrameOffsets = HasLargeStack || !HasBPOrFixedSP ||
-                         HasLargeArgumentList;
+                         HasLargeArgumentList || HasNonSPFrameIndex;
   LLVM_DEBUG(dbgs() << "EstimatedLimit: " << EstimatedRSStackSizeLimit
-                    << "; EstimatedStack" << EstimatedStackSize
-                    << "; EstimatedFPStack" << MaxFixedOffset - MaxFPOffset
-                    << "; BigFrameOffsets: " << BigFrameOffsets
-                    << "\n");
+                    << "; EstimatedStack: " << EstimatedStackSize
+                    << "; EstimatedFPStack: " << MaxFixedOffset - MaxFPOffset
+                    << "; BigFrameOffsets: " << BigFrameOffsets << "\n");
   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.

llvm/trunk/test/CodeGen/Thumb2/mve-stacksplot.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -o - %s -mtriple=thumbv8.1m.main-arm-none-eabi -mattr=+mve.fp -run-pass=stack-protector -run-pass=prologepilog | FileCheck %s
+---
+name: func0
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: '', type: default, offset: 0, size: 16, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -16, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: func0
+    ; CHECK: liveins: $r4, $r5, $r6, $r7, $r8, $r9, $r10, $r11, $lr
+    ; CHECK: $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r4, killed $r5, killed $r6, killed $r7, killed $r8, killed $r9, killed $r10, killed $r11, killed $lr
+    ; CHECK: frame-setup CFI_INSTRUCTION def_cfa_offset 36
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $lr, -4
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r11, -8
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r10, -12
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r9, -16
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r8, -20
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r7, -24
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r6, -28
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r5, -32
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r4, -36
+    ; CHECK: $sp = frame-setup tSUBspi $sp, 5, 14, $noreg
+    ; CHECK: frame-setup CFI_INSTRUCTION def_cfa_offset 56
+    ; CHECK: $r0 = IMPLICIT_DEF
+    ; CHECK: $r1 = IMPLICIT_DEF
+    ; CHECK: $r2 = IMPLICIT_DEF
+    ; CHECK: $r3 = IMPLICIT_DEF
+    ; CHECK: $r4 = IMPLICIT_DEF
+    ; CHECK: $r5 = IMPLICIT_DEF
+    ; CHECK: $r6 = IMPLICIT_DEF
+    ; CHECK: $r7 = IMPLICIT_DEF
+    ; CHECK: $r8 = IMPLICIT_DEF
+    ; CHECK: $r9 = IMPLICIT_DEF
+    ; CHECK: $r10 = IMPLICIT_DEF
+    ; CHECK: $r11 = IMPLICIT_DEF
+    ; CHECK: $r12 = IMPLICIT_DEF
+    ; CHECK: $lr = IMPLICIT_DEF
+    ; CHECK: t2STRi12 killed $r0, $sp, 0, 14, $noreg :: (store 4 into %stack.1)
+    ; CHECK: $r0 = tMOVr killed $sp, 14, $noreg
+    ; CHECK: renamable $q2 = MVE_VLDRBU32 killed $r0, 16, 0, $noreg :: (load 4 from %stack.0 + 12)
+    ; CHECK: $r0 = t2LDRi12 $sp, 0, 14, $noreg :: (load 4 from %stack.1)
+    ; CHECK: KILL $r0
+    ; CHECK: KILL $r1
+    ; CHECK: KILL $r2
+    ; CHECK: KILL $r3
+    ; CHECK: KILL $r4
+    ; CHECK: KILL $r5
+    ; CHECK: KILL $r6
+    ; CHECK: KILL $r7
+    ; CHECK: KILL $r8
+    ; CHECK: KILL $r9
+    ; CHECK: KILL $r10
+    ; CHECK: KILL $r11
+    ; CHECK: KILL $r12
+    ; CHECK: KILL $lr
+    $r0 = IMPLICIT_DEF
+    $r1 = IMPLICIT_DEF
+    $r2 = IMPLICIT_DEF
+    $r3 = IMPLICIT_DEF
+    $r4 = IMPLICIT_DEF
+    $r5 = IMPLICIT_DEF
+    $r6 = IMPLICIT_DEF
+    $r7 = IMPLICIT_DEF
+    $r8 = IMPLICIT_DEF
+    $r9 = IMPLICIT_DEF
+    $r10 = IMPLICIT_DEF
+    $r11 = IMPLICIT_DEF
+    $r12 = IMPLICIT_DEF
+    $lr = IMPLICIT_DEF
+
+    renamable $q2 = MVE_VLDRBU32 %stack.0, 12, 0, $noreg :: (load 4 from %stack.0 + 12)
+
+    KILL $r0
+    KILL $r1
+    KILL $r2
+    KILL $r3
+    KILL $r4
+    KILL $r5
+    KILL $r6
+    KILL $r7
+    KILL $r8
+    KILL $r9
+    KILL $r10
+    KILL $r11
+    KILL $r12
+    KILL $lr
+...
+---
+name: func1
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: '', type: default, offset: 0, size: 16, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -1200, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+  - { id: 1, name: '', type: default, offset: 0, size: 1200, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -1200, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: func1
+    ; CHECK: liveins: $r4, $r5, $r6, $r7, $r8, $r9, $r10, $r11, $lr
+    ; CHECK: $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r4, killed $r5, killed $r6, killed $r7, killed $r8, killed $r9, killed $r10, killed $r11, killed $lr
+    ; CHECK: frame-setup CFI_INSTRUCTION def_cfa_offset 36
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $lr, -4
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r11, -8
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r10, -12
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r9, -16
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r8, -20
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r7, -24
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r6, -28
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r5, -32
+    ; CHECK: frame-setup CFI_INSTRUCTION offset $r4, -36
+    ; CHECK: $sp = frame-setup t2SUBri killed $sp, 1216, 14, $noreg, $noreg
+    ; CHECK: $sp = frame-setup tSUBspi $sp, 1, 14, $noreg
+    ; CHECK: frame-setup CFI_INSTRUCTION def_cfa_offset 1256
+    ; CHECK: $r0 = IMPLICIT_DEF
+    ; CHECK: $r1 = IMPLICIT_DEF
+    ; CHECK: $r2 = IMPLICIT_DEF
+    ; CHECK: $r3 = IMPLICIT_DEF
+    ; CHECK: $r4 = IMPLICIT_DEF
+    ; CHECK: $r5 = IMPLICIT_DEF
+    ; CHECK: $r6 = IMPLICIT_DEF
+    ; CHECK: $r7 = IMPLICIT_DEF
+    ; CHECK: $r8 = IMPLICIT_DEF
+    ; CHECK: $r9 = IMPLICIT_DEF
+    ; CHECK: $r10 = IMPLICIT_DEF
+    ; CHECK: $r11 = IMPLICIT_DEF
+    ; CHECK: $r12 = IMPLICIT_DEF
+    ; CHECK: $lr = IMPLICIT_DEF
+    ; CHECK: t2STRi12 killed $r0, $sp, 0, 14, $noreg :: (store 4 into %stack.2)
+    ; CHECK: $r0 = t2ADDri killed $sp, 1152, 14, $noreg, $noreg
+    ; CHECK: renamable $q2 = MVE_VLDRBU8 killed $r0, 52, 0, $noreg :: (load 4 from %stack.0)
+    ; CHECK: $r0 = t2LDRi12 $sp, 0, 14, $noreg :: (load 4 from %stack.2)
+    ; CHECK: KILL $r0
+    ; CHECK: KILL $r1
+    ; CHECK: KILL $r2
+    ; CHECK: KILL $r3
+    ; CHECK: KILL $r4
+    ; CHECK: KILL $r5
+    ; CHECK: KILL $r6
+    ; CHECK: KILL $r7
+    ; CHECK: KILL $r8
+    ; CHECK: KILL $r9
+    ; CHECK: KILL $r10
+    ; CHECK: KILL $r11
+    ; CHECK: KILL $r12
+    ; CHECK: KILL $lr
+    $r0 = IMPLICIT_DEF
+    $r1 = IMPLICIT_DEF
+    $r2 = IMPLICIT_DEF
+    $r3 = IMPLICIT_DEF
+    $r4 = IMPLICIT_DEF
+    $r5 = IMPLICIT_DEF
+    $r6 = IMPLICIT_DEF
+    $r7 = IMPLICIT_DEF
+    $r8 = IMPLICIT_DEF
+    $r9 = IMPLICIT_DEF
+    $r10 = IMPLICIT_DEF
+    $r11 = IMPLICIT_DEF
+    $r12 = IMPLICIT_DEF
+    $lr = IMPLICIT_DEF
+
+    renamable $q2 = MVE_VLDRBU8 %stack.0, 0, 0, $noreg :: (load 4 from %stack.0)
+
+    KILL $r0
+    KILL $r1
+    KILL $r2
+    KILL $r3
+    KILL $r4
+    KILL $r5
+    KILL $r6
+    KILL $r7
+    KILL $r8
+    KILL $r9
+    KILL $r10
+    KILL $r11
+    KILL $r12
+    KILL $lr
+...