[NFC] PHINode: introduce replaceIncomingBlockWith() function, use it

[lebedev.ri]

Summary

There is PHINode::getBasicBlockIndex(), PHINode::setIncomingBlock()
and PHINode::getNumOperands(), but no function to replace every
specified BasicBlock* predecessor with some other specified BasicBlock*.
Clearly, there are a lot of places that could use that functionality.

[craig.topper]

include/llvm/IR/Instructions.h

2742 ↗

(On Diff #196235)

Would 'replace' be more consistent with other functions than 'change'? And I think we often include 'With' at the end for methods like this. For example User::replaceUsesOfWith

[craig.topper]

include/llvm/IR/Instructions.h

2745 ↗

(On Diff #196412)

Drop the temporary variable. And then you can probably drop the curly braces.

[lebedev.ri]

Address review notes.

[lebedev.ri]

lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp

1366 ↗

(On Diff #196811)

This one is weird.
replacePHIBlock() would already iterate over every operand, so it's basically dead code i suppose.

[craig.topper]

LGTM

llvm/trunk/include/llvm/IR/Instructions.h

BasicBlock *getIncomingBlock(Value::const_user_iterator I) const {
     return getIncomingBlock(I.getUse());
   }
 
   void setIncomingBlock(unsigned i, BasicBlock *BB) {
     assert(BB && "PHI node got a null basic block!");
     block_begin()[i] = BB;
   }
 
+  /// Replace every incoming basic block \p Old to basic block \p New.
+  void replaceIncomingBlockWith(BasicBlock *Old, BasicBlock *New) {
+    assert(New && Old && "PHI node got a null basic block!");
+    for (unsigned Op = 0, NumOps = getNumOperands(); Op != NumOps; ++Op)
+      if (getIncomingBlock(Op) == Old)
+        setIncomingBlock(Op, New);
+  }
+
   /// Add an incoming value to the end of the PHI list
   ///
   void addIncoming(Value *V, BasicBlock *BB) {
     if (getNumOperands() == ReservedSpace)
       growOperands();  // Get more space!
     // Initialize some new operands.
     setNumHungOffUseOperands(getNumOperands() + 1);
     setIncomingValue(getNumOperands() - 1, V);

llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp

for (auto &BB : F) {
     // (and/or) we have to swap the successors (TrueDest, FalseDest), so that
     // we perform the correct update for the PHI nodes.
     // This doesn't change the successor order of the just created branch
     // instruction (or any other instruction).
     if (Opc == Instruction::Or)
       std::swap(TBB, FBB);
 
     // Replace the old BB with the new BB.
-    for (PHINode &PN : TBB->phis()) {
-      int i;
-      while ((i = PN.getBasicBlockIndex(&BB)) >= 0)
-        PN.setIncomingBlock(i, TmpBB);
-    }
+    for (PHINode &PN : TBB->phis())
+      PN.replaceIncomingBlockWith(&BB, TmpBB);
 
     // Add another incoming edge form the new BB.
     for (PHINode &PN : FBB->phis()) {
       auto *Val = PN.getIncomingValueForBlock(&BB);
       PN.addIncoming(Val, TmpBB);
     }
 
     // Update the branch weights (from SelectionDAGBuilder::

llvm/trunk/lib/IR/BasicBlock.cpp

BasicBlock *BasicBlock::splitBasicBlock(iterator I, const Twine &BBName) {
   // _were_ the successors of the 'this' block), and update any PHI nodes in
   // successors.  If there were PHI nodes in the successors, then they need to
   // know that incoming branches will be from New, not from Old.
   //
   for (succ_iterator I = succ_begin(New), E = succ_end(New); I != E; ++I) {
     // Loop over any phi nodes in the basic block, updating the BB field of
     // incoming values...
     BasicBlock *Successor = *I;
-    for (auto &PN : Successor->phis()) {
-      int Idx = PN.getBasicBlockIndex(this);
-      while (Idx != -1) {
-        PN.setIncomingBlock((unsigned)Idx, New);
-        Idx = PN.getBasicBlockIndex(this);
-      }
-    }
+    for (auto &PN : Successor->phis())
+      PN.replaceIncomingBlockWith(this, New);
   }
   return New;
 }
 
 void BasicBlock::replaceSuccessorsPhiUsesWith(BasicBlock *New) {
   Instruction *TI = getTerminator();
   if (!TI)
     // Cope with being called on a BasicBlock that doesn't have a terminator
     // yet. Clang's CodeGenFunction::EmitReturnBlock() likes to do this.
     return;
   for (BasicBlock *Succ : successors(TI)) {
     // N.B. Succ might not be a complete BasicBlock, so don't assume
     // that it ends with a non-phi instruction.
     for (iterator II = Succ->begin(), IE = Succ->end(); II != IE; ++II) {
       PHINode *PN = dyn_cast<PHINode>(II);
       if (!PN)
         break;
-      int i;
-      while ((i = PN->getBasicBlockIndex(this)) >= 0)
-        PN->setIncomingBlock(i, New);
+      PN->replaceIncomingBlockWith(this, New);
     }
   }
 }
 
 /// Return true if this basic block is a landing pad. I.e., it's
 /// the destination of the 'unwind' edge of an invoke instruction.
 bool BasicBlock::isLandingPad() const {
   return isa<LandingPadInst>(getFirstNonPHI());

llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp

class LoopConstrainer {
   // is None if there is no restriction on high end of the restricted iteration
   // space of the main loop.
 
   struct SubRanges {
     Optional<const SCEV *> LowLimit;
     Optional<const SCEV *> HighLimit;
   };
 
-  // A utility function that does a `replaceUsesOfWith' on the incoming block
-  // set of a `PHINode' -- replaces instances of `Block' in the `PHINode's
-  // incoming block list with `ReplaceBy'.
-  static void replacePHIBlock(PHINode *PN, BasicBlock *Block,
-                              BasicBlock *ReplaceBy);
-
   // Compute a safe set of limits for the main loop to run in -- effectively the
   // intersection of `Range' and the iteration space of the original loop.
   // Return None if unable to compute the set of subranges.
   Optional<SubRanges> calculateSubRanges(bool IsSignedPredicate) const;
 
   // Clone `OriginalLoop' and return the result in CLResult.  The IR after
   // running `cloneLoop' is well formed except for the PHI nodes in CLResult --
   // the PHI nodes say that there is an incoming edge from `OriginalPreheader`
LoopConstrainer(Loop &L, LoopInfo &LI,
         Range(R), MainLoopStructure(LS) {}
 
   // Entry point for the algorithm.  Returns true on success.
   bool run();
 };
 
 } // end anonymous namespace
 
-void LoopConstrainer::replacePHIBlock(PHINode *PN, BasicBlock *Block,
-                                      BasicBlock *ReplaceBy) {
-  for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
-    if (PN->getIncomingBlock(i) == Block)
-      PN->setIncomingBlock(i, ReplaceBy);
-}
-
 /// Given a loop with an deccreasing induction variable, is it possible to
 /// safely calculate the bounds of a new loop using the given Predicate.
 static bool isSafeDecreasingBound(const SCEV *Start,
                                   const SCEV *BoundSCEV, const SCEV *Step,
                                   ICmpInst::Predicate Pred,
                                   unsigned LatchBrExitIdx,
                                   Loop *L, ScalarEvolution &SE) {
   if (Pred != ICmpInst::ICMP_SLT && Pred != ICmpInst::ICMP_SGT &&
LoopConstrainer::RewrittenRangeInfo LoopConstrainer::changeIterationSpaceEnd(
   RRI.IndVarEnd = PHINode::Create(IndVarBase->getType(), 2, "indvar.end",
                                   BranchToContinuation);
   RRI.IndVarEnd->addIncoming(IndVarStart, Preheader);
   RRI.IndVarEnd->addIncoming(IndVarBase, RRI.ExitSelector);
 
   // The latch exit now has a branch from `RRI.ExitSelector' instead of
   // `LS.Latch'.  The PHI nodes need to be updated to reflect that.
   for (PHINode &PN : LS.LatchExit->phis())
-    replacePHIBlock(&PN, LS.Latch, RRI.ExitSelector);
+    PN.replaceIncomingBlockWith(LS.Latch, RRI.ExitSelector);
 
   return RRI;
 }
 
 void LoopConstrainer::rewriteIncomingValuesForPHIs(
     LoopStructure &LS, BasicBlock *ContinuationBlock,
     const LoopConstrainer::RewrittenRangeInfo &RRI) const {
   unsigned PHIIndex = 0;
   for (PHINode &PN : LS.Header->phis())
     for (unsigned i = 0, e = PN.getNumIncomingValues(); i < e; ++i)
       if (PN.getIncomingBlock(i) == ContinuationBlock)
         PN.setIncomingValue(i, RRI.PHIValuesAtPseudoExit[PHIIndex++]);
 
   LS.IndVarStart = RRI.IndVarEnd;
 }
 
 BasicBlock *LoopConstrainer::createPreheader(const LoopStructure &LS,
                                              BasicBlock *OldPreheader,
                                              const char *Tag) const {
   BasicBlock *Preheader = BasicBlock::Create(Ctx, Tag, &F, LS.Header);
   BranchInst::Create(LS.Header, Preheader);
 
   for (PHINode &PN : LS.Header->phis())
-    for (unsigned i = 0, e = PN.getNumIncomingValues(); i < e; ++i)
-      replacePHIBlock(&PN, OldPreheader, Preheader);
+    PN.replaceIncomingBlockWith(OldPreheader, Preheader);
 
   return Preheader;
 }
 
 void LoopConstrainer::addToParentLoopIfNeeded(ArrayRef<BasicBlock *> BBs) {
   Loop *ParentLoop = OriginalLoop.getParentLoop();
   if (!ParentLoop)
     return;

llvm/trunk/lib/Transforms/Scalar/LoopInterchange.cpp

static void moveBBContents(BasicBlock *FromBB, Instruction *InsertBefore) {
   auto &FromList = FromBB->getInstList();
 
   ToList.splice(InsertBefore->getIterator(), FromList, FromList.begin(),
                 FromBB->getTerminator()->getIterator());
 }
 
 static void updateIncomingBlock(BasicBlock *CurrBlock, BasicBlock *OldPred,
                                 BasicBlock *NewPred) {
-  for (PHINode &PHI : CurrBlock->phis()) {
-    unsigned Num = PHI.getNumIncomingValues();
-    for (unsigned i = 0; i < Num; ++i) {
-      if (PHI.getIncomingBlock(i) == OldPred)
-        PHI.setIncomingBlock(i, NewPred);
-    }
-  }
+  for (PHINode &PHI : CurrBlock->phis())
+    PHI.replaceIncomingBlockWith(OldPred, NewPred);
 }
 
 /// Update BI to jump to NewBB instead of OldBB. Records updates to
 /// the dominator tree in DTUpdates, if DT should be preserved.
 static void updateSuccessor(BranchInst *BI, BasicBlock *OldBB,
                             BasicBlock *NewBB,
                             std::vector<DominatorTree::UpdateType> &DTUpdates) {
   assert(llvm::count_if(successors(BI),