[llvm] [IR] Implement successors as Use iterators (PR #186616)

via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 14 12:01:54 PDT 2026


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-spir-v

Author: Alexis Engelke (aengelke)

<details>
<summary>Changes</summary>

This is possible since now all successor operands are stored consecutively.

There is just one out-of-line function call instead of one call to getSuccessor() per operand.

---
Full diff: https://github.com/llvm/llvm-project/pull/186616.diff


7 Files Affected:

- (modified) llvm/include/llvm/Analysis/CFGPrinter.h (+3-3) 
- (modified) llvm/include/llvm/IR/CFG.h (+14-116) 
- (modified) llvm/lib/Analysis/BranchProbabilityInfo.cpp (+6-4) 
- (modified) llvm/lib/Target/SPIRV/SPIRVMergeRegionExitTargets.cpp (+1-1) 
- (modified) llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp (+3-5) 
- (modified) llvm/lib/Transforms/Scalar/JumpThreading.cpp (+2-2) 
- (modified) llvm/unittests/Analysis/CFGTest.cpp (+5-4) 


``````````diff
diff --git a/llvm/include/llvm/Analysis/CFGPrinter.h b/llvm/include/llvm/Analysis/CFGPrinter.h
index 5ff6fe10c77db..c48d2e5887919 100644
--- a/llvm/include/llvm/Analysis/CFGPrinter.h
+++ b/llvm/include/llvm/Analysis/CFGPrinter.h
@@ -240,7 +240,7 @@ struct DOTGraphTraits<DOTFuncInfo *> : public DefaultDOTGraphTraits {
 
     // Label source of switch edges with the associated value.
     if (const SwitchInst *SI = dyn_cast<SwitchInst>(Node->getTerminator())) {
-      unsigned SuccNo = I.getSuccessorIndex();
+      unsigned SuccNo = std::distance(succ_begin(SI), I);
 
       if (SuccNo == 0)
         return "def";
@@ -272,8 +272,8 @@ struct DOTGraphTraits<DOTFuncInfo *> : public DefaultDOTGraphTraits {
     if (!CFGInfo->showEdgeWeights())
       return "";
 
-    unsigned OpNo = I.getSuccessorIndex();
     const Instruction *TI = Node->getTerminator();
+    unsigned OpNo = std::distance(succ_begin(TI), I);
     BasicBlock *SuccBB = TI->getSuccessor(OpNo);
     auto BranchProb = CFGInfo->getBPI()->getEdgeProbability(Node, SuccBB);
     double WeightPercent = ((double)BranchProb.getNumerator()) /
@@ -310,7 +310,7 @@ struct DOTGraphTraits<DOTFuncInfo *> : public DefaultDOTGraphTraits {
     if (!WeightsNode)
       return TTAttr;
 
-    OpNo = I.getSuccessorIndex() + 1;
+    OpNo += 1;
     if (OpNo >= WeightsNode->getNumOperands())
       return TTAttr;
     ConstantInt *Weight =
diff --git a/llvm/include/llvm/IR/CFG.h b/llvm/include/llvm/IR/CFG.h
index 96d3b2fbb5b0b..c0d2218c24322 100644
--- a/llvm/include/llvm/IR/CFG.h
+++ b/llvm/include/llvm/IR/CFG.h
@@ -135,124 +135,22 @@ inline const_pred_range predecessors(const BasicBlock *BB) {
 // Instruction and BasicBlock succ_iterator helpers
 //===----------------------------------------------------------------------===//
 
-template <class InstructionT, class BlockT>
-class SuccIterator
-    : public iterator_facade_base<SuccIterator<InstructionT, BlockT>,
-                                  std::random_access_iterator_tag, BlockT, int,
-                                  BlockT *, BlockT *> {
-public:
-  using value_type = BlockT *;
-  using difference_type = std::ptrdiff_t;
-  using pointer = BlockT *;
-  using reference = BlockT *;
-
-private:
-  InstructionT *Inst;
-  int Idx;
-  using Self = SuccIterator<InstructionT, BlockT>;
-
-  inline bool index_is_valid(int Idx) {
-    // Note that we specially support the index of zero being valid even in the
-    // face of a null instruction.
-    return Idx >= 0 && (Idx == 0 || Idx <= (int)Inst->getNumSuccessors());
-  }
-
-  /// Proxy object to allow write access in operator[]
-  class SuccessorProxy {
-    Self It;
-
-  public:
-    explicit SuccessorProxy(const Self &It) : It(It) {}
-
-    SuccessorProxy(const SuccessorProxy &) = default;
-
-    SuccessorProxy &operator=(SuccessorProxy RHS) {
-      *this = reference(RHS);
-      return *this;
-    }
-
-    SuccessorProxy &operator=(reference RHS) {
-      It.Inst->setSuccessor(It.Idx, RHS);
-      return *this;
-    }
-
-    operator reference() const { return *It; }
-  };
-
-public:
-  // begin iterator
-  explicit inline SuccIterator(InstructionT *Inst) : Inst(Inst), Idx(0) {}
-  // end iterator
-  inline SuccIterator(InstructionT *Inst, bool) : Inst(Inst) {
-    if (Inst)
-      Idx = Inst->getNumSuccessors();
-    else
-      // Inst == NULL happens, if a basic block is not fully constructed and
-      // consequently getTerminator() returns NULL. In this case we construct
-      // a SuccIterator which describes a basic block that has zero
-      // successors.
-      // Defining SuccIterator for incomplete and malformed CFGs is especially
-      // useful for debugging.
-      Idx = 0;
-  }
-
-  /// This is used to interface between code that wants to
-  /// operate on terminator instructions directly.
-  int getSuccessorIndex() const { return Idx; }
-
-  inline bool operator==(const Self &x) const { return Idx == x.Idx; }
-
-  inline BlockT *operator*() const { return Inst->getSuccessor(Idx); }
-
-  // We use the basic block pointer directly for operator->.
-  inline BlockT *operator->() const { return operator*(); }
-
-  inline bool operator<(const Self &RHS) const {
-    assert(Inst == RHS.Inst && "Cannot compare iterators of different blocks!");
-    return Idx < RHS.Idx;
-  }
-
-  int operator-(const Self &RHS) const {
-    assert(Inst == RHS.Inst && "Cannot compare iterators of different blocks!");
-    return Idx - RHS.Idx;
-  }
-
-  inline Self &operator+=(int RHS) {
-    int NewIdx = Idx + RHS;
-    assert(index_is_valid(NewIdx) && "Iterator index out of bound");
-    Idx = NewIdx;
-    return *this;
-  }
-
-  inline Self &operator-=(int RHS) { return operator+=(-RHS); }
-
-  // Specially implement the [] operation using a proxy object to support
-  // assignment.
-  inline SuccessorProxy operator[](int Offset) {
-    Self TmpIt = *this;
-    TmpIt += Offset;
-    return SuccessorProxy(TmpIt);
-  }
-
-  /// Get the source BlockT of this iterator.
-  inline BlockT *getSource() {
-    assert(Inst && "Source not available, if basic block was malformed");
-    return Inst->getParent();
-  }
-};
-
-using succ_iterator = SuccIterator<Instruction, BasicBlock>;
-using const_succ_iterator = SuccIterator<const Instruction, const BasicBlock>;
+using succ_iterator = Instruction::succ_iterator;
+using const_succ_iterator = Instruction::const_succ_iterator;
 using succ_range = iterator_range<succ_iterator>;
 using const_succ_range = iterator_range<const_succ_iterator>;
 
-inline succ_iterator succ_begin(Instruction *I) { return succ_iterator(I); }
+inline succ_iterator succ_begin(Instruction *I) {
+  return I ? I->successors().begin() : succ_iterator(nullptr);
+}
 inline const_succ_iterator succ_begin(const Instruction *I) {
-  return const_succ_iterator(I);
+  return I ? I->successors().begin() : const_succ_iterator(nullptr);
+}
+inline succ_iterator succ_end(Instruction *I) {
+  return I ? I->successors().end() : succ_iterator(nullptr);
 }
-inline succ_iterator succ_end(Instruction *I) { return succ_iterator(I, true); }
 inline const_succ_iterator succ_end(const Instruction *I) {
-  return const_succ_iterator(I, true);
+  return I ? I->successors().end() : const_succ_iterator(nullptr);
 }
 inline bool succ_empty(const Instruction *I) {
   return succ_begin(I) == succ_end(I);
@@ -268,16 +166,16 @@ inline const_succ_range successors(const Instruction *I) {
 }
 
 inline succ_iterator succ_begin(BasicBlock *BB) {
-  return succ_iterator(BB->getTerminator());
+  return succ_begin(BB->getTerminator());
 }
 inline const_succ_iterator succ_begin(const BasicBlock *BB) {
-  return const_succ_iterator(BB->getTerminator());
+  return succ_begin(BB->getTerminator());
 }
 inline succ_iterator succ_end(BasicBlock *BB) {
-  return succ_iterator(BB->getTerminator(), true);
+  return succ_end(BB->getTerminator());
 }
 inline const_succ_iterator succ_end(const BasicBlock *BB) {
-  return const_succ_iterator(BB->getTerminator(), true);
+  return succ_end(BB->getTerminator());
 }
 inline bool succ_empty(const BasicBlock *BB) {
   return succ_begin(BB) == succ_end(BB);
diff --git a/llvm/lib/Analysis/BranchProbabilityInfo.cpp b/llvm/lib/Analysis/BranchProbabilityInfo.cpp
index 49c6bc8191c86..22d6ffe012956 100644
--- a/llvm/lib/Analysis/BranchProbabilityInfo.cpp
+++ b/llvm/lib/Analysis/BranchProbabilityInfo.cpp
@@ -395,10 +395,11 @@ bool BranchProbabilityInfo::calcMetadataWeights(const BasicBlock *BB) {
   SmallVector<unsigned, 2> ReachableIdxs;
 
   extractBranchWeights(WeightsNode, Weights);
+  auto Succs = succ_begin(TI);
   for (unsigned I = 0, E = Weights.size(); I != E; ++I) {
     WeightSum += Weights[I];
     const LoopBlock SrcLoopBB = getLoopBlock(BB);
-    const LoopBlock DstLoopBB = getLoopBlock(TI->getSuccessor(I));
+    const LoopBlock DstLoopBB = getLoopBlock(*Succs++);
     auto EstimatedWeight = getEstimatedEdgeWeight({SrcLoopBB, DstLoopBB});
     if (EstimatedWeight &&
         *EstimatedWeight <= static_cast<uint32_t>(BlockExecWeight::UNREACHABLE))
@@ -1104,7 +1105,7 @@ BranchProbabilityInfo::getEdgeProbability(const BasicBlock *Src,
 BranchProbability
 BranchProbabilityInfo::getEdgeProbability(const BasicBlock *Src,
                                           const_succ_iterator Dst) const {
-  return getEdgeProbability(Src, Dst.getSuccessorIndex());
+  return getEdgeProbability(Src, std::distance(succ_begin(Src), Dst));
 }
 
 /// Get the raw edge probability calculated for the block pair. This returns the
@@ -1116,9 +1117,10 @@ BranchProbabilityInfo::getEdgeProbability(const BasicBlock *Src,
     return BranchProbability(llvm::count(successors(Src), Dst), succ_size(Src));
 
   auto Prob = BranchProbability::getZero();
-  for (const_succ_iterator I = succ_begin(Src), E = succ_end(Src); I != E; ++I)
+  for (const_succ_iterator B = succ_begin(Src), I = B, E = succ_end(Src);
+       I != E; ++I)
     if (*I == Dst)
-      Prob += Probs.find(std::make_pair(Src, I.getSuccessorIndex()))->second;
+      Prob += Probs.find(std::make_pair(Src, std::distance(B, I)))->second;
 
   return Prob;
 }
diff --git a/llvm/lib/Target/SPIRV/SPIRVMergeRegionExitTargets.cpp b/llvm/lib/Target/SPIRV/SPIRVMergeRegionExitTargets.cpp
index 3495dac81b5bd..54ead93f2d52d 100644
--- a/llvm/lib/Target/SPIRV/SPIRVMergeRegionExitTargets.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVMergeRegionExitTargets.cpp
@@ -141,7 +141,7 @@ class SPIRVMergeRegionExitTargets : public FunctionPass {
       Instruction *T = Exit->getTerminator();
       for (auto I = succ_begin(T), E = succ_end(T); I != E; ++I)
         if (ExitTargets.contains(*I))
-          T->setSuccessor(I.getSuccessorIndex(), NewExitTarget);
+          I.getUse()->set(NewExitTarget);
     }
 
     CR = CR->Parent;
diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index 386e48f81a93f..69c91b4327e5b 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -1654,20 +1654,18 @@ ComputePostOrders(Function &F,
   BasicBlock *EntryBB = &F.getEntryBlock();
   BBState &MyStates = BBStates[EntryBB];
   MyStates.SetAsEntry();
-  Instruction *EntryTI = EntryBB->getTerminator();
-  SuccStack.push_back(std::make_pair(EntryBB, succ_iterator(EntryTI)));
+  SuccStack.push_back(std::make_pair(EntryBB, succ_begin(EntryBB)));
   Visited.insert(EntryBB);
   OnStack.insert(EntryBB);
   do {
   dfs_next_succ:
     BasicBlock *CurrBB = SuccStack.back().first;
-    succ_iterator SE(CurrBB->getTerminator(), false);
+    succ_iterator SE = succ_end(CurrBB->getTerminator());
 
     while (SuccStack.back().second != SE) {
       BasicBlock *SuccBB = *SuccStack.back().second++;
       if (Visited.insert(SuccBB).second) {
-        SuccStack.push_back(
-            std::make_pair(SuccBB, succ_iterator(SuccBB->getTerminator())));
+        SuccStack.push_back(std::make_pair(SuccBB, succ_begin(SuccBB)));
         BBStates[CurrBB].addSucc(SuccBB);
         BBState &SuccStates = BBStates[SuccBB];
         SuccStates.addPred(CurrBB);
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 1f53f45e9ff64..bdf691c783f24 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2547,9 +2547,9 @@ void JumpThreadingPass::updateBlockFreqAndEdgeWeight(BasicBlock *PredBB,
   // Collect updated outgoing edges' frequencies from BB and use them to update
   // edge probabilities.
   SmallVector<uint64_t, 4> BBSuccFreq;
-  for (succ_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I) {
+  for (succ_iterator B = succ_begin(BB), I = B, E = succ_end(BB); I != E; ++I) {
     auto BB2SuccBBFreq =
-        BBOrigFreq * BPI->getEdgeProbability(BB, I.getSuccessorIndex());
+        BBOrigFreq * BPI->getEdgeProbability(BB, std::distance(B, I));
     auto SuccFreq = (*I == SuccBB) ? BB2SuccBBFreq - NewBBFreq : BB2SuccBBFreq;
     BBSuccFreq.push_back(SuccFreq.getFrequency());
   }
diff --git a/llvm/unittests/Analysis/CFGTest.cpp b/llvm/unittests/Analysis/CFGTest.cpp
index 352ee6a1bf43e..7b61413cc48e6 100644
--- a/llvm/unittests/Analysis/CFGTest.cpp
+++ b/llvm/unittests/Analysis/CFGTest.cpp
@@ -392,11 +392,12 @@ TEST_F(IsPotentiallyReachableTest, BranchInsideLoop) {
 TEST_F(IsPotentiallyReachableTest, ModifyTest) {
   ParseAssembly(BranchInsideLoopIR);
 
-  succ_iterator S = succ_begin(&*++M->getFunction("test")->begin());
-  BasicBlock *OldBB = S[0];
-  S[0] = S[1];
+  BasicBlock *LoopBB = &*++M->getFunction("test")->begin();
+  auto *T = cast<CondBrInst>(LoopBB->getTerminator());
+  BasicBlock *OldBB = T->getSuccessor(0);
+  T->setSuccessor(0, T->getSuccessor(1));
   ExpectPath(false);
-  S[0] = OldBB;
+  T->setSuccessor(0, OldBB);
   ExpectPath(true);
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/186616


More information about the llvm-commits mailing list