ARM-Darwin: keep the frame register reserved even if not updated.

[t.p.northover]

Summary

Darwin platforms need the frame register to always point at a valid record even if it's not updated in a leaf function. Backtraces are more important than one extra GPR.

The basic change is a patch from way back that caused assertion failures and was reverted. I think I've tracked all of those down to r7 being more available to allow stack realignment, which changes whether hasFP returns true at an unexpected point in a function. The shuffling in ARMFrameLowering is designed to counteract that and ensure a consistent frame-pointer in that case.

[t.p.northover]

Ping.

[t.p.northover]

Ping.

llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp

getReservedRegs(const MachineFunction &MF) const {
   const ARMFrameLowering *TFI = getFrameLowering(MF);
 
   // FIXME: avoid re-calculating this every time.
   BitVector Reserved(getNumRegs());
   markSuperRegs(Reserved, ARM::SP);
   markSuperRegs(Reserved, ARM::PC);
   markSuperRegs(Reserved, ARM::FPSCR);
   markSuperRegs(Reserved, ARM::APSR_NZCV);
-  if (TFI->hasFP(MF))
+  if (TFI->hasFP(MF) || STI.isTargetDarwin())
     markSuperRegs(Reserved, getFramePointerReg(STI));
   if (hasBasePointer(MF))
     markSuperRegs(Reserved, BasePtr);
   // Some targets reserve R9.
   if (STI.isR9Reserved())
     markSuperRegs(Reserved, ARM::R9);
   // Reserve D16-D31 if the subtarget doesn't support them.
   if (!STI.hasD32()) {

llvm/lib/Target/ARM/ARMFrameLowering.cpp

checkNumAlignedDPRCS2Regs(MachineFunction &MF, BitVector &SavedRegs) {
 
   // Don't do this for just one d-register. It's not worth it.
   if (NumSpills < 2)
     return;
 
   // Spill the first NumSpills D-registers after realigning the stack.
   MF.getInfo<ARMFunctionInfo>()->setNumAlignedDPRCS2Regs(NumSpills);
 
+  // Mark the stack as being realigned so later code knows a frame pointer
+  // is needed.
+  MF.getFrameInfo().ensureMaxAlignment(16);
+
   // A scratch register is required for the vst1 / vld1 instructions.
   SavedRegs.set(ARM::R4);
 }
 
 bool ARMFrameLowering::enableShrinkWrapping(const MachineFunction &MF) const {
   // For CMSE entry functions, we want to save the FPCXT_NS immediately
   // upon function entry (resp. restore it immmediately before return)
   if (STI.hasV8_1MMainlineOps() &&
const ARMBaseRegisterInfo *RegInfo = static_cast<const ARMBaseRegisterInfo *>(
       MF.getSubtarget().getRegisterInfo());
   const ARMBaseInstrInfo &TII =
       *static_cast<const ARMBaseInstrInfo *>(MF.getSubtarget().getInstrInfo());
   ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
   (void)TRI;  // Silence unused warning in non-assert builds.
-  Register FramePtr = RegInfo->getFrameRegister(MF);
 
   // Spill R4 if Thumb2 function requires stack realignment - it will be used as
   // scratch register. Also spill R4 if Thumb2 function has varsized objects,
   // since it's not always possible to restore sp from fp in a single
   // instruction.
   // FIXME: It will be better just to find spare register here.
   if (AFI->isThumb2Function() &&
       (MFI.hasVarSizedObjects() || RegInfo->needsStackRealignment(MF)))
LLVM_DEBUG(dbgs() << "EstimatedLimit: " << EstimatedRSStackSizeLimit
                     << "; EstimatedStack: " << EstimatedStackSize
                     << "; EstimatedFPStack: " << MaxFixedOffset - MaxFPOffset
                     << "; BigFrameOffsets: " << BigFrameOffsets << "\n");
   if (BigFrameOffsets ||
       !CanEliminateFrame || RegInfo->cannotEliminateFrame(MF)) {
     AFI->setHasStackFrame(true);
 
     if (HasFP) {
+      Register FramePtr = RegInfo->getFrameRegister(MF);
       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);
         LRSpilled = true;
         NumGPRSpills++;
         auto LRPos = llvm::find(UnspilledCS1GPRs, ARM::LR);

llvm/test/CodeGen/ARM/machine-outliner-thunk.ll

 ; THUMB:        $r0, dead $cpsr = tMOVi8 1, 14 /* CC::al */, $noreg
 ; THUMB-NEXT:   $r1, dead $cpsr = tMOVi8 2, 14 /* CC::al */, $noreg
 ; THUMB-NEXT:   $r2, dead $cpsr = tMOVi8 3, 14 /* CC::al */, $noreg
 ; THUMB-NEXT:   $r3, dead $cpsr = tMOVi8 4, 14 /* CC::al */, $noreg
 ; THUMB-NEXT:   tTAILJMPdND @thunk_called_fn, 14 /* CC::al */, $noreg, implicit $sp
 
 ; MACHO-LABEL: name:            OUTLINED_FUNCTION_0
 ; MACHO:        bb.0:
-; MACHO-NEXT:   liveins: $r7, $r6, $r5, $r4, $r11, $r10, $r8, $d15, $d14, $d13, $d12, $d11, $d10, $d9, $d8
+; MACHO-NEXT:   liveins: $r6, $r5, $r4, $r11, $r10, $r8, $d15, $d14, $d13, $d12, $d11, $d10, $d9, $d8
 ; MACHO:        $r0, dead $cpsr = tMOVi8 1, 14 /* CC::al */, $noreg
 ; MACHO-NEXT:   $r1, dead $cpsr = tMOVi8 2, 14 /* CC::al */, $noreg
 ; MACHO-NEXT:   $r2, dead $cpsr = tMOVi8 3, 14 /* CC::al */, $noreg
 ; MACHO-NEXT:   $r3, dead $cpsr = tMOVi8 4, 14 /* CC::al */, $noreg
 ; MACHO-NEXT:   tTAILJMPd @thunk_called_fn, 14 /* CC::al */, $noreg, implicit $sp
 
 attributes #0 = { minsize optsize }

llvm/test/CodeGen/ARM/r7-fixed-darwin.ll

+; RUN: llc -mtriple=thumbv7k-apple-watchos %s -o - | FileCheck %s
+
+; r7 is FP on Darwin, and should be preserved even if we don't create a new
+; frame record for this leaf function. So make huge register pressure to try &
+; tempt LLVM to use it.
+define void @foo([16 x i32]* %ptr) {
+; CHECK-LABEL: foo:
+; CHECK: push.w
+; CHECK: .cfi_offset r7
+; CHECK-NOT: r7
+; CHECK: pop.w
+  %val = load volatile [16 x i32], [16 x i32]* %ptr
+  store volatile [16 x i32] %val, [16 x i32]* %ptr
+  ret void
+}

llvm/test/CodeGen/ARM/realign-adds-fp.ll

+; RUN: llc -mtriple=armv7-apple-ios %s -o - | FileCheck %s
+
+; Realigning the stack pointer to push DPRs should trigger the creation of a
+; proper frame pointer, but previously we didn't push r7 here.
+
+; CHECK-LABEL: foo:
+; CHECK: push {r4, r7, lr}
+; CHECK: add r7, sp, #4
+define void @foo() "frame-pointer"="non-leaf" {
+  tail call void asm sideeffect "", "~{d8},~{d9},~{d10}"()
+  ret void
+}

llvm/test/CodeGen/Thumb/long.ll

if.end:
   %b = add i64 %y, 45
   call void @f13(i64 %b)
 ; CHECK: adds
 ; CHECK: adcs
 ; CHECK: bl
   %c = add i64 %y, 47
   call void @f13(i64 %c)
 ; CHECK: adds
-; CHECK-NEXT: adcs
+; CHECK: adcs
 ; CHECK: bl
   ret void
 }
 
 attributes #0 = { optsize }

llvm/test/CodeGen/Thumb2/2010-03-15-AsmCCClobber.ll

 %s3 = type { %s2, i32, i32, i32*, [4 x i8], float, %s4, i8*, i8* }
 %s4 = type { %s5 }
 %s5 = type { i32 }
 
 ; Make sure the cmp is not scheduled before the InlineAsm that clobbers cc.
 ; CHECK: bl _f2
 ; CHECK: clz {{r[0-9]+}}
 ; CHECK-DAG: lsrs    {{r[0-9]+}}
-; CHECK-DAG: lsls    {{r[0-9]+}}
+; CHECK-DAG: lsl.w    {{r[0-9]+}}
 ; CHECK-NEXT: orr.w   {{r[0-9]+}}
 ; CHECK-NEXT: InlineAsm Start
 define void @test(%s1* %this, i32 %format, i32 %w, i32 %h, i32 %levels, i32* %s, i8* %data, i32* nocapture %rowbytes, void (i8*, i8*)* %release, i8* %info) nounwind {
 entry:
   %tmp1 = getelementptr inbounds %s1, %s1* %this, i32 0, i32 0, i32 0, i32 1, i32 0, i32 0
   store volatile i32 1, i32* %tmp1, align 4
   %tmp12 = getelementptr inbounds %s1, %s1* %this, i32 0, i32 1
   store i32 %levels, i32* %tmp12, align 4