[NFC][DebugInfo][RemoveDIs] Provide an iterator-taking split-block method

[jmorse]

Summary

As per the stack of patches this is attached to, allow users of BasicBlock::splitBasicBlock to provide an iterator for a position, instead of just an instruction pointer. This is to fit with my proposal for how to get rid of debug intrinsics [0]. There are other call-sites that would need to change, but this is sufficient for a stage2clang self host and some other C++ projects to build identical binaries, in the context of the whole remove-DIs project.

This is technically NFC, but I'm not adding reviewers yet as we need to have a discussion about testing first, will put that on discourse.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939

[Orlando]

LGTM (with formatting nits)

llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h

449

nit: clang-format

470

nit: clang-format

llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h

 /// If \p Before is true, splitBlockBefore handles the block
 /// splitting. Otherwise, execution proceeds as described below.
 ///
 /// Everything before \p SplitPt stays in \p Old and everything starting with \p
 /// SplitPt moves to a new block. The two blocks are joined by an unconditional
 /// branch. The new block with name \p BBName is returned.
 ///
 /// FIXME: deprecated, switch to the DomTreeUpdater-based one.
-BasicBlock *SplitBlock(BasicBlock *Old, Instruction *SplitPt, DominatorTree *DT,
+BasicBlock *SplitBlock(BasicBlock *Old, BasicBlock::iterator SplitPt, DominatorTree *DT,
                        LoopInfo *LI = nullptr,
                        MemorySSAUpdater *MSSAU = nullptr,
                        const Twine &BBName = "", bool Before = false);
+inline BasicBlock *SplitBlock(BasicBlock *Old, Instruction *SplitPt, DominatorTree *DT,
+                       LoopInfo *LI = nullptr,
+                       MemorySSAUpdater *MSSAU = nullptr,
+                       const Twine &BBName = "", bool Before = false) {
+  return SplitBlock(Old, SplitPt->getIterator(), DT, LI, MSSAU, BBName, Before);
+}
 
 /// Split the specified block at the specified instruction.
 ///
 /// If \p Before is true, splitBlockBefore handles the block
 /// splitting. Otherwise, execution proceeds as described below.
 ///
 /// Everything before \p SplitPt stays in \p Old and everything starting with \p
 /// SplitPt moves to a new block. The two blocks are joined by an unconditional
 /// branch. The new block with name \p BBName is returned.
-BasicBlock *SplitBlock(BasicBlock *Old, Instruction *SplitPt,
+BasicBlock *SplitBlock(BasicBlock *Old, BasicBlock::iterator SplitPt,
                        DomTreeUpdater *DTU = nullptr, LoopInfo *LI = nullptr,
                        MemorySSAUpdater *MSSAU = nullptr,
                        const Twine &BBName = "", bool Before = false);
+inline BasicBlock *SplitBlock(BasicBlock *Old, Instruction *SplitPt,
+                       DomTreeUpdater *DTU = nullptr, LoopInfo *LI = nullptr,
+                       MemorySSAUpdater *MSSAU = nullptr,
+                       const Twine &BBName = "", bool Before = false) {
+  return SplitBlock(Old, SplitPt->getIterator(), DTU, LI, MSSAU, BBName, Before);
+}
 
 /// Split the specified block at the specified instruction \p SplitPt.
 /// All instructions before \p SplitPt are moved to a new block and all
 /// instructions after \p SplitPt stay in the old block. The new block and the
 /// old block are joined by inserting an unconditional branch to the end of the
 /// new block. The new block with name \p BBName is returned.
-BasicBlock *splitBlockBefore(BasicBlock *Old, Instruction *SplitPt,
+BasicBlock *splitBlockBefore(BasicBlock *Old, BasicBlock::iterator SplitPt,
                              DomTreeUpdater *DTU, LoopInfo *LI,
                              MemorySSAUpdater *MSSAU, const Twine &BBName = "");
+inline BasicBlock *splitBlockBefore(BasicBlock *Old, Instruction *SplitPt,
+                             DomTreeUpdater *DTU, LoopInfo *LI,
+                             MemorySSAUpdater *MSSAU, const Twine &BBName = "") {
+  return splitBlockBefore(Old, SplitPt->getIterator(), DTU, LI, MSSAU, BBName);
+}
 
 /// This method introduces at least one new basic block into the function and
 /// moves some of the predecessors of BB to be predecessors of the new block.
 /// The new predecessors are indicated by the Preds array. The new block is
 /// given a suffix of 'Suffix'. Returns new basic block to which predecessors
 /// from Preds are now pointing.
 ///
 /// If BB is a landingpad block then additional basicblock might be introduced.

 ///   Tail
 ///
 /// If \p ThenBlock is not specified, a new block will be created for it.
 /// If \p Unreachable is true, the newly created block will end with
 /// UnreachableInst, otherwise it branches to Tail.
 /// Returns the NewBasicBlock's terminator.
 ///
 /// Updates DTU and LI if given.
-Instruction *SplitBlockAndInsertIfThen(Value *Cond, Instruction *SplitBefore,
+Instruction *SplitBlockAndInsertIfThen(Value *Cond, BasicBlock::iterator SplitBefore,
                                        bool Unreachable,
                                        MDNode *BranchWeights = nullptr,
                                        DomTreeUpdater *DTU = nullptr,
                                        LoopInfo *LI = nullptr,
                                        BasicBlock *ThenBlock = nullptr);
 
+inline Instruction *SplitBlockAndInsertIfThen(Value *Cond, Instruction *SplitBefore,
+                                       bool Unreachable,
+                                       MDNode *BranchWeights = nullptr,
+                                       DomTreeUpdater *DTU = nullptr,
+                                       LoopInfo *LI = nullptr,
+                                       BasicBlock *ThenBlock = nullptr) {
+  return SplitBlockAndInsertIfThen(Cond, SplitBefore->getIterator(),
+                                   Unreachable, BranchWeights, DTU, LI,
+                                   ThenBlock);
+}
+
 /// Similar to SplitBlockAndInsertIfThen, but the inserted block is on the false
 /// path of the branch.
-Instruction *SplitBlockAndInsertIfElse(Value *Cond, Instruction *SplitBefore,
+Instruction *SplitBlockAndInsertIfElse(Value *Cond, BasicBlock::iterator SplitBefore,
                                        bool Unreachable,
                                        MDNode *BranchWeights = nullptr,
                                        DomTreeUpdater *DTU = nullptr,
                                        LoopInfo *LI = nullptr,
                                        BasicBlock *ElseBlock = nullptr);
 
+inline Instruction *SplitBlockAndInsertIfElse(Value *Cond, Instruction *SplitBefore,
+                                       bool Unreachable,
+                                       MDNode *BranchWeights = nullptr,
+                                       DomTreeUpdater *DTU = nullptr,
+                                       LoopInfo *LI = nullptr,
+                                       BasicBlock *ElseBlock = nullptr) {
+  return SplitBlockAndInsertIfElse(Cond, SplitBefore->getIterator(),
+                                   Unreachable, BranchWeights, DTU, LI,
+                                   ElseBlock);
+}
+
 /// SplitBlockAndInsertIfThenElse is similar to SplitBlockAndInsertIfThen,
 /// but also creates the ElseBlock.
 /// Before:
 ///   Head
 ///   SplitBefore
 ///   Tail
 /// After:
 ///   Head
 ///   if (Cond)
 ///     ThenBlock
 ///   else
 ///     ElseBlock
 ///   SplitBefore
 ///   Tail
 ///
 /// Updates DT if given.
-void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
+void SplitBlockAndInsertIfThenElse(Value *Cond,
+                                   BasicBlock::iterator SplitBefore,
                                    Instruction **ThenTerm,
                                    Instruction **ElseTerm,
                                    MDNode *BranchWeights = nullptr,
                                    DomTreeUpdater *DTU = nullptr,
                                    LoopInfo *LI = nullptr);
 
+inline void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
+                                   Instruction **ThenTerm,
+                                   Instruction **ElseTerm,
+                                   MDNode *BranchWeights = nullptr,
+                                   DomTreeUpdater *DTU = nullptr,
+                                   LoopInfo *LI = nullptr)
+{
+  SplitBlockAndInsertIfThenElse(Cond, SplitBefore->getIterator(), ThenTerm,
+                               ElseTerm, BranchWeights, DTU, LI);
+}
+
 /// Split the containing block at the specified instruction - everything before
 /// SplitBefore stays in the old basic block, and the rest of the instructions
 /// in the BB are moved to a new block. The two blocks are connected by a
 /// conditional branch (with value of Cmp being the condition).
 /// Before:
 ///   Head
 ///   SplitBefore
 ///   Tail

 /// block will be inserted as the TrueBlock. Otherwise a new block will be
 /// created. Likewise for the \p ElseBlock parameter.
 /// If \p UnreachableThen or \p UnreachableElse is true, the corresponding newly
 /// created blocks will end with UnreachableInst, otherwise with branches to
 /// Tail. The function will not modify existing basic blocks passed to it. The
 /// caller must ensure that Tail is reachable from Head.
 /// Returns the newly created blocks in \p ThenBlock and \p ElseBlock.
 /// Updates DTU and LI if given.
-void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
+void SplitBlockAndInsertIfThenElse(Value *Cond,
+                                   BasicBlock::iterator SplitBefore,
                                    BasicBlock **ThenBlock,
                                    BasicBlock **ElseBlock,
                                    bool UnreachableThen = false,
                                    bool UnreachableElse = false,
                                    MDNode *BranchWeights = nullptr,
                                    DomTreeUpdater *DTU = nullptr,
                                    LoopInfo *LI = nullptr);
 
+inline void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
+                                   BasicBlock **ThenBlock,
+                                   BasicBlock **ElseBlock,
+                                   bool UnreachableThen = false,
+                                   bool UnreachableElse = false,
+                                   MDNode *BranchWeights = nullptr,
+                                   DomTreeUpdater *DTU = nullptr,
+                                   LoopInfo *LI = nullptr) {
+  SplitBlockAndInsertIfThenElse(Cond, SplitBefore->getIterator(), ThenBlock,
+    ElseBlock, UnreachableThen, UnreachableElse, BranchWeights, DTU, LI);
+}
+
 /// Insert a for (int i = 0; i < End; i++) loop structure (with the exception
 /// that \p End is assumed > 0, and thus not checked on entry) at \p
 /// SplitBefore.  Returns the first insert point in the loop body, and the
 /// PHINode for the induction variable (i.e. "i" above).
 std::pair<Instruction*, Value*>
 SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore);
 
 /// Utility function for performing a given action on each lane of a vector

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

static bool unswitchTrivialBranch(Loop &L, BranchInst &BI, DominatorTree &DT,
   // Because the loop is in simplified form, *any* other predecessor is enough.
   BasicBlock *UnswitchedBB;
   if (FullUnswitch && LoopExitBB->getUniquePredecessor()) {
     assert(LoopExitBB->getUniquePredecessor() == BI.getParent() &&
            "A branch's parent isn't a predecessor!");
     UnswitchedBB = LoopExitBB;
   } else {
     UnswitchedBB =
-        SplitBlock(LoopExitBB, &LoopExitBB->front(), &DT, &LI, MSSAU);
+        SplitBlock(LoopExitBB, LoopExitBB->begin(), &DT, &LI, MSSAU, "", false);
   }
 
   if (MSSAU && VerifyMemorySSA)
     MSSAU->getMemorySSA()->verifyMemorySSA();
 
   // Actually move the invariant uses into the unswitched position. If possible,
   // we do this by moving the instructions, but when doing partial unswitching
   // we do it by building a new merge of the values in the unswitched position.
static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
   // FIXME: It'd be great if we could merge this with the loop below but LLVM's
   // ranges aren't quite powerful enough yet.
   if (DefaultExitBB) {
     if (pred_empty(DefaultExitBB)) {
       UnswitchedExitBBs.insert(DefaultExitBB);
       rewritePHINodesForUnswitchedExitBlock(*DefaultExitBB, *ParentBB, *OldPH);
     } else {
       auto *SplitBB =
-          SplitBlock(DefaultExitBB, &DefaultExitBB->front(), &DT, &LI, MSSAU);
+          SplitBlock(DefaultExitBB, DefaultExitBB->begin(), &DT, &LI, MSSAU);
       rewritePHINodesForExitAndUnswitchedBlocks(*DefaultExitBB, *SplitBB,
                                                 *ParentBB, *OldPH,
                                                 /*FullUnswitch*/ true);
       DefaultExitBB = SplitExitBBMap[DefaultExitBB] = SplitBB;
     }
   }
   // Note that we must use a reference in the for loop so that we update the
   // container.
if (pred_empty(ExitBB)) {
       continue;
     }
 
     // Otherwise we need to split the exit block so that we retain an exit
     // block from the loop and a target for the unswitched condition.
     BasicBlock *&SplitExitBB = SplitExitBBMap[ExitBB];
     if (!SplitExitBB) {
       // If this is the first time we see this, do the split and remember it.
-      SplitExitBB = SplitBlock(ExitBB, &ExitBB->front(), &DT, &LI, MSSAU);
+      SplitExitBB = SplitBlock(ExitBB, ExitBB->begin(), &DT, &LI, MSSAU);
       rewritePHINodesForExitAndUnswitchedBlocks(*ExitBB, *SplitExitBB,
                                                 *ParentBB, *OldPH,
                                                 /*FullUnswitch*/ true);
     }
     // Update the case pair to point to the split block.
     std::get<1>(ExitCase) = SplitExitBB;
   }
 
for (auto *ExitBB : ExitBlocks) {
     if (SkipBlock(ExitBB))
       continue;
 
     // When we are going to clone an exit, we don't need to clone all the
     // instructions in the exit block and we want to ensure we have an easy
     // place to merge the CFG, so split the exit first. This is always safe to
     // do because there cannot be any non-loop predecessors of a loop exit in
     // loop simplified form.
-    auto *MergeBB = SplitBlock(ExitBB, &ExitBB->front(), &DT, &LI, MSSAU);
+    auto *MergeBB = SplitBlock(ExitBB, ExitBB->begin(), &DT, &LI, MSSAU);
 
     // Rearrange the names to make it easier to write test cases by having the
     // exit block carry the suffix rather than the merge block carrying the
     // suffix.
     MergeBB->takeName(ExitBB);
     ExitBB->setName(Twine(MergeBB->getName()) + ".split");
 
     // Now clone the original exit block.

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

for (BasicBlock &BB : F) {
     if (TI->getNumSuccessors() > 1 && !isa<IndirectBrInst>(TI))
       for (unsigned i = 0, e = TI->getNumSuccessors(); i != e; ++i)
         if (SplitCriticalEdge(TI, i, Options))
           ++NumBroken;
   }
   return NumBroken;
 }
 
-static BasicBlock *SplitBlockImpl(BasicBlock *Old, Instruction *SplitPt,
+static BasicBlock *SplitBlockImpl(BasicBlock *Old, BasicBlock::iterator SplitPt,
                                   DomTreeUpdater *DTU, DominatorTree *DT,
                                   LoopInfo *LI, MemorySSAUpdater *MSSAU,
                                   const Twine &BBName, bool Before) {
   if (Before) {
     DomTreeUpdater LocalDTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
     return splitBlockBefore(Old, SplitPt,
                             DTU ? DTU : (DT ? &LocalDTU : nullptr), LI, MSSAU,
                             BBName);
   }
-  BasicBlock::iterator SplitIt = SplitPt->getIterator();
+  BasicBlock::iterator SplitIt = SplitPt;
   while (isa<PHINode>(SplitIt) || SplitIt->isEHPad()) {
     ++SplitIt;
     assert(SplitIt != SplitPt->getParent()->end());
   }
   std::string Name = BBName.str();
   BasicBlock *New = Old->splitBasicBlock(
       SplitIt, Name.empty() ? Old->getName() + ".split" : Name);
 
static BasicBlock *SplitBlockImpl(BasicBlock *Old, BasicBlock::iterator SplitPt,
   // Move MemoryAccesses still tracked in Old, but part of New now.
   // Update accesses in successor blocks accordingly.
   if (MSSAU)
     MSSAU->moveAllAfterSpliceBlocks(Old, New, &*(New->begin()));
 
   return New;
 }
 
-BasicBlock *llvm::SplitBlock(BasicBlock *Old, Instruction *SplitPt,
+BasicBlock *llvm::SplitBlock(BasicBlock *Old, BasicBlock::iterator SplitPt,
                              DominatorTree *DT, LoopInfo *LI,
                              MemorySSAUpdater *MSSAU, const Twine &BBName,
                              bool Before) {
   return SplitBlockImpl(Old, SplitPt, /*DTU=*/nullptr, DT, LI, MSSAU, BBName,
                         Before);
 }
-BasicBlock *llvm::SplitBlock(BasicBlock *Old, Instruction *SplitPt,
+BasicBlock *llvm::SplitBlock(BasicBlock *Old, BasicBlock::iterator SplitPt,
                              DomTreeUpdater *DTU, LoopInfo *LI,
                              MemorySSAUpdater *MSSAU, const Twine &BBName,
                              bool Before) {
   return SplitBlockImpl(Old, SplitPt, DTU, /*DT=*/nullptr, LI, MSSAU, BBName,
                         Before);
 }
 
-BasicBlock *llvm::splitBlockBefore(BasicBlock *Old, Instruction *SplitPt,
+BasicBlock *llvm::splitBlockBefore(BasicBlock *Old, BasicBlock::iterator SplitPt,
                                    DomTreeUpdater *DTU, LoopInfo *LI,
                                    MemorySSAUpdater *MSSAU,
                                    const Twine &BBName) {
 
-  BasicBlock::iterator SplitIt = SplitPt->getIterator();
+  BasicBlock::iterator SplitIt = SplitPt;
   while (isa<PHINode>(SplitIt) || SplitIt->isEHPad())
     ++SplitIt;
   std::string Name = BBName.str();
   BasicBlock *New = Old->splitBasicBlock(
       SplitIt, Name.empty() ? Old->getName() + ".split" : Name,
       /* Before=*/true);
 
   // The new block lives in whichever loop the old one did. This preserves
ReturnInst *llvm::FoldReturnIntoUncondBranch(ReturnInst *RI, BasicBlock *BB,
 
   if (DTU)
     DTU->applyUpdates({{DominatorTree::Delete, Pred, BB}});
 
   return cast<ReturnInst>(NewRet);
 }
 
 Instruction *llvm::SplitBlockAndInsertIfThen(Value *Cond,
-                                             Instruction *SplitBefore,
+                                             BasicBlock::iterator SplitBefore,
                                              bool Unreachable,
                                              MDNode *BranchWeights,
                                              DomTreeUpdater *DTU, LoopInfo *LI,
                                              BasicBlock *ThenBlock) {
   SplitBlockAndInsertIfThenElse(
       Cond, SplitBefore, &ThenBlock, /* ElseBlock */ nullptr,
       /* UnreachableThen */ Unreachable,
       /* UnreachableElse */ false, BranchWeights, DTU, LI);
   return ThenBlock->getTerminator();
 }
 
 Instruction *llvm::SplitBlockAndInsertIfElse(Value *Cond,
-                                             Instruction *SplitBefore,
+                                             BasicBlock::iterator SplitBefore,
                                              bool Unreachable,
                                              MDNode *BranchWeights,
                                              DomTreeUpdater *DTU, LoopInfo *LI,
                                              BasicBlock *ElseBlock) {
   SplitBlockAndInsertIfThenElse(
       Cond, SplitBefore, /* ThenBlock */ nullptr, &ElseBlock,
       /* UnreachableThen */ false,
       /* UnreachableElse */ Unreachable, BranchWeights, DTU, LI);
   return ElseBlock->getTerminator();
 }
 
-void llvm::SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
+void llvm::SplitBlockAndInsertIfThenElse(Value *Cond, BasicBlock::iterator SplitBefore,
                                          Instruction **ThenTerm,
                                          Instruction **ElseTerm,
                                          MDNode *BranchWeights,
                                          DomTreeUpdater *DTU, LoopInfo *LI) {
   BasicBlock *ThenBlock = nullptr;
   BasicBlock *ElseBlock = nullptr;
   SplitBlockAndInsertIfThenElse(
       Cond, SplitBefore, &ThenBlock, &ElseBlock, /* UnreachableThen */ false,
       /* UnreachableElse */ false, BranchWeights, DTU, LI);
 
   *ThenTerm = ThenBlock->getTerminator();
   *ElseTerm = ElseBlock->getTerminator();
 }
 
 void llvm::SplitBlockAndInsertIfThenElse(
-    Value *Cond, Instruction *SplitBefore, BasicBlock **ThenBlock,
+    Value *Cond, BasicBlock::iterator SplitBefore, BasicBlock **ThenBlock,
     BasicBlock **ElseBlock, bool UnreachableThen, bool UnreachableElse,
     MDNode *BranchWeights, DomTreeUpdater *DTU, LoopInfo *LI) {
   assert((ThenBlock || ElseBlock) &&
          "At least one branch block must be created");
   assert((!UnreachableThen || !UnreachableElse) &&
          "Split block tail must be reachable");
 
   SmallVector<DominatorTree::UpdateType, 8> Updates;
   SmallPtrSet<BasicBlock *, 8> UniqueOrigSuccessors;
   BasicBlock *Head = SplitBefore->getParent();
   if (DTU) {
     UniqueOrigSuccessors.insert(succ_begin(Head), succ_end(Head));
     Updates.reserve(4 + 2 * UniqueOrigSuccessors.size());
   }
 
   LLVMContext &C = Head->getContext();
-  BasicBlock *Tail = Head->splitBasicBlock(SplitBefore->getIterator());
+  BasicBlock *Tail = Head->splitBasicBlock(SplitBefore);
   BasicBlock *TrueBlock = Tail;
   BasicBlock *FalseBlock = Tail;
   bool ThenToTailEdge = false;
   bool ElseToTailEdge = false;
 
   // Encapsulate the logic around creation/insertion/etc of a new block.
   auto handleBlock = [&](BasicBlock **PBB, bool Unreachable, BasicBlock *&BB,
                          bool &ToTailEdge) {

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

static bool mergeConditionalStoreToAddress(
   Value *QPred = QStore->getParent() == QTB ? QCond : QB.CreateNot(QCond);
 
   if (InvertPCond)
     PPred = QB.CreateNot(PPred);
   if (InvertQCond)
     QPred = QB.CreateNot(QPred);
   Value *CombinedPred = QB.CreateOr(PPred, QPred);
 
-  auto *T = SplitBlockAndInsertIfThen(CombinedPred, &*QB.GetInsertPoint(),
+  BasicBlock::iterator InsertPt = QB.GetInsertPoint();
+  auto *T = SplitBlockAndInsertIfThen(CombinedPred, InsertPt,
                                       /*Unreachable=*/false,
                                       /*BranchWeights=*/nullptr, DTU);
+
   QB.SetInsertPoint(T);
   StoreInst *SI = cast<StoreInst>(QB.CreateStore(QPHI, Address));
   SI->setAAMetadata(PStore->getAAMetadata().merge(QStore->getAAMetadata()));
   // Choose the minimum alignment. If we could prove both stores execute, we
   // could use biggest one.  In this case, though, we only know that one of the
   // stores executes.  And we don't know it's safe to take the alignment from a
   // store that doesn't execute.
   SI->setAlignment(std::min(PStore->getAlign(), QStore->getAlign()));