[llvm] e5fc9a3 - [IR] Simplify BasicBlock::removePredecessor. NFCI.

[Jay Foad via llvm-commits]

Author: Jay Foad
Date: 2020-05-20T09:58:21+01:00
New Revision: e5fc9a3604dca40c89cf243a5208a5135821099c

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

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

This is the second attempt at landing this patch, after fixing the
KeepOneInputPHIs behaviour to also keep zero input PHIs.

Differential Revision: https://reviews.llvm.org/D80141

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 1210ed1eb2ef..8eeafdbaa794 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -370,12 +370,12 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   /// except operator delete.
   void dropAllReferences();
 
-  /// Notify the BasicBlock that the predecessor \p Pred is no longer able to
-  /// reach it.
+  /// Update PHI nodes in this BasicBlock before removal of predecessor \p Pred.
+  /// Note that this function does not actually remove the predecessor.
   ///
-  /// 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.
+  /// If \p KeepOneInputPHIs is true then don't remove PHIs that are left with
+  /// zero or one incoming values, and don't simplify PHIs with all incoming
+  /// values the same.
   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 a9fa7cae92d8..64f1d3f3100c 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -317,60 +317,37 @@ iterator_range<BasicBlock::phi_iterator> BasicBlock::phis() {
   return make_range<phi_iterator>(P, nullptr);
 }
 
-/// 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.
+/// Update PHI nodes in this BasicBlock before removal of predecessor \p Pred.
+/// Note that this function does not actually remove the predecessor.
 ///
+/// If \p KeepOneInputPHIs is true then don't remove PHIs that are left with
+/// zero or one incoming values, and don't simplify PHIs with all incoming
+/// values the same.
 void BasicBlock::removePredecessor(BasicBlock *Pred,
                                    bool KeepOneInputPHIs) {
-  assert((hasNUsesOrMore(16)||// Reduce cost of this assertion for complex CFGs.
+  // Use hasNUsesOrMore to bound the cost of this assertion for complex CFGs.
+  assert((hasNUsesOrMore(16) ||
           find(pred_begin(this), pred_end(this), Pred) != pred_end(this)) &&
-         "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
-      }
+         "Pred is not a predecessor!");
 
-      // 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();
+  // 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, !KeepOneInputPHIs);
+    if (!KeepOneInputPHIs) {
+      // If we have a single predecessor, removeIncomingValue erased the PHI
+      // node itself.
+      if (NumPreds > 1) {
+        if (Value *PNV = PN->hasConstantValue()) {
+          // Replace the PHI node with its constant value.
+          PN->replaceAllUsesWith(PNV);
+          PN->eraseFromParent();
+        }
       }
     }
   }