[llvm] [GVN] add reverse leader map to improve compile time (PR #175870)
Princeton Ferro via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 20 19:46:44 PST 2026
https://github.com/Prince781 updated https://github.com/llvm/llvm-project/pull/175870
>From 98d7420697b8b0ccfd16eb8fcb1c959be5751336 Mon Sep 17 00:00:00 2001
From: Princeton Ferro <pferro at nvidia.com>
Date: Mon, 12 Jan 2026 14:10:35 -0800
Subject: [PATCH 1/2] [GVN] use AssertingVH to improve compilation time
The leader map can become very large on unrolled code, making iteration
in verifyRemoved() very slow. Replace this with AssertingVH which checks
that leaders aren't deleted before they're removed form the leader map.
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 9 ++++++++-
llvm/lib/Transforms/Scalar/GVN.cpp | 16 +---------------
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index bc0f108ac8260..9d79edaf1fd0b 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -264,7 +264,13 @@ class GVNPass : public PassInfoMixin<GVNPass> {
class LeaderMap {
public:
struct LeaderTableEntry {
+#ifndef NDEBUG
+ // Use an AssertingVH to ensure leaders aren't deleted before they're
+ // removed from the table.
+ AssertingVH<Value> Val;
+#else
Value *Val;
+#endif
const BasicBlock *BB;
};
@@ -315,7 +321,6 @@ class GVNPass : public PassInfoMixin<GVNPass> {
LLVM_ABI void insert(uint32_t N, Value *V, const BasicBlock *BB);
LLVM_ABI void erase(uint32_t N, Instruction *I, const BasicBlock *BB);
- LLVM_ABI void verifyRemoved(const Value *Inst) const;
void clear() {
NumToLeaders.clear();
TableAllocator.Reset();
@@ -395,7 +400,9 @@ class GVNPass : public PassInfoMixin<GVNPass> {
Value *findLeader(const BasicBlock *BB, uint32_t Num);
void cleanupGlobalSets();
void removeInstruction(Instruction *I);
+#ifndef NDEBUG
void verifyRemoved(const Instruction *I) const;
+#endif
bool splitCriticalEdges();
BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ);
bool
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 72e1131a54a86..c6bc8cbeec347 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -833,19 +833,6 @@ void GVNPass::LeaderMap::erase(uint32_t N, Instruction *I,
}
}
-void GVNPass::LeaderMap::verifyRemoved(const Value *V) const {
- // Walk through the value number scope to make sure the instruction isn't
- // ferreted away in it.
- for (const auto &I : NumToLeaders) {
- (void)I;
- assert(I.second.Entry.Val != V && "Inst still in value numbering scope!");
- assert(
- std::none_of(leader_iterator(&I.second), leader_iterator(nullptr),
- [=](const LeaderTableEntry &E) { return E.Val == V; }) &&
- "Inst still in value numbering scope!");
- }
-}
-
//===----------------------------------------------------------------------===//
// GVN Pass
//===----------------------------------------------------------------------===//
@@ -2277,7 +2264,7 @@ bool GVNPass::ValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
CallInst *Call = nullptr;
auto Leaders = GVN.LeaderTable.getLeaders(Num);
for (const auto &Entry : Leaders) {
- Call = dyn_cast<CallInst>(Entry.Val);
+ Call = dyn_cast<CallInst>(&*Entry.Val);
if (Call && Call->getParent() == PhiBlock)
break;
}
@@ -3198,7 +3185,6 @@ void GVNPass::removeInstruction(Instruction *I) {
/// internal data structures.
void GVNPass::verifyRemoved(const Instruction *Inst) const {
VN.verifyRemoved(Inst);
- LeaderTable.verifyRemoved(Inst);
}
/// BB is declared dead, which implied other blocks become dead as well. This
>From bb16ba467acbf9cc345a757207736611ad790d1e Mon Sep 17 00:00:00 2001
From: Princeton Ferro <pferro at nvidia.com>
Date: Tue, 20 Jan 2026 15:30:30 -0800
Subject: [PATCH 2/2] remove NDEBUG and use AssertingVH in ValueNumbering map
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 10 +--------
llvm/lib/Transforms/Scalar/GVN.cpp | 25 +++--------------------
2 files changed, 4 insertions(+), 31 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 9d79edaf1fd0b..aab65ff335c52 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -159,7 +159,7 @@ class GVNPass : public PassInfoMixin<GVNPass> {
/// as an efficient mechanism to determine the expression-wise equivalence of
/// two values.
class ValueTable {
- DenseMap<Value *, uint32_t> ValueNumbering;
+ DenseMap<AssertingVH<Value>, uint32_t> ValueNumbering;
DenseMap<Expression, uint32_t> ExpressionNumbering;
// Expressions is the vector of Expression. ExprIdx is the mapping from
@@ -240,7 +240,6 @@ class GVNPass : public PassInfoMixin<GVNPass> {
}
void setDomTree(DominatorTree *D) { DT = D; }
uint32_t getNextUnusedValueNumber() { return NextValueNumber; }
- LLVM_ABI void verifyRemoved(const Value *) const;
};
private:
@@ -264,13 +263,9 @@ class GVNPass : public PassInfoMixin<GVNPass> {
class LeaderMap {
public:
struct LeaderTableEntry {
-#ifndef NDEBUG
// Use an AssertingVH to ensure leaders aren't deleted before they're
// removed from the table.
AssertingVH<Value> Val;
-#else
- Value *Val;
-#endif
const BasicBlock *BB;
};
@@ -400,9 +395,6 @@ class GVNPass : public PassInfoMixin<GVNPass> {
Value *findLeader(const BasicBlock *BB, uint32_t Num);
void cleanupGlobalSets();
void removeInstruction(Instruction *I);
-#ifndef NDEBUG
- void verifyRemoved(const Instruction *I) const;
-#endif
bool splitCriticalEdges();
BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ);
bool
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index c6bc8cbeec347..4b046acf1e9be 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -648,7 +648,7 @@ uint32_t GVNPass::ValueTable::lookupOrAdd(MemoryAccess *MA) {
/// lookupOrAdd - Returns the value number for the specified value, assigning
/// it a new number if it did not have one before.
uint32_t GVNPass::ValueTable::lookupOrAdd(Value *V) {
- DenseMap<Value *, uint32_t>::iterator VI = ValueNumbering.find(V);
+ auto VI = ValueNumbering.find(V);
if (VI != ValueNumbering.end())
return VI->second;
@@ -733,7 +733,7 @@ uint32_t GVNPass::ValueTable::lookupOrAdd(Value *V) {
/// Returns the value number of the specified value. Fails if
/// the value has not yet been numbered.
uint32_t GVNPass::ValueTable::lookup(Value *V, bool Verify) const {
- DenseMap<Value *, uint32_t>::const_iterator VI = ValueNumbering.find(V);
+ auto VI = ValueNumbering.find(V);
if (Verify) {
assert(VI != ValueNumbering.end() && "Value not numbered?");
return VI->second;
@@ -776,13 +776,6 @@ void GVNPass::ValueTable::erase(Value *V) {
NumberingBB.erase(Num);
}
-/// verifyRemoved - Verify that the value is removed from all internal data
-/// structures.
-void GVNPass::ValueTable::verifyRemoved(const Value *V) const {
- assert(!ValueNumbering.contains(V) &&
- "Inst still occurs in value numbering map!");
-}
-
//===----------------------------------------------------------------------===//
// LeaderMap External Functions
//===----------------------------------------------------------------------===//
@@ -796,7 +789,7 @@ void GVNPass::LeaderMap::insert(uint32_t N, Value *V, const BasicBlock *BB) {
return;
}
- LeaderListNode *Node = TableAllocator.Allocate<LeaderListNode>();
+ auto *Node = new (TableAllocator.Allocate<LeaderListNode>()) LeaderListNode();
Node->Entry.Val = V;
Node->Entry.BB = BB;
Node->Next = Curr.Next;
@@ -3034,9 +3027,6 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) {
PREInstr = CurInst->clone();
if (!performScalarPREInsertion(PREInstr, PREPred, CurrentBlock, ValNo)) {
// If we failed insertion, make sure we remove the instruction.
-#ifndef NDEBUG
- verifyRemoved(PREInstr);
-#endif
PREInstr->deleteValue();
return false;
}
@@ -3174,19 +3164,10 @@ void GVNPass::removeInstruction(Instruction *I) {
if (MD) MD->removeInstruction(I);
if (MSSAU)
MSSAU->removeMemoryAccess(I);
-#ifndef NDEBUG
- verifyRemoved(I);
-#endif
ICF->removeInstruction(I);
I->eraseFromParent();
}
-/// Verify that the specified instruction does not occur in our
-/// internal data structures.
-void GVNPass::verifyRemoved(const Instruction *Inst) const {
- VN.verifyRemoved(Inst);
-}
-
/// BB is declared dead, which implied other blocks become dead as well. This
/// function is to add all these blocks to "DeadBlocks". For the dead blocks'
/// live successors, update their phi nodes by replacing the operands
More information about the llvm-commits
mailing list