From 3bd9b95b4def1d09c16d7ebb2d685a843d552f1d Mon Sep 17 00:00:00 2001 From: Evan Cheng Date: Tue, 7 Dec 2010 23:08:38 +0000 Subject: [PATCH] Fix a bad prologue / epilogue codegen bug where the compiler would emit illegal vpush instructions to save / restore VFP / NEON registers like this: vpush {d8,d10,d11} vpop {d8,d10,d11} vpush and vpop do not allow gaps in the register list. rdar://8728956 llvm-svn: 121197 --- lib/Target/ARM/ARMFrameInfo.cpp | 149 +++++++++++++++----------- lib/Target/ARM/ARMFrameInfo.h | 9 +- test/CodeGen/ARM/2010-12-07-PEIBug.ll | 40 +++++++ 3 files changed, 131 insertions(+), 67 deletions(-) create mode 100644 test/CodeGen/ARM/2010-12-07-PEIBug.ll diff --git a/lib/Target/ARM/ARMFrameInfo.cpp b/lib/Target/ARM/ARMFrameInfo.cpp index be1bd5542ba..d4aa3eb504a 100644 --- a/lib/Target/ARM/ARMFrameInfo.cpp +++ b/lib/Target/ARM/ARMFrameInfo.cpp @@ -500,7 +500,7 @@ int ARMFrameInfo::getFrameIndexOffset(const MachineFunction &MF, int FI) const { void ARMFrameInfo::emitPushInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, const std::vector &CSI, - unsigned Opc, + unsigned Opc, bool NoGap, bool(*Func)(unsigned, bool)) const { MachineFunction &MF = *MBB.getParent(); const TargetInstrInfo &TII = *MF.getTarget().getInstrInfo(); @@ -509,31 +509,90 @@ void ARMFrameInfo::emitPushInst(MachineBasicBlock &MBB, if (MI != MBB.end()) DL = MI->getDebugLoc(); SmallVector, 4> Regs; - for (unsigned i = CSI.size(); i != 0; --i) { - unsigned Reg = CSI[i-1].getReg(); - if (!(Func)(Reg, STI.isTargetDarwin())) continue; + unsigned i = CSI.size(); + while (i != 0) { + unsigned LastReg = 0; + for (; i != 0; --i) { + unsigned Reg = CSI[i-1].getReg(); + if (!(Func)(Reg, STI.isTargetDarwin())) continue; - // Add the callee-saved register as live-in unless it's LR and - // @llvm.returnaddress is called. If LR is returned for @llvm.returnaddress - // then it's already added to the function and entry block live-in sets. - bool isKill = true; - if (Reg == ARM::LR) { - if (MF.getFrameInfo()->isReturnAddressTaken() && - MF.getRegInfo().isLiveIn(Reg)) - isKill = false; + // Add the callee-saved register as live-in unless it's LR and + // @llvm.returnaddress is called. If LR is returned for @llvm.returnaddress + // then it's already added to the function and entry block live-in sets. + bool isKill = true; + if (Reg == ARM::LR) { + if (MF.getFrameInfo()->isReturnAddressTaken() && + MF.getRegInfo().isLiveIn(Reg)) + isKill = false; + } + + if (isKill) + MBB.addLiveIn(Reg); + + if (NoGap && LastReg) { + if (LastReg != Reg-1) + break; + } + LastReg = Reg; + Regs.push_back(std::make_pair(Reg, isKill)); } - if (isKill) - MBB.addLiveIn(Reg); - - Regs.push_back(std::make_pair(Reg, isKill)); + if (!Regs.empty()) { + MachineInstrBuilder MIB = + AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(Opc),ARM::SP) + .addReg(ARM::SP)); + for (unsigned i = 0, e = Regs.size(); i < e; ++i) + MIB.addReg(Regs[i].first, getKillRegState(Regs[i].second)); + Regs.clear(); + } } +} - if (!Regs.empty()) { - MachineInstrBuilder MIB = AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(Opc), - ARM::SP).addReg(ARM::SP)); - for (unsigned i = 0, e = Regs.size(); i < e; ++i) - MIB.addReg(Regs[i].first, getKillRegState(Regs[i].second)); +void ARMFrameInfo::emitPopInst(MachineBasicBlock &MBB, + MachineBasicBlock::iterator MI, + const std::vector &CSI, + unsigned Opc, bool isVarArg, bool NoGap, + bool(*Func)(unsigned, bool)) const { + MachineFunction &MF = *MBB.getParent(); + const TargetInstrInfo &TII = *MF.getTarget().getInstrInfo(); + ARMFunctionInfo *AFI = MF.getInfo(); + DebugLoc DL = MI->getDebugLoc(); + + SmallVector Regs; + unsigned i = CSI.size(); + while (i != 0) { + unsigned LastReg = 0; + bool DeleteRet = false; + for (; i != 0; --i) { + unsigned Reg = CSI[i-1].getReg(); + if (!(Func)(Reg, STI.isTargetDarwin())) continue; + + if (Reg == ARM::LR && !isVarArg) { + Reg = ARM::PC; + Opc = AFI->isThumbFunction() ? ARM::t2LDMIA_RET : ARM::LDMIA_RET; + // Fold the return instruction into the LDM. + DeleteRet = true; + } + + if (NoGap && LastReg) { + if (LastReg != Reg-1) + break; + } + LastReg = Reg; + Regs.push_back(Reg); + } + + if (!Regs.empty()) { + MachineInstrBuilder MIB = + AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(Opc), ARM::SP) + .addReg(ARM::SP)); + for (unsigned i = 0, e = Regs.size(); i < e; ++i) + MIB.addReg(Regs[i], getDefRegState(true)); + if (DeleteRet) + MI->eraseFromParent(); + MI = MIB; + Regs.clear(); + } } } @@ -550,49 +609,13 @@ bool ARMFrameInfo::spillCalleeSavedRegisters(MachineBasicBlock &MBB, unsigned PushOpc = AFI->isThumbFunction() ? ARM::t2STMDB_UPD : ARM::STMDB_UPD; unsigned FltOpc = ARM::VSTMDDB_UPD; - emitPushInst(MBB, MI, CSI, PushOpc, &isARMArea1Register); - emitPushInst(MBB, MI, CSI, PushOpc, &isARMArea2Register); - emitPushInst(MBB, MI, CSI, FltOpc, &isARMArea3Register); + emitPushInst(MBB, MI, CSI, PushOpc, false, &isARMArea1Register); + emitPushInst(MBB, MI, CSI, PushOpc, false, &isARMArea2Register); + emitPushInst(MBB, MI, CSI, FltOpc, true, &isARMArea3Register); return true; } -void ARMFrameInfo::emitPopInst(MachineBasicBlock &MBB, - MachineBasicBlock::iterator MI, - const std::vector &CSI, - unsigned Opc, bool isVarArg, - bool(*Func)(unsigned, bool)) const { - MachineFunction &MF = *MBB.getParent(); - const TargetInstrInfo &TII = *MF.getTarget().getInstrInfo(); - ARMFunctionInfo *AFI = MF.getInfo(); - DebugLoc DL = MI->getDebugLoc(); - - bool DeleteRet = false; - SmallVector Regs; - for (unsigned i = CSI.size(); i != 0; --i) { - unsigned Reg = CSI[i-1].getReg(); - if (!(Func)(Reg, STI.isTargetDarwin())) continue; - - if (Reg == ARM::LR && !isVarArg) { - Reg = ARM::PC; - Opc = AFI->isThumbFunction() ? ARM::t2LDMIA_RET : ARM::LDMIA_RET; - // Fold the return instruction into the LDM. - DeleteRet = true; - } - - Regs.push_back(Reg); - } - - if (!Regs.empty()) { - MachineInstrBuilder MIB = AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(Opc), - ARM::SP).addReg(ARM::SP)); - for (unsigned i = 0, e = Regs.size(); i < e; ++i) - MIB.addReg(Regs[i], getDefRegState(true)); - if (DeleteRet) - MI->eraseFromParent(); - } -} - bool ARMFrameInfo::restoreCalleeSavedRegisters(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, const std::vector &CSI, @@ -607,9 +630,9 @@ bool ARMFrameInfo::restoreCalleeSavedRegisters(MachineBasicBlock &MBB, unsigned PopOpc = AFI->isThumbFunction() ? ARM::t2LDMIA_UPD : ARM::LDMIA_UPD; unsigned FltOpc = ARM::VLDMDIA_UPD; - emitPopInst(MBB, MI, CSI, FltOpc, isVarArg, &isARMArea3Register); - emitPopInst(MBB, MI, CSI, PopOpc, isVarArg, &isARMArea2Register); - emitPopInst(MBB, MI, CSI, PopOpc, isVarArg, &isARMArea1Register); + emitPopInst(MBB, MI, CSI, FltOpc, isVarArg, true, &isARMArea3Register); + emitPopInst(MBB, MI, CSI, PopOpc, isVarArg, false, &isARMArea2Register); + emitPopInst(MBB, MI, CSI, PopOpc, isVarArg, false, &isARMArea1Register); return true; } diff --git a/lib/Target/ARM/ARMFrameInfo.h b/lib/Target/ARM/ARMFrameInfo.h index 886f7492ab5..2ff77b4b49f 100644 --- a/lib/Target/ARM/ARMFrameInfo.h +++ b/lib/Target/ARM/ARMFrameInfo.h @@ -58,12 +58,13 @@ public: RegScavenger *RS) const; private: - void emitPopInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, - const std::vector &CSI, unsigned Opc, - bool isVarArg, bool(*Func)(unsigned, bool)) const; void emitPushInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, const std::vector &CSI, unsigned Opc, - bool(*Func)(unsigned, bool)) const; + bool NoGap, bool(*Func)(unsigned, bool)) const; + void emitPopInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, + const std::vector &CSI, unsigned Opc, + bool isVarArg, bool NoGap, + bool(*Func)(unsigned, bool)) const; }; } // End llvm namespace diff --git a/test/CodeGen/ARM/2010-12-07-PEIBug.ll b/test/CodeGen/ARM/2010-12-07-PEIBug.ll new file mode 100644 index 00000000000..c65952be3c6 --- /dev/null +++ b/test/CodeGen/ARM/2010-12-07-PEIBug.ll @@ -0,0 +1,40 @@ +; RUN: llc < %s -mtriple=thumbv7-apple-darwin10 -mcpu=cortex-a8 | FileCheck %s +; rdar://8728956 + +define hidden void @foo() nounwind ssp { +entry: +; CHECK: foo: +; CHECK: push {r7, lr} +; CHECK-NEXT: mov r7, sp +; CHECK-NEXT: vpush {d8} +; CHECK-NEXT: vpush {d10, d11} + %tmp40 = load <4 x i8>* undef + %tmp41 = extractelement <4 x i8> %tmp40, i32 2 + %conv42 = zext i8 %tmp41 to i32 + %conv43 = sitofp i32 %conv42 to float + %div44 = fdiv float %conv43, 2.560000e+02 + %vecinit45 = insertelement <4 x float> undef, float %div44, i32 2 + %vecinit46 = insertelement <4 x float> %vecinit45, float 1.000000e+00, i32 3 + store <4 x float> %vecinit46, <4 x float>* undef + br i1 undef, label %if.then105, label %if.else109 + +if.then105: ; preds = %entry + br label %if.end114 + +if.else109: ; preds = %entry + br label %if.end114 + +if.end114: ; preds = %if.else109, %if.then105 + %call185 = call float @bar() + %vecinit186 = insertelement <4 x float> undef, float %call185, i32 1 + %call189 = call float @bar() + %vecinit190 = insertelement <4 x float> %vecinit186, float %call189, i32 2 + %vecinit191 = insertelement <4 x float> %vecinit190, float 1.000000e+00, i32 3 + store <4 x float> %vecinit191, <4 x float>* undef +; CHECK: vpop {d10, d11} +; CHECK-NEXT: vpop {d8} +; CHECK-NEXT: pop {r7, pc} + ret void +} + +declare hidden float @bar() nounwind readnone ssp