From 89665812f54feeab985c0c8b72cf3063128e65b2 Mon Sep 17 00:00:00 2001 From: Alexis Engelke Date: Mon, 6 Apr 2026 22:47:34 +0200 Subject: [PATCH] [Analysis][NFC] Use block numbers in BlockFrequencyInfo (#190669) Block pointers are only stored while constructing the analysis, so the value handle to catch erased blocks is no longer needed when using stable block numbers. --- .../llvm/Analysis/BlockFrequencyInfoImpl.h | 168 ++++++------------ 1 file changed, 59 insertions(+), 109 deletions(-) diff --git a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h index 54f4e70bf2f8..09379056f960 100644 --- a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h +++ b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h @@ -541,7 +541,6 @@ namespace bfi_detail { template struct TypeMap {}; template <> struct TypeMap { using BlockT = BasicBlock; - using BlockKeyT = AssertingVH; using FunctionT = Function; using BranchProbabilityInfoT = BranchProbabilityInfo; using LoopT = Loop; @@ -549,16 +548,12 @@ template <> struct TypeMap { }; template <> struct TypeMap { using BlockT = MachineBasicBlock; - using BlockKeyT = const MachineBasicBlock *; using FunctionT = MachineFunction; using BranchProbabilityInfoT = MachineBranchProbabilityInfo; using LoopT = MachineLoop; using LoopInfoT = MachineLoopInfo; }; -template -class BFICallbackVH; - /// Get the name of a MachineBasicBlock. /// /// Get the name of a MachineBasicBlock. It's templated so that including from @@ -841,7 +836,6 @@ void IrreducibleGraph::addEdges(const BlockNode &Node, /// series by simulation.) template class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase { using BlockT = typename bfi_detail::TypeMap::BlockT; - using BlockKeyT = typename bfi_detail::TypeMap::BlockKeyT; using FunctionT = typename bfi_detail::TypeMap::FunctionT; using BranchProbabilityInfoT = typename bfi_detail::TypeMap::BranchProbabilityInfoT; @@ -849,18 +843,23 @@ template class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase { using LoopInfoT = typename bfi_detail::TypeMap::LoopInfoT; using Successor = GraphTraits; using Predecessor = GraphTraits>; - using BFICallbackVH = - bfi_detail::BFICallbackVH; const BranchProbabilityInfoT *BPI = nullptr; const LoopInfoT *LI = nullptr; const FunctionT *F = nullptr; // All blocks in reverse postorder. - std::vector RPOT; - DenseMap Nodes; + std::vector RPOT; + /// Map from block number to number on RPOT/Freqs. + SmallVector Nodes; + unsigned BlockNumberEpoch; - BlockNode getNode(const BlockT *BB) const { return Nodes.lookup(BB); } + BlockNode getNode(const BlockT *BB) const { + assert(BlockNumberEpoch == + GraphTraits::getNumberEpoch(F)); + unsigned BlockNumber = GraphTraits::getNumber(BB); + return BlockNumber < Nodes.size() ? Nodes[BlockNumber] : BlockNode(); + } const BlockT *getBlock(const BlockNode &Node) const { assert(Node.Index < RPOT.size()); @@ -1020,16 +1019,6 @@ public: void setBlockFreq(const BlockT *BB, BlockFrequency Freq); - void forgetBlock(const BlockT *BB) { - // We don't erase corresponding items from `Freqs`, `RPOT` and other to - // avoid invalidating indices. Doing so would have saved some memory, but - // it's not worth it. - auto It = Nodes.find(BB); - assert(It != Nodes.end() && "cannot forget block that was never seen"); - RPOT[It->second.Index] = {}; // Clear value handle. - Nodes.erase(It); - } - Scaled64 getFloatingBlockFreq(const BlockT *BB) const { return BlockFrequencyInfoImplBase::getFloatingBlockFreq(getNode(BB)); } @@ -1054,45 +1043,6 @@ public: void verifyMatch(BlockFrequencyInfoImpl &Other) const; }; -namespace bfi_detail { - -template -class BFICallbackVH : public CallbackVH { - BFIImplT *BFIImpl; - -public: - BFICallbackVH() = default; - - BFICallbackVH(const BasicBlock *BB, BFIImplT *BFIImpl) - : CallbackVH(BB), BFIImpl(BFIImpl) {} - - virtual ~BFICallbackVH() = default; - - void deleted() override { - BFIImpl->forgetBlock(cast(getValPtr())); - } - - operator const BasicBlock *() const { - Value *V = *static_cast(this); - return cast(V); - } -}; - -/// Dummy implementation since MachineBasicBlocks aren't Values, so ValueHandles -/// don't apply to them. -template -class BFICallbackVH { - const MachineBasicBlock *MBB; - -public: - BFICallbackVH() = default; - BFICallbackVH(const MachineBasicBlock *MBB, BFIImplT *) : MBB(MBB) {} - - operator const MachineBasicBlock *() const { return MBB; } -}; - -} // end namespace bfi_detail - template void BlockFrequencyInfoImpl::calculate(const FunctionT &F, const BranchProbabilityInfoT &BPI, @@ -1130,44 +1080,48 @@ void BlockFrequencyInfoImpl::calculate(const FunctionT &F, // blocks, if any. This is to distinguish between known/existing unreachable // blocks and unknown blocks. for (const BlockT &BB : F) - if (!Nodes.count(&BB)) + if (!getNode(&BB).isValid()) setBlockFreq(&BB, BlockFrequency()); } + + RPOT.clear(); } template void BlockFrequencyInfoImpl::setBlockFreq(const BlockT *BB, BlockFrequency Freq) { - auto [It, Inserted] = Nodes.try_emplace(BB); - if (!Inserted) - BlockFrequencyInfoImplBase::setBlockFreq(It->second, Freq); - else { + assert(BlockNumberEpoch == GraphTraits::getNumberEpoch(F)); + unsigned BlockNumber = GraphTraits::getNumber(BB); + if (Nodes.size() <= BlockNumber) + Nodes.resize(GraphTraits::getMaxNumber(F)); + BlockNode &Node = Nodes[BlockNumber]; + if (!Node.isValid()) { // If BB is a newly added block after BFI is done, we need to create a new // BlockNode for it assigned with a new index. The index can be determined // by the size of Freqs. - BlockNode NewNode(Freqs.size()); - It->second = NewNode; + Node = BlockNode(Freqs.size()); Freqs.emplace_back(); - RPOT.emplace_back(BB, this); - BlockFrequencyInfoImplBase::setBlockFreq(NewNode, Freq); } + BlockFrequencyInfoImplBase::setBlockFreq(Node, Freq); } template void BlockFrequencyInfoImpl::initializeRPOT() { const BlockT *Entry = &F->front(); RPOT.reserve(F->size()); for (const BlockT *BB : post_order(Entry)) - RPOT.emplace_back(BB, this); + RPOT.emplace_back(BB); std::reverse(RPOT.begin(), RPOT.end()); assert(RPOT.size() - 1 <= BlockNode::getMaxIndex() && "More nodes in function than Block Frequency Info supports"); LLVM_DEBUG(dbgs() << "reverse-post-order-traversal\n"); + Nodes.resize(GraphTraits::getMaxNumber(F)); + BlockNumberEpoch = GraphTraits::getNumberEpoch(F); for (auto [Idx, Block] : enumerate(RPOT)) { BlockNode Node = BlockNode(Idx); LLVM_DEBUG(dbgs() << " - " << Idx << ": " << getBlockName(Node) << "\n"); - Nodes[Block] = Node; + Nodes[GraphTraits::getNumber(Block)] = Node; } Working.reserve(RPOT.size()); @@ -1716,48 +1670,44 @@ template void BlockFrequencyInfoImpl::verifyMatch( BlockFrequencyInfoImpl &Other) const { bool Match = true; - DenseMap ValidNodes; - DenseMap OtherValidNodes; - for (auto &Entry : Nodes) { - const BlockT *BB = Entry.first; - if (BB) { - ValidNodes[BB] = Entry.second; - } - } - for (auto &Entry : Other.Nodes) { - const BlockT *BB = Entry.first; - if (BB) { - OtherValidNodes[BB] = Entry.second; - } - } - unsigned NumValidNodes = ValidNodes.size(); - unsigned NumOtherValidNodes = OtherValidNodes.size(); - if (NumValidNodes != NumOtherValidNodes) { - Match = false; - dbgs() << "Number of blocks mismatch: " << NumValidNodes << " vs " - << NumOtherValidNodes << "\n"; - } else { - for (auto &Entry : ValidNodes) { - const BlockT *BB = Entry.first; - BlockNode Node = Entry.second; - if (auto It = OtherValidNodes.find(BB); It != OtherValidNodes.end()) { - BlockNode OtherNode = It->second; - const auto &Freq = Freqs[Node.Index]; - const auto &OtherFreq = Other.Freqs[OtherNode.Index]; - if (Freq.Integer != OtherFreq.Integer) { - Match = false; - dbgs() << "Freq mismatch: " << bfi_detail::getBlockName(BB) << " " - << Freq.Integer << " vs " << OtherFreq.Integer << "\n"; - } - } else { + // Gather blocks for numbers so that we can print names. + SmallVector Blocks; + Blocks.resize(GraphTraits::getMaxNumber(F)); + for (const auto &BB : *F) + Blocks[GraphTraits::getNumber(&BB)] = &BB; + + size_t MinSize = std::min(Nodes.size(), Other.Nodes.size()); + for (size_t i = 0; i < MinSize; ++i) { + if (Nodes[i].isValid() != Other.Nodes[i].isValid()) { + Match = false; + dbgs() << "Block " << bfi_detail::getBlockName(Blocks[i]) + << " existence mismatch.\n"; + } else if (Nodes[i].isValid()) { + const auto &Freq = Freqs[Nodes[i].Index]; + const auto &OtherFreq = Other.Freqs[Other.Nodes[i].Index]; + if (Freq.Integer != OtherFreq.Integer) { Match = false; - dbgs() << "Block " << bfi_detail::getBlockName(BB) << " index " - << Node.Index << " does not exist in Other.\n"; + dbgs() << "Freq mismatch: " << bfi_detail::getBlockName(Blocks[i]) + << " " << Freq.Integer << " vs " << OtherFreq.Integer << "\n"; } } - // If there's a valid node in OtherValidNodes that's not in ValidNodes, - // either the above num check or the check on OtherValidNodes will fail. } + // Block with higher numbers must not exist in either state. + for (size_t i = MinSize; i < Nodes.size(); ++i) { + if (Nodes[i].isValid()) { + Match = false; + dbgs() << "Block " << bfi_detail::getBlockName(Blocks[i]) + << " existence mismatch.\n"; + } + } + for (size_t i = MinSize; i < Other.Nodes.size(); ++i) { + if (Other.Nodes[i].isValid()) { + Match = false; + dbgs() << "Block " << bfi_detail::getBlockName(Blocks[i]) + << " existence mismatch.\n"; + } + } + if (!Match) { dbgs() << "This\n"; print(dbgs());