From 869d60d39d579b2051a8e34f460de72f071c2172 Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Thu, 3 Jun 2010 07:54:20 +0000 Subject: Machine sink could potentially sink instructions into a block where the physical registers it defines then interfere with an existing preg live range. For instance, if we had something like these machine instructions: BB#0 ... = imul ... EFLAGS test ..., EFLAGS jcc BB#2 EFLAGS BB#1 ... ; fallthrough to BB#2 BB#2 ... ; No code that defines EFLAGS jcc ... EFLAGS Machine sink will come along, see that imul implicitly defines EFLAGS, but because it's "dead", it assumes that it can move imul into BB#2. But when it does, imul's "dead" imp-def of EFLAGS is raised from the dead (a zombie) and messes up the condition code for the jump (and pretty much anything else which relies upon it being correct). The solution is to know which pregs are live going into a basic block. However, that information isn't calculated at this point. Nor does the LiveVariables pass take into account non-allocatable physical registers. In lieu of this, we do a *very* conservative pass through the basic block to determine if a preg is live coming out of it. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@105387 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/MachineSink.cpp | 60 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) (limited to 'lib/CodeGen/MachineSink.cpp') diff --git a/lib/CodeGen/MachineSink.cpp b/lib/CodeGen/MachineSink.cpp index 72676fa..7b4bc15 100644 --- a/lib/CodeGen/MachineSink.cpp +++ b/lib/CodeGen/MachineSink.cpp @@ -25,6 +25,7 @@ #include "llvm/Target/TargetRegisterInfo.h" #include "llvm/Target/TargetInstrInfo.h" #include "llvm/Target/TargetMachine.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/Statistic.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" @@ -61,6 +62,7 @@ namespace { bool ProcessBlock(MachineBasicBlock &MBB); bool SinkInstruction(MachineInstr *MI, bool &SawStore); bool AllUsesDominatedByBlock(unsigned Reg, MachineBasicBlock *MBB) const; + bool LiveOutOfBasicBlock(const MachineInstr *MI, unsigned Reg) const; }; } // end anonymous namespace @@ -166,6 +168,44 @@ bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) { return MadeChange; } +/// LiveOutOfBasicBlock - Determine if the physical register, defined and dead +/// in MI, is live on exit from the basic block. +bool MachineSinking::LiveOutOfBasicBlock(const MachineInstr *MI, + unsigned Reg) const { + assert(TargetRegisterInfo::isPhysicalRegister(Reg) && + "Only want to determine if a physical register is live out of a BB!"); + + const MachineBasicBlock *MBB = MI->getParent(); + SmallSet KilledRegs; + MachineBasicBlock::const_iterator I = MBB->end(); + MachineBasicBlock::const_iterator E = MBB->begin(); + assert(I != E && "How can there be an empty block at this point?!"); + + // Loop through the instructions bottom-up. If we see a kill of the preg + // first, then it's not live out of the BB. If we see a use or def first, then + // we assume that it is live out of the BB. + do { + const MachineInstr &CurMI = *--I; + + for (unsigned i = 0, e = CurMI.getNumOperands(); i != e; ++i) { + const MachineOperand &MO = CurMI.getOperand(i); + if (!MO.isReg()) continue; // Ignore non-register operands. + + unsigned MOReg = MO.getReg(); + if (MOReg == 0) continue; + + if (MOReg == Reg) { + if (MO.isKill()) + return false; + if (MO.isUse() || MO.isDef()) + return true; + } + } + } while (I != E); + + return false; +} + /// SinkInstruction - Determine whether it is safe to sink the specified machine /// instruction out of its current block into a successor. bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) { @@ -188,6 +228,7 @@ bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) { // SuccToSinkTo - This is the successor to sink this instruction to, once we // decide. MachineBasicBlock *SuccToSinkTo = 0; + SmallVector PhysRegs; for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) { const MachineOperand &MO = MI->getOperand(i); @@ -216,9 +257,12 @@ bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) { if (AllocatableSet.test(AliasReg)) return false; } - } else if (!MO.isDead()) { - // A def that isn't dead. We can't move it. - return false; + } else { + if (!MO.isDead()) + // A def that isn't dead. We can't move it. + return false; + else + PhysRegs.push_back(Reg); } } else { // Virtual register uses are always safe to sink. @@ -281,7 +325,15 @@ bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) { // happen with loops. if (MI->getParent() == SuccToSinkTo) return false; - + + // If the instruction to move defines a dead physical register which is live + // when leaving the basic block, don't move it because it could turn into a + // "zombie" define of that preg. E.g., EFLAGS. () + for (SmallVectorImpl::const_iterator + I = PhysRegs.begin(), E = PhysRegs.end(); I != E; ++I) + if (LiveOutOfBasicBlock(MI, *I)) + return false; + DEBUG(dbgs() << "Sink instr " << *MI << "\tinto block " << *SuccToSinkTo); // If the block has multiple predecessors, this would introduce computation on -- cgit v1.1