[llvm] 5afba80 - [GVN] use `AssertingVH` for leaders to improve compilation time (#175870)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 13 07:30:02 PDT 2026
Author: Princeton Ferro
Date: 2026-04-13T07:29:57-07:00
New Revision: 5afba800906a5e69518abfb1995567c6186cfd6d
URL: https://github.com/llvm/llvm-project/commit/5afba800906a5e69518abfb1995567c6186cfd6d
DIFF: https://github.com/llvm/llvm-project/commit/5afba800906a5e69518abfb1995567c6186cfd6d.diff
LOG: [GVN] use `AssertingVH` for leaders to improve compilation time (#175870)
Replace the manual check in `verifyRemoved()` with `AssertingVH`
instrumentation. For cases where the leader table becomes very large,
this is a cheaper way to verify we don't have dangling entries in the
leader table.
For this change, we must implement a move constructor for `AssertingVH`
so that we can keep the first entry as an inline-allocated node that
will be handled correctly as the table grows.
Added:
Modified:
llvm/include/llvm/IR/ValueHandle.h
llvm/include/llvm/Transforms/Scalar/GVN.h
llvm/lib/Transforms/Scalar/GVN.cpp
llvm/unittests/IR/ValueHandleTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index 05555bd5efb1b..c6e98d80d54cc 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -46,6 +46,15 @@ class ValueHandleBase {
AddToExistingUseList(RHS.getPrevPtr());
}
+ ValueHandleBase(HandleBaseKind Kind, ValueHandleBase &&RHS)
+ : PrevPair(nullptr, Kind), Val(RHS.getValPtr()) {
+ if (isValid(getValPtr())) {
+ AddToExistingUseList(RHS.getPrevPtr());
+ RHS.RemoveFromUseList();
+ RHS.clearValPtr();
+ }
+ }
+
private:
PointerIntPair<ValueHandleBase**, 2, HandleBaseKind> PrevPair;
ValueHandleBase *Next = nullptr;
@@ -89,6 +98,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())) {
+ AddToExistingUseList(RHS.getPrevPtr());
+ RHS.RemoveFromUseList();
+ RHS.clearValPtr();
+ }
+ return getValPtr();
+ }
+
Value *operator->() const { return getValPtr(); }
Value &operator*() const {
Value *V = getValPtr();
@@ -285,10 +314,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 +334,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..36477e41e900c 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -264,14 +264,20 @@ class GVNPass : public PassInfoMixin<GVNPass> {
class LeaderMap {
public:
struct LeaderTableEntry {
- Value *Val;
+ // Use AssertingVH here to catch dangling Value*'s in the leader table.
+ // Will crash if the value gets deleted before the AssertingVH is
+ // destroyed.
+ AssertingVH<Value> Val;
const BasicBlock *BB;
+ LeaderTableEntry(Value *V, const BasicBlock *BB) : Val(V), BB(BB) {}
};
private:
struct LeaderListNode {
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 +321,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 49165d80f0651..b19b58769ded8 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;
+ const 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;
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.Val = std::move(Next->Entry.Val);
Curr->Entry.BB = Next->Entry.BB;
Curr->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;
}
@@ -3205,7 +3198,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
diff --git a/llvm/unittests/IR/ValueHandleTest.cpp b/llvm/unittests/IR/ValueHandleTest.cpp
index 0d4d0e220b561..571991da6122e 100644
--- a/llvm/unittests/IR/ValueHandleTest.cpp
+++ b/llvm/unittests/IR/ValueHandleTest.cpp
@@ -491,6 +491,23 @@ TEST_F(ValueHandle, PoisoningVH_DoesNotFollowRAUW) {
EXPECT_TRUE(DenseMapInfo<PoisoningVH<Value>>::isEqual(VH, BitcastV.get()));
}
+TEST_F(ValueHandle, AssertingVH_MoveConstructor) {
+ AssertingVH<Value> AVH(BitcastV.get());
+ AssertingVH<Value> Moved(std::move(AVH));
+ EXPECT_EQ(BitcastV.get(), Moved);
+ // After move, AVH should be null.
+ EXPECT_EQ(nullptr, (Value *)AVH);
+}
+
+TEST_F(ValueHandle, AssertingVH_MoveAssignment) {
+ AssertingVH<Value> AVH(BitcastV.get());
+ AssertingVH<Value> Other;
+ Other = std::move(AVH);
+ EXPECT_EQ(BitcastV.get(), Other);
+ // After move, AVH should be null.
+ EXPECT_EQ(nullptr, (Value *)AVH);
+}
+
TEST_F(ValueHandle, AssertingVH_DenseMap) {
DenseMap<AssertingVH<Value>, int> Map;
Map.insert({BitcastV.get(), 1});
More information about the llvm-commits
mailing list