Create a more targeted fix for not sinking instructions into a range where it

will conflict with another live range. The place which creates this scenerio is
the code in X86 that lowers a select instruction by splitting the MBBs. This
eliminates the need to check from the bottom up in an MBB for live pregs.



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

lib/CodeGen/MachineSink.cpp

@@ -25,7 +25,6 @@
 #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"
@@ -62,7 +61,6 @@ 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
 
@@ -168,44 +166,6 @@ 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) {
@@ -228,7 +188,6 @@ 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);
@@ -257,12 +216,9 @@ 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
-          PhysRegs.push_back(Reg);
+      } else if (!MO.isDead()) {
+        // A def that isn't dead. We can't move it.
+        return false;
       }
     } else {
       // Virtual register uses are always safe to sink.
@@ -329,10 +285,14 @@ bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) {
   // 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))
+  for (unsigned I = 0, E = MI->getNumOperands(); I != E; ++I) {
+    const MachineOperand &MO = MI->getOperand(I);
+    if (!MO.isReg()) continue;
+    unsigned Reg = MO.getReg();
+    if (Reg == 0 || !TargetRegisterInfo::isPhysicalRegister(Reg)) continue;
+    if (SuccToSinkTo->isLiveIn(Reg))
       return false;
+  }
 
   DEBUG(dbgs() << "Sink instr " << *MI << "\tinto block " << *SuccToSinkTo);
 

lib/Target/X86/X86ISelLowering.cpp

@@ -8465,22 +8465,42 @@ X86TargetLowering::EmitLoweredSelect(MachineInstr *MI,
   MachineBasicBlock *sinkMBB = F->CreateMachineBasicBlock(LLVM_BB);
   unsigned Opc =
     X86::GetCondBranchFromCond((X86::CondCode)MI->getOperand(3).getImm());
+
   BuildMI(BB, DL, TII->get(Opc)).addMBB(sinkMBB);
   F->insert(It, copy0MBB);
   F->insert(It, sinkMBB);
+
   // Update machine-CFG edges by first adding all successors of the current
   // block to the new block which will contain the Phi node for the select.
   for (MachineBasicBlock::succ_iterator I = BB->succ_begin(),
          E = BB->succ_end(); I != E; ++I)
     sinkMBB->addSuccessor(*I);
+
   // Next, remove all successors of the current block, and add the true
   // and fallthrough blocks as its successors.
   while (!BB->succ_empty())
     BB->removeSuccessor(BB->succ_begin());
+
   // Add the true and fallthrough blocks as its successors.
   BB->addSuccessor(copy0MBB);
   BB->addSuccessor(sinkMBB);
 
+  // If the EFLAGS register isn't dead in the terminator, then claim that it's
+  // live into the sink and copy blocks.
+  const MachineFunction *MF = BB->getParent();
+  const TargetRegisterInfo *TRI = MF->getTarget().getRegisterInfo();
+  BitVector ReservedRegs = TRI->getReservedRegs(*MF);
+  const MachineInstr *Term = BB->getFirstTerminator();
+
+  for (unsigned I = 0, E = Term->getNumOperands(); I != E; ++I) {
+    const MachineOperand &MO = Term->getOperand(I);
+    if (!MO.isReg() || MO.isKill() || MO.isDead()) continue;
+    unsigned Reg = MO.getReg();
+    if (Reg != X86::EFLAGS) continue;
+    copy0MBB->addLiveIn(Reg);
+    sinkMBB->addLiveIn(Reg);
+  }
+
   //  copy0MBB:
   //   %FalseValue = ...
   //   # fallthrough to sinkMBB

test/CodeGen/X86/MachineSink-CritEdge.ll

@@ -1,10 +1,4 @@
 ; 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"