From e54f6dca5074fe99d913ae1ce9bc43cf7a856b10 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Sun, 1 Dec 2013 14:16:24 +0000 Subject: [PATCH] ARM: fix bug in -Oz stack adjustment folding Previously, we clobbered callee-saved registers when folding an "add sp, #N" into a "pop {rD, ...}" instruction. This change checks whether a register we're going to add to the "pop" could actually be live outside the function before doing so and should fix the issue. This should fix PR18081. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@196046 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/ARMBaseInstrInfo.cpp | 25 ++++++++++++++++++------- lib/Target/ARM/ARMBaseRegisterInfo.h | 8 ++++++++ lib/Target/ARM/ARMFrameLowering.cpp | 7 ------- lib/Target/ARM/Thumb1FrameLowering.cpp | 7 ------- test/CodeGen/ARM/fold-stack-adjust.ll | 4 ++-- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/lib/Target/ARM/ARMBaseInstrInfo.cpp b/lib/Target/ARM/ARMBaseInstrInfo.cpp index dc3e81a1136..20fdcddccbc 100644 --- a/lib/Target/ARM/ARMBaseInstrInfo.cpp +++ b/lib/Target/ARM/ARMBaseInstrInfo.cpp @@ -1909,29 +1909,40 @@ bool llvm::tryFoldSPUpdateIntoPushPop(MachineFunction &MF, 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 in the POP. RegList.push_back(MachineOperand::CreateReg(CurReg, true, false, false, true)); + --RegsNeeded; } if (RegsNeeded > 0) diff --git a/lib/Target/ARM/ARMBaseRegisterInfo.h b/lib/Target/ARM/ARMBaseRegisterInfo.h index 0d4f54f8917..e28fff68f4e 100644 --- a/lib/Target/ARM/ARMBaseRegisterInfo.h +++ b/lib/Target/ARM/ARMBaseRegisterInfo.h @@ -72,6 +72,14 @@ static inline bool isARMArea3Register(unsigned Reg, bool isIOS) { } } +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; diff --git a/lib/Target/ARM/ARMFrameLowering.cpp b/lib/Target/ARM/ARMFrameLowering.cpp index 7b02803c51f..3e72d3690aa 100644 --- a/lib/Target/ARM/ARMFrameLowering.cpp +++ b/lib/Target/ARM/ARMFrameLowering.cpp @@ -82,13 +82,6 @@ ARMFrameLowering::canSimplifyCallFramePseudos(const MachineFunction &MF) const { 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) { diff --git a/lib/Target/ARM/Thumb1FrameLowering.cpp b/lib/Target/ARM/Thumb1FrameLowering.cpp index d921c82cfb0..cfb33f5b821 100644 --- a/lib/Target/ARM/Thumb1FrameLowering.cpp +++ b/lib/Target/ARM/Thumb1FrameLowering.cpp @@ -215,13 +215,6 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF) const { 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() && diff --git a/test/CodeGen/ARM/fold-stack-adjust.ll b/test/CodeGen/ARM/fold-stack-adjust.ll index c8c48faffb2..8bda7683f90 100644 --- a/test/CodeGen/ARM/fold-stack-adjust.ll +++ b/test/CodeGen/ARM/fold-stack-adjust.ll @@ -15,7 +15,7 @@ define void @check_simple() minsize { ; 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} @@ -23,7 +23,7 @@ define void @check_simple() minsize { ; 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.