[llvm] [GVN] use `AssertingVH` for leaders to improve compilation time (PR #175870)
Princeton Ferro via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 1 16:55:24 PDT 2026
https://github.com/Prince781 updated https://github.com/llvm/llvm-project/pull/175870
>From 87d5ab3fe8c00b18e773f3ec9f0fcd0e4c0002db Mon Sep 17 00:00:00 2001
From: Princeton Ferro <pferro at nvidia.com>
Date: Wed, 21 Jan 2026 13:03:57 -0800
Subject: [PATCH] [GVN] use AssertingVH for leaders to improve compilation time
Occasionally we see code where a lot of instructions share the same
value number, making the iteration in verifyRemoved() very slow (e.g.
~1hr on internal kernels). Use AssertingVH for the Val field in
LeaderTableEntry so that verification becomes implicit: if a value is
deleted while still tracked in the leader table, the handle asserts
immediately at deletion rather than requiring a linear scan.
To avoid a compile-time regression from storing leader nodes by pointer
(which was previously needed because AssertingVH could not be moved),
add move constructor and move assignment to AssertingVH and its base
class ValueHandleBase. In ABI-breaking mode the move properly
deregisters the source handle from the per-Value use list and registers
the destination; in non-ABI-breaking mode it reduces to std::exchange
on a raw pointer. This lets DenseMap store LeaderListNode by value
(as before), preserving the original cache-friendly layout with no
extra allocation per entry.
Measured on CTMark: -0.008% instructions:u (within noise).
Co-Authored-By: Claude Sonnet 4.6 <noreply at anthropic.com>
---
llvm/include/llvm/IR/ValueHandle.h | 43 +++++++++++++++++
llvm/include/llvm/Transforms/Scalar/GVN.h | 27 +++++++++--
llvm/lib/Transforms/Scalar/GVN.cpp | 56 ++++++++++-------------
3 files changed, 90 insertions(+), 36 deletions(-)
diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index 05555bd5efb1b..a353e6e8349e5 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -19,6 +19,7 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/Compiler.h"
#include <cassert>
+#include <utility>
namespace llvm {
@@ -46,6 +47,15 @@ class ValueHandleBase {
AddToExistingUseList(RHS.getPrevPtr());
}
+ ValueHandleBase(HandleBaseKind Kind, ValueHandleBase &&RHS)
+ : PrevPair(nullptr, Kind), Val(RHS.getValPtr()) {
+ if (isValid(getValPtr())) {
+ RHS.RemoveFromUseList();
+ RHS.clearValPtr();
+ AddToUseList();
+ }
+ }
+
private:
PointerIntPair<ValueHandleBase**, 2, HandleBaseKind> PrevPair;
ValueHandleBase *Next = nullptr;
@@ -89,6 +99,26 @@ class ValueHandleBase {
return getValPtr();
}
+ Value *operator=(ValueHandleBase &&RHS) {
+ if (getValPtr() == RHS.getValPtr()) {
+ if (this != &RHS) {
+ if (isValid(RHS.getValPtr()))
+ RHS.RemoveFromUseList();
+ RHS.clearValPtr();
+ }
+ return getValPtr();
+ }
+ if (isValid(getValPtr()))
+ RemoveFromUseList();
+ setValPtr(RHS.getValPtr());
+ if (isValid(getValPtr())) {
+ RHS.RemoveFromUseList();
+ RHS.clearValPtr();
+ AddToUseList();
+ }
+ return getValPtr();
+ }
+
Value *operator->() const { return getValPtr(); }
Value &operator*() const {
Value *V = getValPtr();
@@ -285,10 +315,12 @@ class AssertingVH
AssertingVH() : ValueHandleBase(Assert) {}
AssertingVH(ValueTy *P) : ValueHandleBase(Assert, GetAsValue(P)) {}
AssertingVH(const AssertingVH &RHS) : ValueHandleBase(Assert, RHS) {}
+ AssertingVH(AssertingVH &&RHS) : ValueHandleBase(Assert, std::move(RHS)) {}
#else
AssertingVH() : ThePtr(nullptr) {}
AssertingVH(ValueTy *P) : ThePtr(GetAsValue(P)) {}
AssertingVH(const AssertingVH &) = default;
+ AssertingVH(AssertingVH &&RHS) : ThePtr(std::exchange(RHS.ThePtr, nullptr)) {}
#endif
operator ValueTy*() const {
@@ -303,6 +335,17 @@ class AssertingVH
setValPtr(RHS.getValPtr());
return getValPtr();
}
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
+ ValueTy *operator=(AssertingVH<ValueTy> &&RHS) {
+ ValueHandleBase::operator=(std::move(RHS));
+ return getValPtr();
+ }
+#else
+ ValueTy *operator=(AssertingVH<ValueTy> &&RHS) {
+ ThePtr = std::exchange(RHS.ThePtr, nullptr);
+ return getValPtr();
+ }
+#endif
ValueTy *operator->() const { return getValPtr(); }
ValueTy &operator*() const { return *getValPtr(); }
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index f79896e993d9b..b0ae10e2006c5 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -263,15 +263,24 @@ class GVNPass : public PassInfoMixin<GVNPass> {
/// have that value number. Use findLeader to query it.
class LeaderMap {
public:
- struct LeaderTableEntry {
- Value *Val;
+ class LeaderTableEntry {
+ public:
+ /// Assigning a Value* here registers a callback on the Value* that will
+ /// trigger an assertion when it's deleted as long as this AssertingVH
+ /// remains live. This catches any cases where GVN fails to remove a
+ /// Value* from the leader table after removing it from the IR.
+ AssertingVH<Value> Val;
const BasicBlock *BB;
+ LeaderTableEntry(Value *V, const BasicBlock *BB) : Val(V), BB(BB) {}
};
private:
- struct LeaderListNode {
+ class LeaderListNode {
+ public:
LeaderTableEntry Entry;
LeaderListNode *Next;
+ LeaderListNode(Value *V, const BasicBlock *BB, LeaderListNode *Next)
+ : Entry(V, BB), Next(Next) {}
};
DenseMap<uint32_t, LeaderListNode> NumToLeaders;
BumpPtrAllocator TableAllocator;
@@ -315,8 +324,18 @@ 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() {
+ // Manually destroy non-head nodes (in BumpPtrAllocator) to properly
+ // clean up AssertingVH handles before Reset(). Head nodes are destroyed
+ // by NumToLeaders.clear() below.
+ for (auto &[_, HeadNode] : NumToLeaders) {
+ LeaderListNode *N = HeadNode.Next;
+ while (N) {
+ auto *Next = N->Next;
+ N->~LeaderListNode();
+ N = Next;
+ }
+ }
NumToLeaders.clear();
TableAllocator.Reset();
}
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 7cab4be169123..7efea0999ebdf 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -789,26 +789,25 @@ void GVNPass::ValueTable::verifyRemoved(const Value *V) const {
/// Push a new Value to the LeaderTable onto the list for its value number.
void GVNPass::LeaderMap::insert(uint32_t N, Value *V, const BasicBlock *BB) {
- LeaderListNode &Curr = NumToLeaders[N];
- if (!Curr.Entry.Val) {
- Curr.Entry.Val = V;
- Curr.Entry.BB = BB;
- return;
+ auto [It, Inserted] = NumToLeaders.try_emplace(N, V, BB, nullptr);
+ if (!Inserted) {
+ // Key already exists: insert new node after the head.
+ auto *NewSlot = TableAllocator.Allocate<LeaderListNode>();
+ new (NewSlot) LeaderListNode(V, BB, It->second.Next);
+ It->second.Next = NewSlot;
}
-
- LeaderListNode *Node = TableAllocator.Allocate<LeaderListNode>();
- Node->Entry.Val = V;
- Node->Entry.BB = BB;
- Node->Next = Curr.Next;
- Curr.Next = Node;
}
/// Scan the list of values corresponding to a given
/// value number, and remove the given instruction if encountered.
void GVNPass::LeaderMap::erase(uint32_t N, Instruction *I,
const BasicBlock *BB) {
+ auto It = NumToLeaders.find(N);
+ if (It == NumToLeaders.end())
+ return;
+
LeaderListNode *Prev = nullptr;
- LeaderListNode *Curr = &NumToLeaders[N];
+ LeaderListNode *Curr = &It->second; // head is stored by value; take address
while (Curr && (Curr->Entry.Val != I || Curr->Entry.BB != BB)) {
Prev = Curr;
@@ -819,33 +818,27 @@ void GVNPass::LeaderMap::erase(uint32_t N, Instruction *I,
return;
if (Prev) {
+ // Non-head node: unlink and destroy
Prev->Next = Curr->Next;
+ Curr->~LeaderListNode();
+ TableAllocator.Deallocate<LeaderListNode>(Curr);
} else {
+ // Head node (stored by value in DenseMap)
if (!Curr->Next) {
- Curr->Entry.Val = nullptr;
- Curr->Entry.BB = nullptr;
+ // Only node; erase from map (DenseMap calls the destructor)
+ NumToLeaders.erase(It);
} else {
+ // Move second node's data into head, then destroy second node
LeaderListNode *Next = Curr->Next;
- Curr->Entry.Val = Next->Entry.Val;
- Curr->Entry.BB = Next->Entry.BB;
- Curr->Next = Next->Next;
+ It->second.Entry.Val = std::move(Next->Entry.Val);
+ It->second.Entry.BB = Next->Entry.BB;
+ It->second.Next = Next->Next;
+ Next->~LeaderListNode();
+ TableAllocator.Deallocate<LeaderListNode>(Next);
}
}
}
-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
//===----------------------------------------------------------------------===//
@@ -2287,7 +2280,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;
}
@@ -3211,7 +3204,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
More information about the llvm-commits
mailing list