Enhance the sinking code to handle diamond patterns. Patch by

Carlo Alberto Ferraris.


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

lib/Transforms/Scalar/Sink.cpp

@@ -27,6 +27,7 @@
 using namespace llvm;
 
 STATISTIC(NumSunk, "Number of instructions sunk");
+STATISTIC(NumSinkIter, "Number of sinking iterations");
 
 namespace {
   class Sinking : public FunctionPass {
@@ -55,6 +56,7 @@ namespace {
     bool ProcessBlock(BasicBlock &BB);
     bool SinkInstruction(Instruction *I, SmallPtrSet<Instruction *, 8> &Stores);
     bool AllUsesDominatedByBlock(Instruction *Inst, BasicBlock *BB) const;
+    bool IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo) const;
   };
 } // end anonymous namespace
   
@@ -98,20 +100,19 @@ bool Sinking::runOnFunction(Function &F) {
   LI = &getAnalysis<LoopInfo>();
   AA = &getAnalysis<AliasAnalysis>();
 
-  bool EverMadeChange = false;
+  bool MadeChange, EverMadeChange = false;
-  
-  while (1) {
-    bool MadeChange = false;
   
+  do {
+    MadeChange = false;
+    DEBUG(dbgs() << "Sinking iteration " << NumSinkIter << "\n");
     // Process all basic blocks.
     for (Function::iterator I = F.begin(), E = F.end(); 
          I != E; ++I)
       MadeChange |= ProcessBlock(*I);
+    EverMadeChange |= MadeChange;
+    NumSinkIter++;
+  } while (MadeChange);
   
-    // If this iteration over the code changed anything, keep iterating.
-    if (!MadeChange) break;
-    EverMadeChange = true;
-  } 
   return EverMadeChange;
 }
 
@@ -174,6 +175,43 @@ static bool isSafeToMove(Instruction *Inst, AliasAnalysis *AA,
   return true;
 }
 
+/// IsAcceptableTarget - Return true if it is possible to sink the instruction
+/// in the specified basic block.
+bool Sinking::IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo) const {
+  assert(Inst && "Instruction to be sunk is null");
+  assert(SuccToSinkTo && "Candidate sink target is null");
+  
+  // It is not possible to sink an instruction into its own block.  This can
+  // happen with loops.
+  if (Inst->getParent() == SuccToSinkTo)
+    return false;
+    
+  // If the block has multiple predecessors, this would introduce computation 
+  // on different code paths.  We could split the critical edge, but for now we
+  // just punt.
+  // FIXME: Split critical edges if not backedges.
+  if (SuccToSinkTo->getUniquePredecessor() != Inst->getParent()) {
+    // We cannot sink a load across a critical edge - there may be stores in
+    // other code paths.
+    if (!isSafeToSpeculativelyExecute(Inst))
+      return false;
+    
+    // We don't want to sink across a critical edge if we don't dominate the
+    // successor. We could be introducing calculations to new code paths.
+    if (!DT->dominates(Inst->getParent(), SuccToSinkTo))
+      return false;
+    
+    // Don't sink instructions into a loop.
+    Loop *succ = LI->getLoopFor(SuccToSinkTo), *cur = LI->getLoopFor(Inst->getParent());
+    if (succ != 0 && succ != cur)
+      return false;
+  }
+  
+  // Finally, check that all the uses of the instruction are actually
+  // dominated by the candidate
+  return AllUsesDominatedByBlock(Inst, SuccToSinkTo);
+}
+
 /// SinkInstruction - Determine whether it is safe to sink the specified machine
 /// instruction out of its current block into a successor.
 bool Sinking::SinkInstruction(Instruction *Inst,
@@ -190,85 +228,41 @@ bool Sinking::SinkInstruction(Instruction *Inst,
   // "x = y + z" down if it kills y and z would increase the live ranges of y
   // and z and only shrink the live range of x.
   
-  // Loop over all the operands of the specified instruction.  If there is
-  // anything we can't handle, bail out.
-  BasicBlock *ParentBlock = Inst->getParent();
-  
   // SuccToSinkTo - This is the successor to sink this instruction to, once we
   // decide.
   BasicBlock *SuccToSinkTo = 0;
   
-  // FIXME: This picks a successor to sink into based on having one
-  // successor that dominates all the uses.  However, there are cases where
-  // sinking can happen but where the sink point isn't a successor.  For
-  // example:
-  //   x = computation
-  //   if () {} else {}
-  //   use x
-  // the instruction could be sunk over the whole diamond for the 
-  // if/then/else (or loop, etc), allowing it to be sunk into other blocks
-  // after that.
-  
   // Instructions can only be sunk if all their uses are in blocks
   // dominated by one of the successors.
-  // Look at all the successors and decide which one
+  // Look at all the postdominators and see if we can sink it in one.
-  // we should sink to.
+  DomTreeNode *DTN = DT->getNode(Inst->getParent());
-  for (succ_iterator SI = succ_begin(ParentBlock),
+  for (DomTreeNode::iterator I = DTN->begin(), E = DTN->end(); 
-       E = succ_end(ParentBlock); SI != E; ++SI) {
+      I != E && SuccToSinkTo == 0; ++I) {
-    if (AllUsesDominatedByBlock(Inst, *SI)) {
+    BasicBlock *Candidate = (*I)->getBlock();
-      SuccToSinkTo = *SI;
+    if ((*I)->getIDom()->getBlock() == Inst->getParent() && 
-      break;
+        IsAcceptableTarget(Inst, Candidate))
+      SuccToSinkTo = Candidate;
   }
+
+  // If no suitable postdominator was found, look at all the successors and 
+  // decide which one we should sink to, if any.
+  for (succ_iterator I = succ_begin(Inst->getParent()), 
+      E = succ_end(Inst->getParent()); I != E && SuccToSinkTo == 0; ++I) {
+    if (IsAcceptableTarget(Inst, *I))
+      SuccToSinkTo = *I;
   }
       
   // If we couldn't find a block to sink to, ignore this instruction.
   if (SuccToSinkTo == 0)
     return false;
   
-  // It is not possible to sink an instruction into its own block.  This can
+  DEBUG(dbgs() << "Sink" << *Inst << " (";
-  // happen with loops.
+        WriteAsOperand(dbgs(), Inst->getParent(), false); 
-  if (Inst->getParent() == SuccToSinkTo)
+        dbgs() << " -> ";
-    return false;
+        WriteAsOperand(dbgs(), SuccToSinkTo, false); 
-  
+        dbgs() << ")\n");
-  DEBUG(dbgs() << "Sink instr " << *Inst);
-  DEBUG(dbgs() << "to block ";
-        WriteAsOperand(dbgs(), SuccToSinkTo, false));
-  
-  // If the block has multiple predecessors, this would introduce computation on
-  // a path that it doesn't already exist.  We could split the critical edge,
-  // but for now we just punt.
-  // FIXME: Split critical edges if not backedges.
-  if (SuccToSinkTo->getUniquePredecessor() != ParentBlock) {
-    // We cannot sink a load across a critical edge - there may be stores in
-    // other code paths.
-    if (!isSafeToSpeculativelyExecute(Inst)) {
-      DEBUG(dbgs() << " *** PUNTING: Wont sink load along critical edge.\n");
-      return false;
-    }
-
-    // We don't want to sink across a critical edge if we don't dominate the
-    // successor. We could be introducing calculations to new code paths.
-    if (!DT->dominates(ParentBlock, SuccToSinkTo)) {
-      DEBUG(dbgs() << " *** PUNTING: Critical edge found\n");
-      return false;
-    }
-
-    // Don't sink instructions into a loop.
-    if (LI->isLoopHeader(SuccToSinkTo)) {
-      DEBUG(dbgs() << " *** PUNTING: Loop header found\n");
-      return false;
-    }
-
-    // Otherwise we are OK with sinking along a critical edge.
-    DEBUG(dbgs() << "Sinking along critical edge.\n");
-  }
-  
-  // Determine where to insert into.  Skip phi nodes.
-  BasicBlock::iterator InsertPos = SuccToSinkTo->begin();
-  while (InsertPos != SuccToSinkTo->end() && isa<PHINode>(InsertPos))
-    ++InsertPos;
   
   // Move the instruction.
-  Inst->moveBefore(InsertPos);
+  Inst->moveBefore(SuccToSinkTo->getFirstInsertionPt());
   return true;
 }

test/Transforms/Sink/basic.ll

@@ -36,3 +36,29 @@ true:
 false:
   ret i32 0
 }
+
+; Sink to the nearest post-dominator
+
+;      CHECK: @diamond
+;      CHECK: X:
+; CHECK-NEXT: phi
+; CHECK-NEXT: mul nsw
+; CHECK-NEXT: sub
+
+define i32 @diamond(i32 %a, i32 %b, i32 %c) {
+  %1 = mul nsw i32 %c, %b
+  %2 = icmp sgt i32 %a, 0
+  br i1 %2, label %B0, label %B1
+
+B0:                                       ; preds = %0
+  br label %X
+
+B1:                                      ; preds = %0
+  br label %X
+
+X:                                     ; preds = %5, %3
+  %.01 = phi i32 [ %c, %B0 ], [ %a, %B1 ]
+  %R = sub i32 %1, %.01
+  ret i32 %R
+}
+