diff --git a/lib/CodeGen/MachineSink.cpp b/lib/CodeGen/MachineSink.cpp index 72676faa899..7b4bc15d8c0 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 diff --git a/test/CodeGen/X86/MachineSink-CritEdge.ll b/test/CodeGen/X86/MachineSink-CritEdge.ll index 74a1049772a..db06b329f9b 100644 --- a/test/CodeGen/X86/MachineSink-CritEdge.ll +++ b/test/CodeGen/X86/MachineSink-CritEdge.ll @@ -1,4 +1,10 @@ ; RUN: llc < %s | FileCheck %s +; XFAIL: * +; +; See . This test isn't valid after we made machine +; sinking more conservative about sinking instructions that define a preg into a +; block when we don't know if the preg is killed within the current block. + target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64" target triple = "x86_64-apple-darwin10.0.0" diff --git a/test/CodeGen/X86/sink-hoist.ll b/test/CodeGen/X86/sink-hoist.ll index 031c01e9af7..acba5288c0d 100644 --- a/test/CodeGen/X86/sink-hoist.ll +++ b/test/CodeGen/X86/sink-hoist.ll @@ -44,26 +44,33 @@ return: ; Sink instructions with dead EFLAGS defs. -; CHECK: zzz: -; CHECK: je -; CHECK-NEXT: orb +; FIXME: Unfail the zzz test if we can correctly mark pregs with the kill flag. +; +; See . This test isn't valid after we made machine +; sinking more conservative about sinking instructions that define a preg into a +; block when we don't know if the preg is killed within the current block. -define zeroext i8 @zzz(i8 zeroext %a, i8 zeroext %b) nounwind readnone { -entry: - %tmp = zext i8 %a to i32 ; [#uses=1] - %tmp2 = icmp eq i8 %a, 0 ; [#uses=1] - %tmp3 = or i8 %b, -128 ; [#uses=1] - %tmp4 = and i8 %b, 127 ; [#uses=1] - %b_addr.0 = select i1 %tmp2, i8 %tmp4, i8 %tmp3 ; [#uses=1] - ret i8 %b_addr.0 -} + +; FIXMEHECK: zzz: +; FIXMEHECK: je +; FIXMEHECK-NEXT: orb + +; define zeroext i8 @zzz(i8 zeroext %a, i8 zeroext %b) nounwind readnone { +; entry: +; %tmp = zext i8 %a to i32 ; [#uses=1] +; %tmp2 = icmp eq i8 %a, 0 ; [#uses=1] +; %tmp3 = or i8 %b, -128 ; [#uses=1] +; %tmp4 = and i8 %b, 127 ; [#uses=1] +; %b_addr.0 = select i1 %tmp2, i8 %tmp4, i8 %tmp3 ; [#uses=1] +; ret i8 %b_addr.0 +; } ; Codegen should hoist and CSE these constants. ; CHECK: vv: -; CHECK: LCPI3_0(%rip), %xmm0 -; CHECK: LCPI3_1(%rip), %xmm1 -; CHECK: LCPI3_2(%rip), %xmm2 +; CHECK: LCPI2_0(%rip), %xmm0 +; CHECK: LCPI2_1(%rip), %xmm1 +; CHECK: LCPI2_2(%rip), %xmm2 ; CHECK: align ; CHECK-NOT: LCPI ; CHECK: ret