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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@105387 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/MachineSink.cpp b/lib/CodeGen/MachineSink.cpp
index 72676faa899981323c7a47b92a348d1cb21c838e..7b4bc15d8c038d6c4accdd896d6aef8b639f6754 100644 (file)
--- 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<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 @@ bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) {
   // 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 @@ 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. (<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

diff --git a/test/CodeGen/X86/MachineSink-CritEdge.ll b/test/CodeGen/X86/MachineSink-CritEdge.ll
index 74a1049772a02cd1fa3bac35b14e39cf002c5088..db06b329f9b9093319f6d6e763b7f3970fab991d 100644 (file)
--- 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 <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"
 

diff --git a/test/CodeGen/X86/sink-hoist.ll b/test/CodeGen/X86/sink-hoist.ll
index 031c01e9af7d672309b2a154acc923b5eae5dad5..acba5288c0d18a2283cce82a15ee4f1b6ddee1f5 100644 (file)
--- 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
-
-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