[llvm] b42b30c - Revert "[IR] Simplify BasicBlock::removePredecessor. NFCI."

[Jay Foad via llvm-commits]

Author: Jay Foad
Date: 2020-05-20T08:01:43+01:00
New Revision: b42b30c335bccccca95470aa47dade2e25cb8994

URL: https://github.com/llvm/llvm-project/commit/b42b30c335bccccca95470aa47dade2e25cb8994
DIFF: https://github.com/llvm/llvm-project/commit/b42b30c335bccccca95470aa47dade2e25cb8994.diff

LOG: Revert "[IR] Simplify BasicBlock::removePredecessor. NFCI."

This reverts commit 59f49f7ee7f3397e000f7e11facb4a5605cd1cab.

It was causing buildbot failures.

Added: 
    

Modified: 
    llvm/include/llvm/IR/BasicBlock.h
    llvm/lib/IR/BasicBlock.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 312d2a0569ad..1210ed1eb2ef 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -370,8 +370,12 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   /// except operator delete.
   void dropAllReferences();
 
-  /// Update PHI nodes in this BasicBlock before removal of predecessor \p Pred.
-  /// Note that this function does not actually remove the predecessor.
+  /// Notify the BasicBlock that the predecessor \p Pred is no longer able to
+  /// reach it.
+  ///
+  /// This is actually not used to update the Predecessor list, but is actually
+  /// used to update the PHI nodes that reside in the block.  Note that this
+  /// should be called while the predecessor still refers to this block.
   void removePredecessor(BasicBlock *Pred, bool KeepOneInputPHIs = false);
 
   bool canSplitPredecessors() const;

diff  --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index fb4639c675d5..a9fa7cae92d8 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -317,31 +317,58 @@ iterator_range<BasicBlock::phi_iterator> BasicBlock::phis() {
   return make_range<phi_iterator>(P, nullptr);
 }
 
-/// Update PHI nodes in this BasicBlock before removal of predecessor \p Pred.
-/// Note that this function does not actually remove the predecessor.
+/// This method is used to notify a BasicBlock that the
+/// specified Predecessor of the block is no longer able to reach it.  This is
+/// actually not used to update the Predecessor list, but is actually used to
+/// update the PHI nodes that reside in the block.  Note that this should be
+/// called while the predecessor still refers to this block.
+///
 void BasicBlock::removePredecessor(BasicBlock *Pred,
                                    bool KeepOneInputPHIs) {
-  // Use hasNUsesOrMore to bound the cost of this assertion for complex CFGs.
-  assert((hasNUsesOrMore(16) ||
+  assert((hasNUsesOrMore(16)||// Reduce cost of this assertion for complex CFGs.
           find(pred_begin(this), pred_end(this), Pred) != pred_end(this)) &&
-         "Pred is not a predecessor!");
+         "removePredecessor: BB is not a predecessor!");
+
+  if (InstList.empty()) return;
+  PHINode *APN = dyn_cast<PHINode>(&front());
+  if (!APN) return;   // Quick exit.
+
+  // If there are exactly two predecessors, then we want to nuke the PHI nodes
+  // altogether.
+  unsigned max_idx = APN->getNumIncomingValues();
+  assert(max_idx != 0 && "PHI Node in block with 0 predecessors!?!?!");
+
+  // <= Two predecessors BEFORE I remove one?
+  if (max_idx <= 2 && !KeepOneInputPHIs) {
+    // Yup, loop through and nuke the PHI nodes
+    while (PHINode *PN = dyn_cast<PHINode>(&front())) {
+      // Remove the predecessor first.
+      PN->removeIncomingValue(Pred, !KeepOneInputPHIs);
+
+      // If the PHI _HAD_ two uses, replace PHI node with its now *single* value
+      if (max_idx == 2) {
+        if (PN->getIncomingValue(0) != PN)
+          PN->replaceAllUsesWith(PN->getIncomingValue(0));
+        else
+          // We are left with an infinite loop with no entries: kill the PHI.
+          PN->replaceAllUsesWith(UndefValue::get(PN->getType()));
+        getInstList().pop_front();    // Remove the PHI node
+      }
 
-  // Return early if there are no PHI nodes to update.
-  if (!isa<PHINode>(begin()))
-    return;
-  unsigned NumPreds = cast<PHINode>(front()).getNumIncomingValues();
-
-  // Update all PHI nodes.
-  for (iterator II = begin(); isa<PHINode>(II);) {
-    PHINode *PN = cast<PHINode>(II++);
-    PN->removeIncomingValue(Pred);
-    // If we have a single predecessor, removeIncomingValue erased the PHI node
-    // itself.
-    // FIXME in practice "KeepOneInputPHIs" means "KeepConstantPHIs" and some
-    // callers seem to rely on that.
-    if (NumPreds > 1 && !KeepOneInputPHIs) {
-      if (Value *PNV = PN->hasConstantValue()) {
-        // Replace the PHI node with its constant value.
+      // If the PHI node already only had one entry, it got deleted by
+      // removeIncomingValue.
+    }
+  } else {
+    // Okay, now we know that we need to remove predecessor #pred_idx from all
+    // PHI nodes.  Iterate over each PHI node fixing them up
+    PHINode *PN;
+    for (iterator II = begin(); (PN = dyn_cast<PHINode>(II)); ) {
+      ++II;
+      PN->removeIncomingValue(Pred, false);
+      // If all incoming values to the Phi are the same, we can replace the Phi
+      // with that value.
+      Value* PNV = nullptr;
+      if (!KeepOneInputPHIs && (PNV = PN->hasConstantValue())) {
         PN->replaceAllUsesWith(PNV);
         PN->eraseFromParent();
       }