[llvm] [GVN] Decouple GVNValueTable and GVNPass (NFC) (PR #226232)
Momchil Velikov via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 25 06:31:44 PDT 2026
https://github.com/momchil-velikov updated https://github.com/llvm/llvm-project/pull/226232
>From 932f71c77b2f1abf73749bc2647482ea24bc4c85 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Wed, 23 Sep 2026 16:45:56 +0100
Subject: [PATCH 1/3] [GVN] Decouple GVNValueTable and GVNPass (NFC)
This is a first patch in series with the ultimate goal to move `GVNPass`
out of `GVN.h` and into `GVN.cpp.`
This is not straightforward because because of the rather tangled
dependencies between `GVNPass`, `GVNHoistPass`, `ValueTable`,
and `LeaderMap`.
The `GVNHoistPass` pass references `GVNPass::ValueTable`, by peeking
into `GVNPass`, resp. including `GVN.h`. Since `GVNHoistPass` does not
actually depend on `GVNPass` itself, but only on the `ValueTable` it
contains, it would make more sense to move `ValueTable` out of `GVN.h`
to its own header file, to be included by both `GVN.h` and
`GVNHoist.cpp.`
That's not entirely straightforward either since `ValueTable` has
several member functions taking a `GVNPass` reference as an argument.
This prevents moving `GVNPass` out of `GVN.h` and into an anonymous
namespace in `GVN.cpp`.
These `GVNPass` (in `ValueTable`) references are only used to access
the `LeaderMap`. Consequently, `LeaderMap` can be moved out of `GVNPass`
and into the `llvm` namespace and forward declared for its uses by
`ValueTable` (when `ValueTable` gets its own header).
This patch moves the `LeaderMap` out of `GVNPass`, into the `llvm`
namespace, and renames it to `GVNLeaderMap`.
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 168 +++++++++++-----------
llvm/lib/Transforms/Scalar/GVN.cpp | 35 ++---
2 files changed, 103 insertions(+), 100 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 3867afffbf550e..237bb743c6faf8 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -116,6 +116,84 @@ struct GVNOptions {
}
};
+/// A mapping from value numbers to lists of Value*'s that
+/// have that value number. Use findLeader to query it.
+class GVNLeaderMap {
+public:
+ struct LeaderTableEntry {
+ // 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;
+
+public:
+ class leader_iterator {
+ const LeaderListNode *Current;
+
+ public:
+ using iterator_category = std::forward_iterator_tag;
+ using value_type = const LeaderTableEntry;
+ using difference_type = std::ptrdiff_t;
+ using pointer = value_type *;
+ using reference = value_type &;
+
+ leader_iterator(const LeaderListNode *C) : Current(C) {}
+ leader_iterator &operator++() {
+ assert(Current && "Dereferenced end of leader list!");
+ Current = Current->Next;
+ return *this;
+ }
+ bool operator==(const leader_iterator &Other) const {
+ return Current == Other.Current;
+ }
+ bool operator!=(const leader_iterator &Other) const {
+ return Current != Other.Current;
+ }
+ reference operator*() const { return Current->Entry; }
+ };
+
+ iterator_range<leader_iterator> getLeaders(uint32_t N) {
+ auto I = NumToLeaders.find(N);
+ if (I == NumToLeaders.end()) {
+ return iterator_range(leader_iterator(nullptr), leader_iterator(nullptr));
+ }
+
+ return iterator_range(leader_iterator(&I->second),
+ leader_iterator(nullptr));
+ }
+
+ LLVM_ABI void insert(uint32_t N, Value *V, const BasicBlock *BB);
+ LLVM_ABI void erase(uint32_t N, Instruction *I, const BasicBlock *BB);
+ 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();
+ }
+};
+
/// The core GVN pass object.
///
/// FIXME: We should have a good summary of the GVN algorithm implemented by
@@ -197,11 +275,13 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
uint32_t lookupOrAddCall(CallInst *C);
uint32_t computeLoadStoreVN(Instruction *I);
uint32_t phiTranslateImpl(const BasicBlock *BB, const BasicBlock *PhiBlock,
- uint32_t Num, GVNPass &GVN);
+ uint32_t Num, GVNLeaderMap &LeaderTable);
bool areCallValsEqual(uint32_t Num, uint32_t NewNum, const BasicBlock *Pred,
- const BasicBlock *PhiBlock, GVNPass &GVN);
+ const BasicBlock *PhiBlock,
+ GVNLeaderMap &LeaderTable);
std::pair<uint32_t, bool> assignExpNewValueNum(Expression &Exp);
- bool areAllValsInBB(uint32_t Num, const BasicBlock *BB, GVNPass &GVN);
+ bool areAllValsInBB(uint32_t Num, const BasicBlock *BB,
+ GVNLeaderMap &LeaderTable);
void addMemoryStateToExp(Instruction *I, Expression &Exp);
public:
@@ -219,7 +299,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
LLVM_ABI uint32_t lookupPtrToInt(Value *Ptr, Type *Ty);
LLVM_ABI uint32_t phiTranslate(const BasicBlock *BB,
const BasicBlock *PhiBlock, uint32_t Num,
- GVNPass &GVN);
+ GVNLeaderMap &LeaderTable);
LLVM_ABI void eraseTranslateCacheEntry(uint32_t Num,
const BasicBlock &CurrBlock);
LLVM_ABI bool exists(Value *V) const;
@@ -258,85 +338,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
ValueTable VN;
- /// A mapping from value numbers to lists of Value*'s that
- /// have that value number. Use findLeader to query it.
- class LeaderMap {
- public:
- struct LeaderTableEntry {
- // 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;
-
- public:
- class leader_iterator {
- const LeaderListNode *Current;
-
- public:
- using iterator_category = std::forward_iterator_tag;
- using value_type = const LeaderTableEntry;
- using difference_type = std::ptrdiff_t;
- using pointer = value_type *;
- using reference = value_type &;
-
- leader_iterator(const LeaderListNode *C) : Current(C) {}
- leader_iterator &operator++() {
- assert(Current && "Dereferenced end of leader list!");
- Current = Current->Next;
- return *this;
- }
- bool operator==(const leader_iterator &Other) const {
- return Current == Other.Current;
- }
- bool operator!=(const leader_iterator &Other) const {
- return Current != Other.Current;
- }
- reference operator*() const { return Current->Entry; }
- };
-
- iterator_range<leader_iterator> getLeaders(uint32_t N) {
- auto I = NumToLeaders.find(N);
- if (I == NumToLeaders.end()) {
- return iterator_range(leader_iterator(nullptr),
- leader_iterator(nullptr));
- }
-
- return iterator_range(leader_iterator(&I->second),
- leader_iterator(nullptr));
- }
-
- LLVM_ABI void insert(uint32_t N, Value *V, const BasicBlock *BB);
- LLVM_ABI void erase(uint32_t N, Instruction *I, const BasicBlock *BB);
- 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();
- }
- };
- LeaderMap LeaderTable;
+ GVNLeaderMap LeaderTable;
// Map the block to reversed postorder traversal number. It is used to
// find back edge easily.
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index a9392998ede759..bf7115106fa6a3 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -804,7 +804,7 @@ 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) {
+void GVNLeaderMap::insert(uint32_t N, Value *V, const BasicBlock *BB) {
const auto &[It, Inserted] = NumToLeaders.try_emplace(N, V, BB, nullptr);
if (!Inserted) {
// Key already exists: insert new node after the head.
@@ -816,8 +816,7 @@ void GVNPass::LeaderMap::insert(uint32_t N, Value *V, const BasicBlock *BB) {
/// 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) {
+void GVNLeaderMap::erase(uint32_t N, Instruction *I, const BasicBlock *BB) {
auto It = NumToLeaders.find(N);
if (It == NumToLeaders.end())
return;
@@ -2921,20 +2920,21 @@ GVNPass::ValueTable::assignExpNewValueNum(Expression &Exp) {
/// Return whether all the values related with the same \p num are
/// defined in \p BB.
bool GVNPass::ValueTable::areAllValsInBB(uint32_t Num, const BasicBlock *BB,
- GVNPass &GVN) {
+ GVNLeaderMap &LeaderTable) {
return all_of(
- GVN.LeaderTable.getLeaders(Num),
- [=](const LeaderMap::LeaderTableEntry &L) { return L.BB == BB; });
+ LeaderTable.getLeaders(Num),
+ [=](const GVNLeaderMap::LeaderTableEntry &L) { return L.BB == BB; });
}
/// Wrap phiTranslateImpl to provide caching functionality.
uint32_t GVNPass::ValueTable::phiTranslate(const BasicBlock *Pred,
const BasicBlock *PhiBlock,
- uint32_t Num, GVNPass &GVN) {
+ uint32_t Num,
+ GVNLeaderMap &LeaderTable) {
auto FindRes = PhiTranslateTable.find({Num, Pred});
if (FindRes != PhiTranslateTable.end())
return FindRes->second;
- uint32_t NewNum = phiTranslateImpl(Pred, PhiBlock, Num, GVN);
+ uint32_t NewNum = phiTranslateImpl(Pred, PhiBlock, Num, LeaderTable);
PhiTranslateTable.insert({{Num, Pred}, NewNum});
return NewNum;
}
@@ -2944,9 +2944,9 @@ uint32_t GVNPass::ValueTable::phiTranslate(const BasicBlock *Pred,
bool GVNPass::ValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
const BasicBlock *Pred,
const BasicBlock *PhiBlock,
- GVNPass &GVN) {
+ GVNLeaderMap &LeaderTable) {
CallInst *Call = nullptr;
- auto Leaders = GVN.LeaderTable.getLeaders(Num);
+ auto Leaders = LeaderTable.getLeaders(Num);
for (const auto &Entry : Leaders) {
Call = dyn_cast<CallInst>(&*Entry.Val);
if (Call && Call->getParent() == PhiBlock)
@@ -2978,7 +2978,8 @@ bool GVNPass::ValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
/// the phis in BB.
uint32_t GVNPass::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
const BasicBlock *PhiBlock,
- uint32_t Num, GVNPass &GVN) {
+ uint32_t Num,
+ GVNLeaderMap &LeaderTable) {
// See if we can refine the value number by looking at the PN incoming value
// for the given predecessor.
if (PHINode *PN = NumberingPhi[Num]) {
@@ -3018,7 +3019,7 @@ uint32_t GVNPass::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
// If there is any value related with Num is defined in a BB other than
// PhiBlock, it cannot depend on a phi in PhiBlock without going through
// a backedge. We can do an early exit in that case to save compile time.
- if (!areAllValsInBB(Num, PhiBlock, GVN))
+ if (!areAllValsInBB(Num, PhiBlock, LeaderTable))
return Num;
if (Num >= ExprIdx.size() || ExprIdx[Num] == 0)
@@ -3033,7 +3034,7 @@ uint32_t GVNPass::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
(I > 0 && Exp.Opcode == Instruction::ExtractValue) ||
(I > 1 && Exp.Opcode == Instruction::ShuffleVector))
continue;
- Exp.VarArgs[I] = phiTranslate(Pred, PhiBlock, Exp.VarArgs[I], GVN);
+ Exp.VarArgs[I] = phiTranslate(Pred, PhiBlock, Exp.VarArgs[I], LeaderTable);
}
if (Exp.Commutative) {
@@ -3050,7 +3051,8 @@ uint32_t GVNPass::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
if (uint32_t NewNum = ExpressionNumbering[Exp]) {
if (Exp.Opcode == Instruction::Call && NewNum != Num)
- return areCallValsEqual(Num, NewNum, Pred, PhiBlock, GVN) ? NewNum : Num;
+ return areCallValsEqual(Num, NewNum, Pred, PhiBlock, LeaderTable) ? NewNum
+ : Num;
return NewNum;
}
return Num;
@@ -3604,8 +3606,7 @@ bool GVNPass::performScalarPREInsertion(Instruction *Instr, BasicBlock *Pred,
Success = false;
break;
}
- uint32_t TValNo =
- VN.phiTranslate(Pred, Curr, VN.lookup(Op), *this);
+ uint32_t TValNo = VN.phiTranslate(Pred, Curr, VN.lookup(Op), LeaderTable);
if (Value *V = findLeader(Pred, TValNo)) {
Instr->setOperand(I, V);
} else {
@@ -3697,7 +3698,7 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) {
break;
}
- uint32_t TValNo = VN.phiTranslate(P, CurrentBlock, ValNo, *this);
+ uint32_t TValNo = VN.phiTranslate(P, CurrentBlock, ValNo, LeaderTable);
Value *PredV = findLeader(P, TValNo);
if (!PredV) {
PredMap.push_back(std::make_pair(static_cast<Value *>(nullptr), P));
>From a806ba79b850d28f9aa017e47f97d812bbd918b1 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Fri, 25 Sep 2026 11:14:39 +0100
Subject: [PATCH 2/3] [fixup] Fix a comment
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 237bb743c6faf8..553b7fc245250b 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -117,7 +117,7 @@ struct GVNOptions {
};
/// A mapping from value numbers to lists of Value*'s that
-/// have that value number. Use findLeader to query it.
+/// have that value number. Use getLeaders to query it.
class GVNLeaderMap {
public:
struct LeaderTableEntry {
>From 35a519dafe12e4822cd68b788384c2cd81af015a Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Fri, 25 Sep 2026 14:30:29 +0100
Subject: [PATCH 3/3] [fixup] Empty commit in an attempt to unstuck CI
More information about the llvm-commits
mailing list