[llvm-commits] [llvm] r105387 - in /llvm/trunk: lib/CodeGen/MachineSink.cpp test/CodeGen/X86/MachineSink-CritEdge.ll test/CodeGen/X86/sink-hoist.ll
Bill Wendling
isanbard at gmail.com
Thu Jun 3 00:54:20 PDT 2010
Author: void
Date: Thu Jun 3 02:54:20 2010
New Revision: 105387
URL: http://llvm.org/viewvc/llvm-project?rev=105387&view=rev
Log:
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<imp-def,dead>
test ..., EFLAGS<imp-def>
jcc BB#2 EFLAGS<imp-use>
BB#1
... ; fallthrough to BB#2
BB#2
... ; No code that defines EFLAGS
jcc ... EFLAGS<imp-use>
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.
Modified:
llvm/trunk/lib/CodeGen/MachineSink.cpp
llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll
llvm/trunk/test/CodeGen/X86/sink-hoist.ll
Modified: llvm/trunk/lib/CodeGen/MachineSink.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=105387&r1=105386&r2=105387&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineSink.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineSink.cpp Thu Jun 3 02:54:20 2010
@@ -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 @@
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 @@
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<unsigned, 8> 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 @@
// SuccToSinkTo - This is the successor to sink this instruction to, once we
// decide.
MachineBasicBlock *SuccToSinkTo = 0;
+ SmallVector<unsigned, 4> PhysRegs;
for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
const MachineOperand &MO = MI->getOperand(i);
@@ -216,9 +257,12 @@
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 @@
// 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. (<rdar://problem/8030636>)
+ for (SmallVectorImpl<unsigned>::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
Modified: llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll?rev=105387&r1=105386&r2=105387&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll (original)
+++ llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll Thu Jun 3 02:54:20 2010
@@ -1,4 +1,10 @@
; RUN: llc < %s | FileCheck %s
+; XFAIL: *
+;
+; See <rdar://problem/8030636>. 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"
Modified: llvm/trunk/test/CodeGen/X86/sink-hoist.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sink-hoist.ll?rev=105387&r1=105386&r2=105387&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/sink-hoist.ll (original)
+++ llvm/trunk/test/CodeGen/X86/sink-hoist.ll Thu Jun 3 02:54:20 2010
@@ -44,26 +44,33 @@
; Sink instructions with dead EFLAGS defs.
-; CHECK: zzz:
-; CHECK: je
-; CHECK-NEXT: orb
-
-define zeroext i8 @zzz(i8 zeroext %a, i8 zeroext %b) nounwind readnone {
-entry:
- %tmp = zext i8 %a to i32 ; <i32> [#uses=1]
- %tmp2 = icmp eq i8 %a, 0 ; <i1> [#uses=1]
- %tmp3 = or i8 %b, -128 ; <i8> [#uses=1]
- %tmp4 = and i8 %b, 127 ; <i8> [#uses=1]
- %b_addr.0 = select i1 %tmp2, i8 %tmp4, i8 %tmp3 ; <i8> [#uses=1]
- ret i8 %b_addr.0
-}
+; FIXME: Unfail the zzz test if we can correctly mark pregs with the kill flag.
+;
+; See <rdar://problem/8030636>. 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.
+
+
+; 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 ; <i32> [#uses=1]
+; %tmp2 = icmp eq i8 %a, 0 ; <i1> [#uses=1]
+; %tmp3 = or i8 %b, -128 ; <i8> [#uses=1]
+; %tmp4 = and i8 %b, 127 ; <i8> [#uses=1]
+; %b_addr.0 = select i1 %tmp2, i8 %tmp4, i8 %tmp3 ; <i8> [#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
More information about the llvm-commits
mailing list