[llvm] Require GraphHasNodeNumbers in llvm::LoopInfoBase; drop the DenseMap fallback (PR #207905)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 9 00:32:06 PDT 2026
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/207905
>From 7d92ebadc1ca7230d9a137f325f2f96eb2005c44 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Mon, 6 Jul 2026 23:20:53 -0700
Subject: [PATCH 1/2] Require GraphHasNodeNumbers in llvm::LoopInfoBase; drop
the DenseMap fallback
LoopInfoBase kept a DenseMap fallback for block types whose GraphTraits
has no getNumber() (std::conditional_t on GraphHasNodeNumbers). With the
last such in-tree users now numbered (mlir::Block and BOLT's
BinaryBasicBlock), require GraphHasNodeNumbers via static_assert and make
BBMap unconditionally a SmallVector indexed by block number, dropping the
DenseMap branch in getLoopFor/changeLoopFor/removeBlock/analyze/verify.
Depends on numbering mlir::Block and bolt::BinaryBasicBlock.
Aided by Claude Opus 4.8
---
llvm/include/llvm/Support/GenericLoopInfo.h | 78 +++++++------------
.../llvm/Support/GenericLoopInfoImpl.h | 51 +++++-------
2 files changed, 47 insertions(+), 82 deletions(-)
diff --git a/llvm/include/llvm/Support/GenericLoopInfo.h b/llvm/include/llvm/Support/GenericLoopInfo.h
index 9b3bcd09e289d..faebdcb526e0d 100644
--- a/llvm/include/llvm/Support/GenericLoopInfo.h
+++ b/llvm/include/llvm/Support/GenericLoopInfo.h
@@ -524,10 +524,13 @@ raw_ostream &operator<<(raw_ostream &OS, const LoopBase<BlockT, LoopT> &Loop) {
///
template <class BlockT, class LoopT> class LoopInfoBase {
- // BBMap - Mapping of basic blocks to the inner most loop they occur in
- std::conditional_t<GraphHasNodeNumbers<const BlockT *>, SmallVector<LoopT *>,
- DenseMap<const BlockT *, LoopT *>>
- BBMap;
+ static_assert(GraphHasNodeNumbers<const BlockT *>,
+ "LoopInfo requires GraphTraits<BlockT *>::getNumber (see "
+ "GraphHasNodeNumbers)");
+
+ // Mapping of each block, indexed by its number, to the innermost loop it
+ // occurs in (or null).
+ SmallVector<LoopT *> BBMap;
using ParentT = decltype(std::declval<const BlockT *>()->getParent());
ParentT ParentPtr = nullptr;
@@ -615,25 +618,20 @@ template <class BlockT, class LoopT> class LoopInfoBase {
private:
/// Verify that used block numbers are still valid.
void verifyBlockNumberEpoch(ParentT BBParent) const {
- if constexpr (GraphHasNodeNumbers<BlockT *>) {
- assert(ParentPtr == BBParent &&
- "loop info queried with block of other function");
- assert(BlockNumberEpoch ==
- GraphTraits<ParentT>::getNumberEpoch(ParentPtr) &&
- "loop info used with outdated block numbers");
- }
+ assert(ParentPtr == BBParent &&
+ "loop info queried with block of other function");
+ assert(BlockNumberEpoch ==
+ GraphTraits<ParentT>::getNumberEpoch(ParentPtr) &&
+ "loop info used with outdated block numbers");
}
public:
/// Return the inner most loop that BB lives in. If a basic block is in no
/// loop (for example the entry node), null is returned.
LoopT *getLoopFor(const BlockT *BB) const {
- if constexpr (GraphHasNodeNumbers<const BlockT *>) {
- verifyBlockNumberEpoch(BB->getParent());
- unsigned Number = GraphTraits<const BlockT *>::getNumber(BB);
- return Number < BBMap.size() ? BBMap[Number] : nullptr;
- } else
- return BBMap.lookup(BB);
+ verifyBlockNumberEpoch(BB->getParent());
+ unsigned Number = GraphTraits<const BlockT *>::getNumber(BB);
+ return Number < BBMap.size() ? BBMap[Number] : nullptr;
}
/// Same as getLoopFor.
@@ -684,22 +682,14 @@ template <class BlockT, class LoopT> class LoopInfoBase {
/// This should be used by transformations that restructure the loop hierarchy
/// tree.
void changeLoopFor(const BlockT *BB, LoopT *L) {
- if constexpr (GraphHasNodeNumbers<const BlockT *>) {
- verifyBlockNumberEpoch(BB->getParent());
- unsigned Number = GraphTraits<const BlockT *>::getNumber(BB);
- if (Number >= BBMap.size()) {
- unsigned Max = GraphTraits<decltype(BB->getParent())>::getMaxNumber(
- BB->getParent());
- BBMap.resize(Number >= Max ? Number + 1 : Max);
- }
- BBMap[Number] = L;
- } else {
- if (!L) {
- BBMap.erase(BB);
- return;
- }
- BBMap[BB] = L;
+ verifyBlockNumberEpoch(BB->getParent());
+ unsigned Number = GraphTraits<const BlockT *>::getNumber(BB);
+ if (Number >= BBMap.size()) {
+ unsigned Max =
+ GraphTraits<decltype(BB->getParent())>::getMaxNumber(BB->getParent());
+ BBMap.resize(Number >= Max ? Number + 1 : Max);
}
+ BBMap[Number] = L;
}
/// Replace the specified loop in the top-level loops list with the indicated
@@ -722,24 +712,14 @@ template <class BlockT, class LoopT> class LoopInfoBase {
/// including all of the Loop objects it is nested in and our mapping from
/// BasicBlocks to loops.
void removeBlock(BlockT *BB) {
- if constexpr (GraphHasNodeNumbers<BlockT *>) {
- verifyBlockNumberEpoch(BB->getParent());
- unsigned Number = GraphTraits<BlockT *>::getNumber(BB);
- if (Number >= BBMap.size())
- return;
-
- for (LoopT *L = BBMap[Number]; L; L = L->getParentLoop())
- L->removeBlockFromLoop(BB);
- BBMap[Number] = nullptr;
- } else {
- auto I = BBMap.find(BB);
- if (I != BBMap.end()) {
- for (LoopT *L = I->second; L; L = L->getParentLoop())
- L->removeBlockFromLoop(BB);
+ verifyBlockNumberEpoch(BB->getParent());
+ unsigned Number = GraphTraits<BlockT *>::getNumber(BB);
+ if (Number >= BBMap.size())
+ return;
- BBMap.erase(I);
- }
- }
+ for (LoopT *L = BBMap[Number]; L; L = L->getParentLoop())
+ L->removeBlockFromLoop(BB);
+ BBMap[Number] = nullptr;
}
// Internals
diff --git a/llvm/include/llvm/Support/GenericLoopInfoImpl.h b/llvm/include/llvm/Support/GenericLoopInfoImpl.h
index 5d827c1cab8c2..e0a06ad75842c 100644
--- a/llvm/include/llvm/Support/GenericLoopInfoImpl.h
+++ b/llvm/include/llvm/Support/GenericLoopInfoImpl.h
@@ -577,12 +577,9 @@ void PopulateLoopsDFS<BlockT, LoopT>::insertIntoLoop(BlockT *Block) {
template <class BlockT, class LoopT>
void LoopInfoBase<BlockT, LoopT>::analyze(const DomTreeBase<BlockT> &DomTree) {
const DomTreeNodeBase<BlockT> *DomRoot = DomTree.getRootNode();
- if constexpr (GraphHasNodeNumbers<const BlockT *>) {
- ParentPtr = DomRoot->getBlock()->getParent();
- BlockNumberEpoch = GraphTraits<ParentT>::getNumberEpoch(ParentPtr);
- unsigned Max = GraphTraits<ParentT>::getMaxNumber(ParentPtr);
- BBMap.resize(Max);
- }
+ ParentPtr = DomRoot->getBlock()->getParent();
+ BlockNumberEpoch = GraphTraits<ParentT>::getNumberEpoch(ParentPtr);
+ BBMap.resize(GraphTraits<ParentT>::getMaxNumber(ParentPtr));
// Visit dominator tree nodes in reverse preorder: like postorder, this
// guarantees a sub-loop is discovered before the outer loop.
@@ -762,33 +759,21 @@ void LoopInfoBase<BlockT, LoopT>::verify(
// Verify that blocks are mapped to valid loops.
#ifndef NDEBUG
- if constexpr (GraphHasNodeNumbers<const BlockT *>) {
- for (auto It : enumerate(BBMap)) {
- LoopT *L = It.value();
- unsigned Number = It.index();
- if (!L)
- continue;
- assert(Loops.count(L) && "orphaned loop");
- // We have no way to map block numbers back to blocks, so find it.
- auto BBIt = find_if(L->Blocks, [&Number](BlockT *BB) {
- return GraphTraits<BlockT *>::getNumber(BB) == Number;
- });
- BlockT *BB = BBIt != L->Blocks.end() ? *BBIt : nullptr;
- assert(BB && "orphaned block");
- for (LoopT *ChildLoop : *L)
- assert(!ChildLoop->contains(BB) &&
- "BBMap should point to the innermost loop containing BB");
- }
- } else {
- for (auto &Entry : BBMap) {
- const BlockT *BB = Entry.first;
- LoopT *L = Entry.second;
- assert(Loops.count(L) && "orphaned loop");
- assert(L->contains(BB) && "orphaned block");
- for (LoopT *ChildLoop : *L)
- assert(!ChildLoop->contains(BB) &&
- "BBMap should point to the innermost loop containing BB");
- }
+ for (auto It : enumerate(BBMap)) {
+ LoopT *L = It.value();
+ unsigned Number = It.index();
+ if (!L)
+ continue;
+ assert(Loops.count(L) && "orphaned loop");
+ // We have no way to map block numbers back to blocks, so find it.
+ auto BBIt = find_if(L->Blocks, [&Number](BlockT *BB) {
+ return GraphTraits<BlockT *>::getNumber(BB) == Number;
+ });
+ BlockT *BB = BBIt != L->Blocks.end() ? *BBIt : nullptr;
+ assert(BB && "orphaned block");
+ for (LoopT *ChildLoop : *L)
+ assert(!ChildLoop->contains(BB) &&
+ "BBMap should point to the innermost loop containing BB");
}
// Recompute LoopInfo to verify loops structure.
>From faa11c718e1453a0f552160d9ad5cdfad0532c9d Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Thu, 9 Jul 2026 00:31:55 -0700
Subject: [PATCH 2/2] BBMap.resize(Max)
---
llvm/include/llvm/Support/GenericLoopInfo.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Support/GenericLoopInfo.h b/llvm/include/llvm/Support/GenericLoopInfo.h
index faebdcb526e0d..77f9cd731f71f 100644
--- a/llvm/include/llvm/Support/GenericLoopInfo.h
+++ b/llvm/include/llvm/Support/GenericLoopInfo.h
@@ -687,7 +687,8 @@ template <class BlockT, class LoopT> class LoopInfoBase {
if (Number >= BBMap.size()) {
unsigned Max =
GraphTraits<decltype(BB->getParent())>::getMaxNumber(BB->getParent());
- BBMap.resize(Number >= Max ? Number + 1 : Max);
+ assert(Number < Max);
+ BBMap.resize(Max);
}
BBMap[Number] = L;
}
More information about the llvm-commits
mailing list