[llvm] [GVN] Remove unused debug helper (NFC) (PR #210326)
Momchil Velikov via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 17 06:30:06 PDT 2026
https://github.com/momchil-velikov created https://github.com/llvm/llvm-project/pull/210326
None
>From e1784a875479f2bb3fd139c7fff75393a3b91dc5 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Tue, 7 Jul 2026 09:48:01 +0000
Subject: [PATCH 1/3] [GVN] Remove the "private" `llvm::gvn` namespace (NFC)
Move `AvailableValue` and `AvailableValueInBlock` into GVNPass, similar
to other helper types.
Retain `llvm::gvn::GVNLegacyPass` as just `llvm::GVNLegacyPass` -
"legacy" is already a sufficent hint and it is not going to become more
"private" by stacking "gvn" prefixes to the name.
Ideally, `GVNLegacyPass` should be defined in an anonymous namespace, but
that is not possible because it is declared as a friend of GVNPass.
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 19 ++++++-------------
llvm/lib/Transforms/Scalar/GVN.cpp | 10 ++++++----
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 9142defb34de2..8b70d61fd9f3f 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -61,15 +61,6 @@ class PHINode;
class TargetLibraryInfo;
class Value;
class IntrinsicInst;
-/// A private "module" namespace for types and utilities used by GVN. These
-/// are implementation details and should not be used by clients.
-namespace LLVM_LIBRARY_VISIBILITY_NAMESPACE gvn {
-
-struct AvailableValue;
-struct AvailableValueInBlock;
-class GVNLegacyPass;
-
-} // end namespace gvn
/// A set of parameters to control various transforms performed by GVN pass.
// Each of the optional boolean parameters can be set to:
@@ -133,6 +124,8 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
public:
struct Expression;
+ struct AvailableValue;
+ struct AvailableValueInBlock;
GVNPass(GVNOptions Options = {}) : Options(Options) {}
@@ -248,7 +241,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
};
private:
- friend class gvn::GVNLegacyPass;
+ friend class GVNLegacyPass;
friend struct DenseMapInfo<Expression>;
MemoryDependenceResults *MD = nullptr;
@@ -354,7 +347,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
bool InvalidBlockRPONumbers = true;
using LoadDepVect = SmallVector<NonLocalDepResult, 64>;
- using AvailValInBlkVect = SmallVector<gvn::AvailableValueInBlock, 64>;
+ using AvailValInBlkVect = SmallVector<AvailableValueInBlock, 64>;
using UnavailBlkVect = SmallVector<BasicBlock *, 64>;
bool runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
@@ -454,7 +447,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
/// Given a local dependency (Def or Clobber) determine if a value is
/// available for the load.
- std::optional<gvn::AvailableValue>
+ std::optional<AvailableValue>
AnalyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
Value *Address);
@@ -462,7 +455,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
/// \p TrueAddr and \p FalseAddr guarded by \p Cond), determine whether a
/// value is available by finding dominating values for both addresses. If
/// so, the load can be rematerialized as a select of those two values.
- std::optional<gvn::AvailableValue>
+ std::optional<AvailableValue>
AnalyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
Value *FalseAddr, Instruction *From);
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 1b7bcb10be8f8..dc9c1fb0ba719 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -81,10 +81,12 @@
#include <utility>
using namespace llvm;
-using namespace llvm::gvn;
using namespace llvm::VNCoercion;
using namespace PatternMatch;
+using AvailableValue = GVNPass::AvailableValue;
+using AvailableValueInBlock = GVNPass::AvailableValueInBlock;
+
#define DEBUG_TYPE "gvn"
STATISTIC(NumGVNInstr, "Number of instructions deleted");
@@ -193,7 +195,7 @@ template <> struct llvm::DenseMapInfo<GVNPass::Expression> {
/// Materialization of an AvailableValue never fails. An AvailableValue is
/// implicitly associated with a rematerialization point which is the
/// location of the instruction from which it was formed.
-struct llvm::gvn::AvailableValue {
+struct llvm::GVNPass::AvailableValue {
enum class ValType {
SimpleVal, // A simple offsetted value that is accessed.
LoadVal, // A value produced by a load.
@@ -289,7 +291,7 @@ struct llvm::gvn::AvailableValue {
/// Represents an AvailableValue which can be rematerialized at the end of
/// the associated BasicBlock.
-struct llvm::gvn::AvailableValueInBlock {
+struct llvm::GVNPass::AvailableValueInBlock {
/// BB - The basic block in question.
BasicBlock *BB = nullptr;
@@ -3996,7 +3998,7 @@ void GVNPass::assignValNumForDeadCode() {
}
}
-class llvm::gvn::GVNLegacyPass : public FunctionPass {
+class llvm::GVNLegacyPass : public FunctionPass {
public:
static char ID; // Pass identification, replacement for typeid.
>From 2293acff88a03097a610c682769ad316d6b1fc0e Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Tue, 7 Jul 2026 11:06:39 +0000
Subject: [PATCH 2/3] [GVN] Rename some functions to follow LLVM naming
conventions (NFC)
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 8 ++---
llvm/lib/Transforms/Scalar/GVN.cpp | 38 +++++++++++------------
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 8b70d61fd9f3f..ffe07fb2ec7e2 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -448,7 +448,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
/// Given a local dependency (Def or Clobber) determine if a value is
/// available for the load.
std::optional<AvailableValue>
- AnalyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
+ analyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
Value *Address);
/// Given a select-dependency for the load (the load address is a select of
@@ -456,13 +456,13 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
/// value is available by finding dominating values for both addresses. If
/// so, the load can be rematerialized as a select of those two values.
std::optional<AvailableValue>
- AnalyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
+ analyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
Value *FalseAddr, Instruction *From);
/// Given a list of non-local dependencies, determine if a value is
/// available for the load in each specified block. If it is, add it to
/// ValuesPerBlock. If not, add it to UnavailableBlocks.
- void AnalyzeLoadAvailability(LoadInst *Load,
+ void analyzeLoadAvailability(LoadInst *Load,
SmallVectorImpl<ReachingMemVal> &Deps,
AvailValInBlkVect &ValuesPerBlock,
UnavailBlkVect &UnavailableBlocks);
@@ -472,7 +472,7 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
LoadInst *findLoadToHoistIntoPred(BasicBlock *Pred, BasicBlock *LoadBB,
LoadInst *Load);
- bool PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
+ bool performLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
UnavailBlkVect &UnavailableBlocks);
/// Try to replace a load which executes on each loop iteraiton with Phi
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index dc9c1fb0ba719..ee050d5a3861c 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -965,7 +965,7 @@ enum class AvailabilityState : char {
/// 1) we know the block *is* fully available.
/// 2) we do not know whether the block is fully available or not, but we are
/// currently speculating that it will be.
-static bool IsValueFullyAvailableInBlock(
+static bool isValueFullyAvailableInBlock(
BasicBlock *BB,
DenseMap<BasicBlock *, AvailabilityState> &FullyAvailableBlocks) {
SmallVector<BasicBlock *, 32> Worklist;
@@ -1100,7 +1100,7 @@ static void replaceValuesPerBlockEntry(
/// construct SSA form, allowing us to eliminate Load. This returns the value
/// that should be used at Load's definition site.
static Value *
-ConstructSSAForLoadSet(LoadInst *Load,
+constructSSAForLoadSet(LoadInst *Load,
SmallVectorImpl<AvailableValueInBlock> &ValuesPerBlock,
GVNPass &GVN) {
// Check for the fully redundant, dominating load case. In this case, we can
@@ -1330,7 +1330,7 @@ static Value *findDominatingValue(const MemoryLocation &Loc, Type *LoadTy,
}
std::optional<AvailableValue>
-GVNPass::AnalyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
+GVNPass::analyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
Value *FalseAddr, Instruction *From) {
assert(TrueAddr->getType() == Load->getPointerOperandType() &&
"Invalid address type of true side of select dependency");
@@ -1352,7 +1352,7 @@ GVNPass::AnalyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
}
std::optional<AvailableValue>
-GVNPass::AnalyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
+GVNPass::analyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
Value *Address) {
assert(Load->isUnordered() && "rules below are incorrect for ordered access");
assert((Dep.Kind == DepKind::Def || Dep.Kind == DepKind::Clobber) &&
@@ -1479,7 +1479,7 @@ GVNPass::AnalyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
// loads and DepInst that may clobber the loads.
if (auto *Sel = dyn_cast<SelectInst>(DepInst)) {
assert(Sel->getType() == Load->getPointerOperandType());
- if (auto AV = AnalyzeSelectAvailability(Load, Sel->getCondition(),
+ if (auto AV = analyzeSelectAvailability(Load, Sel->getCondition(),
Sel->getTrueValue(),
Sel->getFalseValue(), DepInst))
return AV;
@@ -1494,7 +1494,7 @@ GVNPass::AnalyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
return std::nullopt;
}
-void GVNPass::AnalyzeLoadAvailability(LoadInst *Load,
+void GVNPass::analyzeLoadAvailability(LoadInst *Load,
SmallVectorImpl<ReachingMemVal> &Deps,
AvailValInBlkVect &ValuesPerBlock,
UnavailBlkVect &UnavailableBlocks) {
@@ -1521,7 +1521,7 @@ void GVNPass::AnalyzeLoadAvailability(LoadInst *Load,
// load as a select of the two reaching values (one per side). The values
// are searched for at the end of DepBB.
if (Dep.Kind == DepKind::Select) {
- if (auto AV = AnalyzeSelectAvailability(
+ if (auto AV = analyzeSelectAvailability(
Load, const_cast<Value *>(Dep.SelCond),
const_cast<Value *>(Dep.SelTrueAddr),
const_cast<Value *>(Dep.SelFalseAddr), DepBB->getTerminator())) {
@@ -1537,7 +1537,7 @@ void GVNPass::AnalyzeLoadAvailability(LoadInst *Load,
// the pointer operand of the load if PHI translation occurs. Make sure
// to consider the right address.
if (auto AV =
- AnalyzeLoadAvailability(Load, Dep, const_cast<Value *>(Dep.Addr))) {
+ analyzeLoadAvailability(Load, Dep, const_cast<Value *>(Dep.Addr))) {
// subtlety: because we know this was a non-local dependency, we know
// it's safe to materialize anywhere between the instruction within
// DepInfo and the end of it's block.
@@ -1609,7 +1609,7 @@ LoadInst *GVNPass::findLoadToHoistIntoPred(BasicBlock *Pred, BasicBlock *LoadBB,
// If an identical load doesn't depends on any local instructions, it can
// be safely moved to PredBB.
// Also check for the implicit control flow instructions. See the comments
- // in PerformLoadPRE for details.
+ // in performLoadPRE for details.
if (!HasLocalDep && !ICF->isDominatedByICFIFromSameBlock(&Inst))
return cast<LoadInst>(&Inst);
@@ -1693,8 +1693,8 @@ void GVNPass::eliminatePartiallyRedundantLoad(
}
// Perform PHI construction.
- Value *V = ConstructSSAForLoadSet(Load, ValuesPerBlock, *this);
- // ConstructSSAForLoadSet is responsible for combining metadata.
+ Value *V = constructSSAForLoadSet(Load, ValuesPerBlock, *this);
+ // constructSSAForLoadSet is responsible for combining metadata.
ICF->removeUsersOf(Load);
Load->replaceAllUsesWith(V);
if (isa<PHINode>(V))
@@ -1710,7 +1710,7 @@ void GVNPass::eliminatePartiallyRedundantLoad(
salvageAndRemoveInstruction(Load);
}
-bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
+bool GVNPass::performLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
UnavailBlkVect &UnavailableBlocks) {
// Okay, we have *some* definitions of the value. This means that the value
// is available in some of our (transitive) predecessors. Lets think about
@@ -1793,7 +1793,7 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
return false;
}
- if (IsValueFullyAvailableInBlock(Pred, FullyAvailableBlocks)) {
+ if (isValueFullyAvailableInBlock(Pred, FullyAvailableBlocks)) {
continue;
}
@@ -2122,7 +2122,7 @@ bool GVNPass::processNonLocalLoad(LoadInst *Load,
// Step 1: Analyze the availability of the load.
AvailValInBlkVect ValuesPerBlock;
UnavailBlkVect UnavailableBlocks;
- AnalyzeLoadAvailability(Load, Deps, ValuesPerBlock, UnavailableBlocks);
+ analyzeLoadAvailability(Load, Deps, ValuesPerBlock, UnavailableBlocks);
// If we have no predecessors that produce a known value for this load, exit
// early.
@@ -2138,8 +2138,8 @@ bool GVNPass::processNonLocalLoad(LoadInst *Load,
LLVM_DEBUG(dbgs() << "GVN REMOVING NONLOCAL LOAD: " << *Load << '\n');
// Perform PHI construction.
- Value *V = ConstructSSAForLoadSet(Load, ValuesPerBlock, *this);
- // ConstructSSAForLoadSet is responsible for combining metadata.
+ Value *V = constructSSAForLoadSet(Load, ValuesPerBlock, *this);
+ // constructSSAForLoadSet is responsible for combining metadata.
ICF->removeUsersOf(Load);
Load->replaceAllUsesWith(V);
@@ -2166,7 +2166,7 @@ bool GVNPass::processNonLocalLoad(LoadInst *Load,
return Changed;
if (performLoopLoadPRE(Load, ValuesPerBlock, UnavailableBlocks) ||
- PerformLoadPRE(Load, ValuesPerBlock, UnavailableBlocks))
+ performLoadPRE(Load, ValuesPerBlock, UnavailableBlocks))
return true;
return Changed;
@@ -2595,7 +2595,7 @@ void GVNPass::collectClobberList(SmallVectorImpl<MemoryAccess *> &Clobbers,
/// Entrypoint for the MemorySSA-based redundant load elimination algorithm.
/// Given as input a load instruction, the function computes the set of reaching
-/// memory values, one per predecessor path, that AnalyzeLoadAvailability can
+/// memory values, one per predecessor path, that analyzeLoadAvailability can
/// later use to establish whether the load may be eliminated. A reaching value
/// may be of the following descriptor kind:
/// * Def: a precise instruction that produces the exact bits the load would
@@ -2835,7 +2835,7 @@ bool GVNPass::processLoad(LoadInst *L) {
return false;
}
- auto AV = AnalyzeLoadAvailability(L, MemVal, L->getPointerOperand());
+ auto AV = analyzeLoadAvailability(L, MemVal, L->getPointerOperand());
if (!AV)
return false;
>From 67ad2e492b9f7a5ead697bfd8ec1e21aef2973cf Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Tue, 7 Jul 2026 14:33:47 +0000
Subject: [PATCH 3/3] [GVN] Remove unused debug helper (NFC)
The `GVNPass::dump` method is not used anywhere. Moreover, there's
no `GVNPass` state that corresponds to its parameter type. Even if a
`GVNPass::dump` method could be useful, this one wasn't it.
---
llvm/include/llvm/Transforms/Scalar/GVN.h | 1 -
llvm/lib/Transforms/Scalar/GVN.cpp | 11 -----------
2 files changed, 12 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index ffe07fb2ec7e2..0b9c8dc88fe14 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -491,7 +491,6 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
// Other helper routines.
bool processInstruction(Instruction *I);
bool processBlock(BasicBlock *BB);
- void dump(DenseMap<uint32_t, Value *> &Map) const;
bool iterateOnFunction(Function &F);
bool performPRE(Function &F);
bool performScalarPRE(Instruction *I);
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index ee050d5a3861c..155308f0e7f71 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -934,17 +934,6 @@ void GVNPass::salvageAndRemoveInstruction(Instruction *I) {
removeInstruction(I);
}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-LLVM_DUMP_METHOD void GVNPass::dump(DenseMap<uint32_t, Value *> &Map) const {
- errs() << "{\n";
- for (const auto &[Num, Exp] : Map) {
- errs() << Num << "\n";
- Exp->dump();
- }
- errs() << "}\n";
-}
-#endif
-
enum class AvailabilityState : char {
/// We know the block *is not* fully available. This is a fixpoint.
Unavailable = 0,
More information about the llvm-commits
mailing list