[llvm-branch-commits] [llvm] [GVN] Decouple GVNValueTable and GVNPass (PR #211540)
Momchil Velikov via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jul 23 05:54:55 PDT 2026
https://github.com/momchil-velikov created https://github.com/llvm/llvm-project/pull/211540
`GVNValueTable` has several member functions taking a `GVNPass` reference as an argument. This prevents moving `GVNPass` out of `GVN.h` and into `GVN.cpp.`
The `GVNPass` reference is only used to access the `LeaderMap`. This patch moves the `LeaderMap` type into its own header file, `GVNLeaderMap.h`, under class name `GVNLeaderMap` and changes the `GVNValueTable` member functions to take a `GVNLeaderMap` reference instead of a `GVNPass` reference.
>From 88462d9c50637a08a0473bd57ce0b52d7f675d6f Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Wed, 22 Jul 2026 16:51:16 +0100
Subject: [PATCH] [GVN] Decouple GVNValueTable and GVNPass
`GVNValueTable` has several member functions taking a `GVNPass`
reference as an argument. This prevents moving `GVNPass`
out of `GVN.h` and into `GVN.cpp.`
The `GVNPass` reference is only used to access the `LeaderMap`. This patch moves
the `LeaderMap` type into its own header file, `GVNLeaderMap.h`, under class
name `GVNLeaderMap` and changes the `GVNValueTable` member functions to take a
`GVNLeaderMap` reference instead of a `GVNPass` reference.
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 82 +-------------
.../llvm/Transforms/Scalar/GVNLeaderMap.h | 102 ++++++++++++++++++
.../llvm/Transforms/Scalar/GVNValueTable.h | 13 ++-
llvm/lib/Transforms/Scalar/GVN.cpp | 38 +++----
4 files changed, 129 insertions(+), 106 deletions(-)
create mode 100644 llvm/include/llvm/Transforms/Scalar/GVNLeaderMap.h
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 4d8a2d3eda820..f5816313bf214 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -26,6 +26,7 @@
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Transforms/Scalar/GVNLeaderMap.h"
#include "llvm/Transforms/Scalar/GVNValueTable.h"
#include <cstdint>
@@ -145,86 +146,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
AAResults *AA = nullptr;
MemorySSAUpdater *MSSAU = nullptr;
GVNValueTable 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/include/llvm/Transforms/Scalar/GVNLeaderMap.h b/llvm/include/llvm/Transforms/Scalar/GVNLeaderMap.h
new file mode 100644
index 0000000000000..5b04346ece6e6
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Scalar/GVNLeaderMap.h
@@ -0,0 +1,102 @@
+//===- GVNLeaderMap.h - Leader map for GVN --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file / This file provides a data structure for mapping value numbers to
+/// lists of values that have that value number.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_SCALAR_GVNLEADERMAP_H
+#define LLVM_TRANSFORMS_SCALAR_GVNLEADERMAP_H
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/Allocator.h"
+
+namespace llvm {
+
+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();
+ }
+};
+} // namespace llvm
+
+#endif // LLVM_TRANSFORMS_SCALAR_GVNLEADERMAP_H
\ No newline at end of file
diff --git a/llvm/include/llvm/Transforms/Scalar/GVNValueTable.h b/llvm/include/llvm/Transforms/Scalar/GVNValueTable.h
index 5d93461c1260c..9ee4b6b125643 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVNValueTable.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVNValueTable.h
@@ -26,7 +26,6 @@
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Compiler.h"
-#include "llvm/Transforms/Scalar/GVNValueTable.h"
#include <cstdint>
#include <optional>
@@ -47,8 +46,7 @@ class EarliestEscapeAnalysis;
class ExtractValueInst;
class Function;
class FunctionPass;
-class GVNLegacyPass;
-class GVNPass;
+class GVNLeaderMap;
class GetElementPtrInst;
class ImplicitControlFlowTracking;
class LoadInst;
@@ -115,11 +113,12 @@ class GVNValueTable {
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:
@@ -138,7 +137,7 @@ class GVNValueTable {
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;
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index d763bab153822..35e21e0b63017 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -674,7 +674,8 @@ uint32_t GVNValueTable::computeLoadStoreVN(Instruction *I) {
/// the phis in BB.
uint32_t GVNValueTable::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]) {
@@ -714,7 +715,7 @@ uint32_t GVNValueTable::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)
@@ -729,7 +730,7 @@ uint32_t GVNValueTable::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) {
@@ -746,7 +747,8 @@ uint32_t GVNValueTable::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;
@@ -756,9 +758,10 @@ uint32_t GVNValueTable::phiTranslateImpl(const BasicBlock *Pred,
// Return false if the result is unknown.
bool GVNValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
const BasicBlock *Pred,
- const BasicBlock *PhiBlock, GVNPass &GVN) {
+ const BasicBlock *PhiBlock,
+ 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)
@@ -804,11 +807,10 @@ std::pair<uint32_t, bool> GVNValueTable::assignExpNewValueNum(Expression &Exp) {
/// Return whether all the values related with the same \p num are
/// defined in \p BB.
bool GVNValueTable::areAllValsInBB(uint32_t Num, const BasicBlock *BB,
- GVNPass &GVN) {
- return all_of(GVN.LeaderTable.getLeaders(Num),
- [=](const GVNPass::LeaderMap::LeaderTableEntry &L) {
- return L.BB == BB;
- });
+ GVNLeaderMap &LeaderTable) {
+ return all_of(
+ LeaderTable.getLeaders(Num),
+ [=](const GVNLeaderMap::LeaderTableEntry &L) { return L.BB == BB; });
}
/// Include the incoming memory state into the hash of the expression for the
@@ -965,11 +967,11 @@ uint32_t GVNValueTable::lookupPtrToInt(Value *Ptr, Type *Ty) {
/// Wrap phiTranslateImpl to provide caching functionality.
uint32_t GVNValueTable::phiTranslate(const BasicBlock *Pred,
const BasicBlock *PhiBlock, uint32_t Num,
- GVNPass &GVN) {
+ 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;
}
@@ -1023,7 +1025,7 @@ void GVNValueTable::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.
@@ -1035,8 +1037,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;
@@ -3559,8 +3560,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 {
@@ -3652,7 +3652,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));
More information about the llvm-branch-commits
mailing list