[llvm] [VPlan] Prevent uses of materialized VPSymbolicValues. (NFC) (PR #182318)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 10 07:08:09 PDT 2026
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/182318
>From af9765ab965b9a42a0522de95336b319a438f20f Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 17 Feb 2026 19:11:35 +0000
Subject: [PATCH] [VPlan] Prevent uses of materialized VPSymbolicValues.
After VPSymbolicValues (like VF and VFxUF) are materialized via
replaceAllUsesWith, they should not be accessed again. This patch:
1. Tracks materialization state in VPSymbolicValue.
2. Asserts if the materialized VPValue is used again. Currently it
adds asserts to various member functions, preventing calling them
on materialized symbolic values.
Note that this still allows some uses (e.g. comparing VPSymbolicValue
references or pointers), but this should be relatively harmless given
that it is impossible to (re-)add any users. If we want to further
tighten the checks, we could add asserts to the accessors or override
operator&, but that will require more changes and not add much extra
guards I think.
Depends on https://github.com/llvm/llvm-project/pull/182146 to fix a
current access violation (included in PR).
---
llvm/lib/Transforms/Vectorize/VPlan.cpp | 3 +
llvm/lib/Transforms/Vectorize/VPlan.h | 8 +--
.../Transforms/Vectorize/VPlanAnalysis.cpp | 3 +-
.../Transforms/Vectorize/VPlanTransforms.cpp | 15 +++--
llvm/lib/Transforms/Vectorize/VPlanValue.h | 56 +++++++++++++++++--
.../Transforms/Vectorize/VPlanTest.cpp | 43 ++++++++++++++
6 files changed, 113 insertions(+), 15 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 0ceeb570e8b1f..12869f1cbf7e5 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1407,11 +1407,14 @@ bool VPValue::isDefinedOutsideLoopRegions() const {
}
void VPValue::replaceAllUsesWith(VPValue *New) {
replaceUsesWithIf(New, [](VPUser &, unsigned) { return true; });
+ if (auto *SV = dyn_cast<VPSymbolicValue>(this))
+ SV->markMaterialized();
}
void VPValue::replaceUsesWithIf(
VPValue *New,
llvm::function_ref<bool(VPUser &U, unsigned Idx)> ShouldReplace) {
+ assertNotMaterialized();
// Note that this early exit is required for correctness; the implementation
// below relies on the number of users for this VPValue to decrease, which
// isn't the case if this == New.
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index da2f6f8c7cd03..fe709b51a020e 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -4733,14 +4733,14 @@ class VPlan {
VPSymbolicValue &getVectorTripCount() { return VectorTripCount; }
/// Returns the VF of the vector loop region.
- VPValue &getVF() { return VF; };
- const VPValue &getVF() const { return VF; };
+ VPSymbolicValue &getVF() { return VF; };
+ const VPSymbolicValue &getVF() const { return VF; };
/// Returns the UF of the vector loop region.
- VPValue &getUF() { return UF; };
+ VPSymbolicValue &getUF() { return UF; };
/// Returns VF * UF of the vector loop region.
- VPValue &getVFxUF() { return VFxUF; }
+ VPSymbolicValue &getVFxUF() { return VFxUF; }
LLVMContext &getContext() const {
return getScalarHeader()->getIRBasicBlock()->getContext();
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index 998e48d411f50..55f2a626c293a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -436,7 +436,8 @@ SmallVector<VPRegisterUsage, 8> llvm::calculateRegisterUsageForPlan(
// the loop (not including non-recipe values such as arguments and
// constants).
SmallSetVector<VPValue *, 8> LoopInvariants;
- LoopInvariants.insert(&Plan.getVectorTripCount());
+ if (Plan.getVectorTripCount().getNumUsers() > 0)
+ LoopInvariants.insert(&Plan.getVectorTripCount());
// We scan the loop in a topological order in order and assign a number to
// each recipe. We use RPO to ensure that defs are met before their users. We
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 9e4756e381bd2..f25f48af4165f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -4936,6 +4936,9 @@ void VPlanTransforms::materializeConstantVectorTripCount(
assert(Plan.hasUF(BestUF) && "BestUF is not available in Plan");
VPValue *TC = Plan.getTripCount();
+ if (TC->getNumUsers() == 0)
+ return;
+
// Skip cases for which the trip count may be non-trivial to materialize.
// I.e., when a scalar tail is absent - due to tail folding, or when a scalar
// tail is required.
@@ -5130,14 +5133,18 @@ void VPlanTransforms::materializeVectorTripCount(VPlan &Plan,
void VPlanTransforms::materializeFactors(VPlan &Plan, VPBasicBlock *VectorPH,
ElementCount VFEC) {
+ // If VF and VFxUF have already been materialized (no remaining users),
+ // there's nothing more to do.
+ if (Plan.getVF().getNumUsers() == 0) {
+ assert(Plan.getVFxUF().getNumUsers() == 0 &&
+ "VF and VFxUF must be materialized together");
+ return;
+ }
+
VPBuilder Builder(VectorPH, VectorPH->begin());
Type *TCTy = VPTypeAnalysis(Plan).inferScalarType(Plan.getTripCount());
VPValue &VF = Plan.getVF();
VPValue &VFxUF = Plan.getVFxUF();
- // Note that after the transform, no further uses of Plan.getVF and
- // Plan.getVFxUF should be added.
- // TODO: Add assertions for this.
-
// If there are no users of the runtime VF, compute VFxUF by constant folding
// the multiplication of VF and UF.
if (VF.getNumUsers() == 0) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index 4ef78341e0654..b7d923e302369 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -101,11 +101,24 @@ class LLVM_ABI_FOR_TEST VPValue {
void dump() const;
#endif
- unsigned getNumUsers() const { return Users.size(); }
- void addUser(VPUser &User) { Users.push_back(&User); }
+ /// Assert that this VPValue has not been materialized, if it is a
+ /// VPSymbolicValue.
+ void assertNotMaterialized() const;
+
+ unsigned getNumUsers() const {
+ if (Users.empty())
+ return 0;
+ assertNotMaterialized();
+ return Users.size();
+ }
+ void addUser(VPUser &User) {
+ assertNotMaterialized();
+ Users.push_back(&User);
+ }
/// Remove a single \p User from the list of users.
void removeUser(VPUser &User) {
+ assertNotMaterialized();
// The same user can be added multiple times, e.g. because the same VPValue
// is used twice by the same VPUser. Remove a single one.
auto *I = find(Users, &User);
@@ -118,10 +131,22 @@ class LLVM_ABI_FOR_TEST VPValue {
typedef iterator_range<user_iterator> user_range;
typedef iterator_range<const_user_iterator> const_user_range;
- user_iterator user_begin() { return Users.begin(); }
- const_user_iterator user_begin() const { return Users.begin(); }
- user_iterator user_end() { return Users.end(); }
- const_user_iterator user_end() const { return Users.end(); }
+ user_iterator user_begin() {
+ assertNotMaterialized();
+ return Users.begin();
+ }
+ const_user_iterator user_begin() const {
+ assertNotMaterialized();
+ return Users.begin();
+ }
+ user_iterator user_end() {
+ assertNotMaterialized();
+ return Users.end();
+ }
+ const_user_iterator user_end() const {
+ assertNotMaterialized();
+ return Users.end();
+ }
user_range users() { return user_range(user_begin(), user_end()); }
const_user_range users() const {
return const_user_range(user_begin(), user_end());
@@ -226,6 +251,19 @@ struct VPSymbolicValue : public VPValue {
static bool classof(const VPValue *V) {
return V->getVPValueID() == VPVSymbolicSC;
}
+
+#if !defined(NDEBUG)
+ /// Returns true if this symbolic value has been materialized.
+ bool isMaterialized() const { return Materialized; }
+
+ /// Mark this symbolic value as materialized.
+ void markMaterialized() { Materialized = true; }
+
+private:
+ /// Track whether this symbolic value has been materialized (replaced).
+ /// After materialization, accessing users should trigger an assertion.
+ bool Materialized = false;
+#endif
};
/// A VPValue defined by a recipe that produces one or more values.
@@ -427,6 +465,12 @@ class VPDef {
unsigned getNumDefinedValues() const { return DefinedValues.size(); }
};
+inline void VPValue::assertNotMaterialized() const {
+ assert((!isa<VPSymbolicValue>(this) ||
+ !cast<VPSymbolicValue>(this)->isMaterialized()) &&
+ "accessing materialized symbolic value");
+}
+
} // namespace llvm
#endif // LLVM_TRANSFORMS_VECTORIZE_VPLAN_VALUE_H
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index 254755bb2bf4c..e526114589913 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -1745,5 +1745,48 @@ TEST_F(VPRecipeTest, CastToVPSingleDefRecipe) {
// TODO: check other VPSingleDefRecipes.
}
+TEST_F(VPInstructionTest, VPSymbolicValueMaterialization) {
+ IntegerType *Int64 = IntegerType::get(C, 64);
+ VPlan &Plan = getPlan();
+
+ // Initially, VF is not materialized.
+ EXPECT_FALSE(Plan.getVF().isMaterialized());
+
+ // Create a recipe that uses VF.
+ VPValue *VF = &Plan.getVF();
+ VPInstruction *I = new VPInstruction(VPInstruction::StepVector, {});
+ VPBasicBlock &VPBB = *Plan.createVPBasicBlock("");
+ VPBB.appendRecipe(I);
+ I->addOperand(VF);
+
+ // Replace VF with a constant.
+ VPValue *Const1 = Plan.getOrAddLiveIn(ConstantInt::get(Int64, 1));
+ VF->replaceAllUsesWith(Const1);
+
+ // Now VF should be materialized.
+ EXPECT_TRUE(Plan.getVF().isMaterialized());
+}
+
+#if GTEST_HAS_DEATH_TEST
+#ifndef NDEBUG
+TEST_F(VPInstructionTest, VPSymbolicValueAddUserAfterMaterialization) {
+ IntegerType *Int64 = IntegerType::get(C, 64);
+ VPlan &Plan = getPlan();
+
+ // Materialize VF by replacing all uses.
+ VPValue *VF = &Plan.getVF();
+ VPValue *Const1 = Plan.getOrAddLiveIn(ConstantInt::get(Int64, 1));
+ VF->replaceAllUsesWith(Const1);
+ EXPECT_TRUE(Plan.getVF().isMaterialized());
+
+ // Adding a new user to a materialized value should crash.
+ VPInstruction *I = new VPInstruction(VPInstruction::StepVector, {});
+ VPBasicBlock &VPBB = *Plan.createVPBasicBlock("");
+ VPBB.appendRecipe(I);
+ EXPECT_DEATH(I->addOperand(VF), "accessing materialized symbolic value");
+}
+#endif
+#endif
+
} // namespace
} // namespace llvm
More information about the llvm-commits
mailing list