[PATCH] ARM: fix Oz sp-adjust folding to respect callee-saved registers

[Tim Northover]

Oops. It seems the original commit was almost guaranteed to break on any code it applied to since it clobbered callee-saved registers with gleeful abandon.

In order to still be useful, a slight refactoring also allows registers to be skipped from the list if the instruction allows. This permits useful lists like {r0, r1, r2, r3, pc} to be used.
    
This should fix http://llvm.org/bugs/show_bug.cgi?id=18081.

http://llvm-reviews.chandlerc.com/D2285

Files:
  lib/Target/ARM/ARMBaseInstrInfo.cpp
  lib/Target/ARM/ARMBaseRegisterInfo.h
  lib/Target/ARM/ARMFrameLowering.cpp
  lib/Target/ARM/Thumb1FrameLowering.cpp
  test/CodeGen/ARM/fold-stack-adjust.ll

Index: lib/Target/ARM/ARMBaseInstrInfo.cpp
===================================================================
--- lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -1909,29 +1909,40 @@
 
   MachineBasicBlock *MBB = MI->getParent();
   const TargetRegisterInfo *TRI = MF.getRegInfo().getTargetRegisterInfo();
+  const MCPhysReg *CSRegs = TRI->getCalleeSavedRegs(&MF);
 
   // Now try to find enough space in the reglist to allocate NumBytes.
   for (unsigned CurReg = FirstReg - 1; CurReg >= RD0Reg && RegsNeeded;
-       --CurReg, --RegsNeeded) {
+       --CurReg) {
     if (!IsPop) {
       // Pushing any register is completely harmless, mark the
       // register involved as undef since we don't care about it in
       // the slightest.
       RegList.push_back(MachineOperand::CreateReg(CurReg, false, false,
                                                   false, false, true));
+      --RegsNeeded;
       continue;
     }
 
-    // However, we can only pop an extra register if it's not live. Otherwise we
-    // might clobber a return value register. We assume that once we find a live
-    // return register all lower ones will be too so there's no use proceeding.
-    if (MBB->computeRegisterLiveness(TRI, CurReg, MI) !=
-        MachineBasicBlock::LQR_Dead)
-      return false;
+    // However, we can only pop an extra register if it's not live. For
+    // registers live within the function we might clobber a return value
+    // register; the other way a register can be live here is if it's
+    // callee-saved.
+    if (isCalleeSavedRegister(CurReg, CSRegs) ||
+        MBB->computeRegisterLiveness(TRI, CurReg, MI) !=
+            MachineBasicBlock::LQR_Dead) {
+      // VFP pops don't allow holes in the register list, so any skip is fatal
+      // for our transformation. GPR pops do, so we should just keep looking.
+      if (IsVFPPushPop)
+        return false;
+      else
+        continue;
+    }
 
     // Mark the unimportant registers as <def,dead> in the POP.
     RegList.push_back(MachineOperand::CreateReg(CurReg, true, false, false,
                                                 true));
+    --RegsNeeded;
   }
 
   if (RegsNeeded > 0)
Index: lib/Target/ARM/ARMBaseRegisterInfo.h
===================================================================
--- lib/Target/ARM/ARMBaseRegisterInfo.h
+++ lib/Target/ARM/ARMBaseRegisterInfo.h
@@ -72,6 +72,14 @@
   }
 }
 
+static inline bool isCalleeSavedRegister(unsigned Reg,
+                                         const MCPhysReg *CSRegs) {
+  for (unsigned i = 0; CSRegs[i]; ++i)
+    if (Reg == CSRegs[i])
+      return true;
+  return false;
+}
+
 class ARMBaseRegisterInfo : public ARMGenRegisterInfo {
 protected:
   const ARMSubtarget &STI;
Index: lib/Target/ARM/ARMFrameLowering.cpp
===================================================================
--- lib/Target/ARM/ARMFrameLowering.cpp
+++ lib/Target/ARM/ARMFrameLowering.cpp
@@ -82,13 +82,6 @@
   return hasReservedCallFrame(MF) || MF.getFrameInfo()->hasVarSizedObjects();
 }
 
-static bool isCalleeSavedRegister(unsigned Reg, const uint16_t *CSRegs) {
-  for (unsigned i = 0; CSRegs[i]; ++i)
-    if (Reg == CSRegs[i])
-      return true;
-  return false;
-}
-
 static bool isCSRestore(MachineInstr *MI,
                         const ARMBaseInstrInfo &TII,
                         const uint16_t *CSRegs) {
Index: lib/Target/ARM/Thumb1FrameLowering.cpp
===================================================================
--- lib/Target/ARM/Thumb1FrameLowering.cpp
+++ lib/Target/ARM/Thumb1FrameLowering.cpp
@@ -215,13 +215,6 @@
     AFI->setShouldRestoreSPFromFP(true);
 }
 
-static bool isCalleeSavedRegister(unsigned Reg, const uint16_t *CSRegs) {
-  for (unsigned i = 0; CSRegs[i]; ++i)
-    if (Reg == CSRegs[i])
-      return true;
-  return false;
-}
-
 static bool isCSRestore(MachineInstr *MI, const uint16_t *CSRegs) {
   if (MI->getOpcode() == ARM::tLDRspi &&
       MI->getOperand(1).isFI() &&
Index: test/CodeGen/ARM/fold-stack-adjust.ll
===================================================================
--- test/CodeGen/ARM/fold-stack-adjust.ll
+++ test/CodeGen/ARM/fold-stack-adjust.ll
@@ -15,15 +15,15 @@
 ; CHECK-NOT: sub sp, sp,
 ; ...
 ; CHECK-NOT: add sp, sp,
-; CHECK: pop.w {r7, r8, r9, r10, r11, pc}
+; CHECK: pop.w {r0, r1, r2, r3, r11, pc}
 
 ; CHECK-T1-LABEL: check_simple:
 ; CHECK-T1: push {r3, r4, r5, r6, r7, lr}
 ; CHECK-T1: add r7, sp, #16
 ; CHECK-T1-NOT: sub sp, sp,
 ; ...
 ; CHECK-T1-NOT: add sp, sp,
-; CHECK-T1: pop {r3, r4, r5, r6, r7, pc}
+; CHECK-T1: pop {r0, r1, r2, r3, r7, pc}
 
   ; iOS always has a frame pointer and messing with the push affects
   ; how it's set in the prologue. Make sure we get that right.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2285.1.patch
Type: text/x-patch
Size: 4748 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131128/2dc23b0c/attachment.bin>