[llvm] [Analysis][NFC] Use block numbers in BlockFrequencyInfo (PR #190669)
Alexis Engelke via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 6 13:18:48 PDT 2026
https://github.com/aengelke created https://github.com/llvm/llvm-project/pull/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.
>From 9d0544d790d6b4b26ca7156b481a4ad62049b8ea Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Mon, 6 Apr 2026 20:18:08 +0000
Subject: [PATCH] [spr] initial version
Created using spr 1.3.8-wip
---
.../llvm/Analysis/BlockFrequencyInfoImpl.h | 166 ++++++------------
1 file changed, 58 insertions(+), 108 deletions(-)
diff --git a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
index 54f4e70bf2f84..09379056f9600 100644
--- a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
+++ b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
@@ -541,7 +541,6 @@ namespace bfi_detail {
template <class BlockT> struct TypeMap {};
template <> struct TypeMap<BasicBlock> {
using BlockT = BasicBlock;
- using BlockKeyT = AssertingVH<const BasicBlock>;
using FunctionT = Function;
using BranchProbabilityInfoT = BranchProbabilityInfo;
using LoopT = Loop;
@@ -549,16 +548,12 @@ template <> struct TypeMap<BasicBlock> {
};
template <> struct TypeMap<MachineBasicBlock> {
using BlockT = MachineBasicBlock;
- using BlockKeyT = const MachineBasicBlock *;
using FunctionT = MachineFunction;
using BranchProbabilityInfoT = MachineBranchProbabilityInfo;
using LoopT = MachineLoop;
using LoopInfoT = MachineLoopInfo;
};
-template <class BlockT, class BFIImplT>
-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 BT> class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase {
using BlockT = typename bfi_detail::TypeMap<BT>::BlockT;
- using BlockKeyT = typename bfi_detail::TypeMap<BT>::BlockKeyT;
using FunctionT = typename bfi_detail::TypeMap<BT>::FunctionT;
using BranchProbabilityInfoT =
typename bfi_detail::TypeMap<BT>::BranchProbabilityInfoT;
@@ -849,18 +843,23 @@ template <class BT> class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase {
using LoopInfoT = typename bfi_detail::TypeMap<BT>::LoopInfoT;
using Successor = GraphTraits<const BlockT *>;
using Predecessor = GraphTraits<Inverse<const BlockT *>>;
- using BFICallbackVH =
- bfi_detail::BFICallbackVH<BlockT, BlockFrequencyInfoImpl>;
const BranchProbabilityInfoT *BPI = nullptr;
const LoopInfoT *LI = nullptr;
const FunctionT *F = nullptr;
// All blocks in reverse postorder.
- std::vector<BFICallbackVH> RPOT;
- DenseMap<const BlockT *, BlockNode> Nodes;
-
- BlockNode getNode(const BlockT *BB) const { return Nodes.lookup(BB); }
+ std::vector<const BlockT *> RPOT;
+ /// Map from block number to number on RPOT/Freqs.
+ SmallVector<BlockNode, 0> Nodes;
+ unsigned BlockNumberEpoch;
+
+ BlockNode getNode(const BlockT *BB) const {
+ assert(BlockNumberEpoch ==
+ GraphTraits<const FunctionT *>::getNumberEpoch(F));
+ unsigned BlockNumber = GraphTraits<const BlockT *>::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 @@ template <class BT> class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase {
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 @@ template <class BT> class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase {
void verifyMatch(BlockFrequencyInfoImpl<BT> &Other) const;
};
-namespace bfi_detail {
-
-template <class BFIImplT>
-class BFICallbackVH<BasicBlock, BFIImplT> : 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<BasicBlock>(getValPtr()));
- }
-
- operator const BasicBlock *() const {
- Value *V = *static_cast<const CallbackVH *>(this);
- return cast<BasicBlock>(V);
- }
-};
-
-/// Dummy implementation since MachineBasicBlocks aren't Values, so ValueHandles
-/// don't apply to them.
-template <class BFIImplT>
-class BFICallbackVH<MachineBasicBlock, BFIImplT> {
- const MachineBasicBlock *MBB;
-
-public:
- BFICallbackVH() = default;
- BFICallbackVH(const MachineBasicBlock *MBB, BFIImplT *) : MBB(MBB) {}
-
- operator const MachineBasicBlock *() const { return MBB; }
-};
-
-} // end namespace bfi_detail
-
template <class BT>
void BlockFrequencyInfoImpl<BT>::calculate(const FunctionT &F,
const BranchProbabilityInfoT &BPI,
@@ -1130,44 +1080,48 @@ void BlockFrequencyInfoImpl<BT>::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 <class BT>
void BlockFrequencyInfoImpl<BT>::setBlockFreq(const BlockT *BB,
BlockFrequency Freq) {
- auto [It, Inserted] = Nodes.try_emplace(BB);
- if (!Inserted)
- BlockFrequencyInfoImplBase::setBlockFreq(It->second, Freq);
- else {
+ assert(BlockNumberEpoch == GraphTraits<const FunctionT *>::getNumberEpoch(F));
+ unsigned BlockNumber = GraphTraits<const BlockT *>::getNumber(BB);
+ if (Nodes.size() <= BlockNumber)
+ Nodes.resize(GraphTraits<const FunctionT *>::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 <class BT> void BlockFrequencyInfoImpl<BT>::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<const FunctionT *>::getMaxNumber(F));
+ BlockNumberEpoch = GraphTraits<const FunctionT *>::getNumberEpoch(F);
for (auto [Idx, Block] : enumerate(RPOT)) {
BlockNode Node = BlockNode(Idx);
LLVM_DEBUG(dbgs() << " - " << Idx << ": " << getBlockName(Node) << "\n");
- Nodes[Block] = Node;
+ Nodes[GraphTraits<const BlockT *>::getNumber(Block)] = Node;
}
Working.reserve(RPOT.size());
@@ -1716,48 +1670,44 @@ template <class BT>
void BlockFrequencyInfoImpl<BT>::verifyMatch(
BlockFrequencyInfoImpl<BT> &Other) const {
bool Match = true;
- DenseMap<const BlockT *, BlockNode> ValidNodes;
- DenseMap<const BlockT *, BlockNode> OtherValidNodes;
- for (auto &Entry : Nodes) {
- const BlockT *BB = Entry.first;
- if (BB) {
- ValidNodes[BB] = Entry.second;
+ // Gather blocks for numbers so that we can print names.
+ SmallVector<const BlockT *> Blocks;
+ Blocks.resize(GraphTraits<const FunctionT *>::getMaxNumber(F));
+ for (const auto &BB : *F)
+ Blocks[GraphTraits<const BlockT *>::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() << "Freq mismatch: " << bfi_detail::getBlockName(Blocks[i])
+ << " " << Freq.Integer << " vs " << OtherFreq.Integer << "\n";
+ }
}
}
- for (auto &Entry : Other.Nodes) {
- const BlockT *BB = Entry.first;
- if (BB) {
- OtherValidNodes[BB] = Entry.second;
+ // 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";
}
}
- 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 {
- Match = false;
- dbgs() << "Block " << bfi_detail::getBlockName(BB) << " index "
- << Node.Index << " does not exist in Other.\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 there's a valid node in OtherValidNodes that's not in ValidNodes,
- // either the above num check or the check on OtherValidNodes will fail.
}
+
if (!Match) {
dbgs() << "This\n";
print(dbgs());
More information about the llvm-commits
mailing list