From 296925dc169b45e7535abdccc8dc143a8bec7f0a Mon Sep 17 00:00:00 2001 From: Evan Cheng Date: Wed, 23 Sep 2009 06:28:31 +0000 Subject: [PATCH] Fix PR5024. LiveVariables physical register defs should *commit* only after all of the defs are processed. Also fix a implicit_def propagation bug: a implicit_def of a physical register should be applied to uses of the sub-registers. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@82616 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/LiveVariables.h | 6 +- lib/CodeGen/LiveIntervalAnalysis.cpp | 4 + lib/CodeGen/LiveVariables.cpp | 125 +++++++++++++----- .../ARM/2009-09-22-LiveVariablesBug.ll | 23 ++++ 4 files changed, 125 insertions(+), 33 deletions(-) create mode 100644 test/CodeGen/ARM/2009-09-22-LiveVariablesBug.ll diff --git a/include/llvm/CodeGen/LiveVariables.h b/include/llvm/CodeGen/LiveVariables.h index 52d5595fc36..45c72f5fe89 100644 --- a/include/llvm/CodeGen/LiveVariables.h +++ b/include/llvm/CodeGen/LiveVariables.h @@ -149,7 +149,11 @@ private: // Intermediate data structures bool HandlePhysRegKill(unsigned Reg, MachineInstr *MI); void HandlePhysRegUse(unsigned Reg, MachineInstr *MI); - void HandlePhysRegDef(unsigned Reg, MachineInstr *MI); + void HandlePhysRegDef(unsigned Reg, MachineInstr *MI, + SmallVector &Defs, + SmallVector &SuperDefs); + void UpdatePhysRegDefs(MachineInstr *MI, SmallVector &Defs); + void UpdateSuperRegDefs(MachineInstr *MI, SmallVector &Defs); /// FindLastPartialDef - Return the last partial def of the specified register. /// Also returns the sub-registers that're defined by the instruction. diff --git a/lib/CodeGen/LiveIntervalAnalysis.cpp b/lib/CodeGen/LiveIntervalAnalysis.cpp index abab1eef686..825cf396ab1 100644 --- a/lib/CodeGen/LiveIntervalAnalysis.cpp +++ b/lib/CodeGen/LiveIntervalAnalysis.cpp @@ -141,6 +141,10 @@ void LiveIntervals::processImplicitDefs() { if (MI->getOpcode() == TargetInstrInfo::IMPLICIT_DEF) { unsigned Reg = MI->getOperand(0).getReg(); ImpDefRegs.insert(Reg); + if (TargetRegisterInfo::isPhysicalRegister(Reg)) { + for (const unsigned *SS = tri_->getSubRegisters(Reg); *SS; ++SS) + ImpDefRegs.insert(*SS); + } ImpDefMIs.push_back(MI); continue; } diff --git a/lib/CodeGen/LiveVariables.cpp b/lib/CodeGen/LiveVariables.cpp index e8a94bdca40..86a8ea9f205 100644 --- a/lib/CodeGen/LiveVariables.cpp +++ b/lib/CodeGen/LiveVariables.cpp @@ -407,7 +407,9 @@ bool LiveVariables::HandlePhysRegKill(unsigned Reg, MachineInstr *MI) { return true; } -void LiveVariables::HandlePhysRegDef(unsigned Reg, MachineInstr *MI) { +void LiveVariables::HandlePhysRegDef(unsigned Reg, MachineInstr *MI, + SmallVector &Defs, + SmallVector &SuperDefs) { // What parts of the register are previously defined? SmallSet Live; if (PhysRegDef[Reg] || PhysRegUse[Reg]) { @@ -465,36 +467,24 @@ void LiveVariables::HandlePhysRegDef(unsigned Reg, MachineInstr *MI) { // EAX = // AX = EAX, EAX // ... - /// = EAX - if (hasRegisterUseBelow(SuperReg, MI, MI->getParent())) { - MI->addOperand(MachineOperand::CreateReg(SuperReg, false/*IsDef*/, - true/*IsImp*/,true/*IsKill*/)); - MI->addOperand(MachineOperand::CreateReg(SuperReg, true/*IsDef*/, - true/*IsImp*/)); - PhysRegDef[SuperReg] = MI; - PhysRegUse[SuperReg] = NULL; - Processed.insert(SuperReg); - for (const unsigned *SS = TRI->getSubRegisters(SuperReg); *SS; ++SS) { - PhysRegDef[*SS] = MI; - PhysRegUse[*SS] = NULL; - Processed.insert(*SS); - } - } else { - // Otherwise, the super register is killed. - if (HandlePhysRegKill(SuperReg, MI)) { - PhysRegDef[SuperReg] = NULL; - PhysRegUse[SuperReg] = NULL; - for (const unsigned *SS = TRI->getSubRegisters(SuperReg); *SS; ++SS) { - PhysRegDef[*SS] = NULL; - PhysRegUse[*SS] = NULL; - Processed.insert(*SS); - } - } - } + // = EAX + SuperDefs.push_back(SuperReg); + Processed.insert(SuperReg); + for (const unsigned *SS = TRI->getSubRegisters(SuperReg); *SS; ++SS) + Processed.insert(*SS); } } // Remember this def. + Defs.push_back(Reg); + } +} + +void LiveVariables::UpdatePhysRegDefs(MachineInstr *MI, + SmallVector &Defs) { + while (!Defs.empty()) { + unsigned Reg = Defs.back(); + Defs.pop_back(); PhysRegDef[Reg] = MI; PhysRegUse[Reg] = NULL; for (const unsigned *SubRegs = TRI->getSubRegisters(Reg); @@ -505,6 +495,66 @@ void LiveVariables::HandlePhysRegDef(unsigned Reg, MachineInstr *MI) { } } +namespace { + struct RegSorter { + const TargetRegisterInfo *TRI; + + RegSorter(const TargetRegisterInfo *tri) : TRI(tri) { } + bool operator()(unsigned A, unsigned B) { + if (TRI->isSubRegister(A, B)) + return true; + else if (TRI->isSubRegister(B, A)) + return false; + return A < B; + } + }; +} + +void LiveVariables::UpdateSuperRegDefs(MachineInstr *MI, + SmallVector &SuperDefs) { + // This instruction has defined part of some registers. If there are no + // more uses below MI, then the last use / def becomes kill / dead. + if (SuperDefs.empty()) + return; + + RegSorter RS(TRI); + std::sort(SuperDefs.begin(), SuperDefs.end(), RS); + SmallSet Processed; + for (unsigned j = 0, ee = SuperDefs.size(); j != ee; ++j) { + unsigned SuperReg = SuperDefs[j]; + if (!Processed.insert(SuperReg)) + continue; + if (hasRegisterUseBelow(SuperReg, MI, MI->getParent())) { + // Previous use / def is not the last use / dead def. It's now + // partially re-defined. + MI->addOperand(MachineOperand::CreateReg(SuperReg, false/*IsDef*/, + true/*IsImp*/,true/*IsKill*/)); + MI->addOperand(MachineOperand::CreateReg(SuperReg, true/*IsDef*/, + true/*IsImp*/)); + PhysRegDef[SuperReg] = MI; + PhysRegUse[SuperReg] = NULL; + for (const unsigned *SS = TRI->getSubRegisters(SuperReg); *SS; ++SS) { + Processed.insert(*SS); + PhysRegDef[*SS] = MI; + PhysRegUse[*SS] = NULL; + } + } else { + // Previous use / def is kill / dead. It's not being re-defined. + HandlePhysRegKill(SuperReg, MI); + PhysRegDef[SuperReg] = 0; + PhysRegUse[SuperReg] = NULL; + for (const unsigned *SS = TRI->getSubRegisters(SuperReg); *SS; ++SS) { + Processed.insert(*SS); + if (PhysRegDef[*SS] == MI) + continue; // This instruction may have defined it. + PhysRegDef[*SS] = MI; + PhysRegUse[*SS] = NULL; + } + } + } + SuperDefs.clear(); +} + bool LiveVariables::runOnMachineFunction(MachineFunction &mf) { MF = &mf; MRI = &mf.getRegInfo(); @@ -537,11 +587,15 @@ bool LiveVariables::runOnMachineFunction(MachineFunction &mf) { MachineBasicBlock *MBB = *DFI; // Mark live-in registers as live-in. + SmallVector Defs; + SmallVector SuperDefs; for (MachineBasicBlock::const_livein_iterator II = MBB->livein_begin(), EE = MBB->livein_end(); II != EE; ++II) { assert(TargetRegisterInfo::isPhysicalRegister(*II) && "Cannot have a live-in virtual register!"); - HandlePhysRegDef(*II, 0); + HandlePhysRegDef(*II, 0, Defs, SuperDefs); + UpdatePhysRegDefs(0, Defs); + SuperDefs.clear(); } // Loop over all of the instructions, processing them. @@ -587,9 +641,13 @@ bool LiveVariables::runOnMachineFunction(MachineFunction &mf) { unsigned MOReg = DefRegs[i]; if (TargetRegisterInfo::isVirtualRegister(MOReg)) HandleVirtRegDef(MOReg, MI); - else if (!ReservedRegisters[MOReg]) - HandlePhysRegDef(MOReg, MI); + else if (!ReservedRegisters[MOReg]) { + HandlePhysRegDef(MOReg, MI, Defs, SuperDefs); + } } + + UpdateSuperRegDefs(MI, SuperDefs); + UpdatePhysRegDefs(MI, Defs); } // Handle any virtual assignments from PHI nodes which might be at the @@ -627,8 +685,11 @@ bool LiveVariables::runOnMachineFunction(MachineFunction &mf) { // Loop over PhysRegDef / PhysRegUse, killing any registers that are // available at the end of the basic block. for (unsigned i = 0; i != NumRegs; ++i) - if (PhysRegDef[i] || PhysRegUse[i]) - HandlePhysRegDef(i, 0); + if (PhysRegDef[i] || PhysRegUse[i]) { + HandlePhysRegDef(i, 0, Defs, SuperDefs); + UpdatePhysRegDefs(0, Defs); + SuperDefs.clear(); + } std::fill(PhysRegDef, PhysRegDef + NumRegs, (MachineInstr*)0); std::fill(PhysRegUse, PhysRegUse + NumRegs, (MachineInstr*)0); diff --git a/test/CodeGen/ARM/2009-09-22-LiveVariablesBug.ll b/test/CodeGen/ARM/2009-09-22-LiveVariablesBug.ll new file mode 100644 index 00000000000..30931a2ffb6 --- /dev/null +++ b/test/CodeGen/ARM/2009-09-22-LiveVariablesBug.ll @@ -0,0 +1,23 @@ +; RUN: llc < %s -mtriple=armv7-none-linux-gnueabi -mattr=+neon + +; PR5024 + +%bar = type { %foo, %foo } +%foo = type { <4 x float> } + +declare arm_aapcs_vfpcc float @aaa(%foo* nocapture) nounwind readonly + +declare arm_aapcs_vfpcc %bar* @bbb(%bar*, <4 x float>, <4 x float>) nounwind + +define arm_aapcs_vfpcc void @ccc(i8* nocapture %pBuffer, i32 %numItems) nounwind { +entry: + br i1 undef, label %return, label %bb.nph + +bb.nph: ; preds = %entry + %0 = call arm_aapcs_vfpcc %bar* @bbb(%bar* undef, <4 x float> undef, <4 x float> undef) nounwind ; <%bar*> [#uses=0] + %1 = call arm_aapcs_vfpcc float @aaa(%foo* undef) nounwind ; [#uses=0] + unreachable + +return: ; preds = %entry + ret void +} -- 2.34.1