diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp index 5dd34c77fac92..c193477f3d356 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(this)) + SV->markMaterialized(); } void VPValue::replaceUsesWithIf( VPValue *New, llvm::function_ref 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 d007d8fb36721..d624c2df4e2f7 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -4734,14 +4734,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 llvm::calculateRegisterUsageForPlan( // the loop (not including non-recipe values such as arguments and // constants). SmallSetVector 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 419c2f464a8ef..b77a70bbe9b8f 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().isMaterialized()) { + assert(Plan.getVFxUF().isMaterialized() && + "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..56e512849cb08 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_range; typedef iterator_range 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,20 @@ struct VPSymbolicValue : public VPValue { static bool classof(const VPValue *V) { return V->getVPValueID() == VPVSymbolicSC; } + + /// Returns true if this symbolic value has been materialized. + bool isMaterialized() const { return Materialized; } + + /// Mark this symbolic value as materialized. + void markMaterialized() { + assert(!Materialized && "VPSymbolicValue already materialized"); + Materialized = true; + } + +private: + /// Track whether this symbolic value has been materialized (replaced). + /// After materialization, accessing users should trigger an assertion. + bool Materialized = false; }; /// A VPValue defined by a recipe that produces one or more values. @@ -427,6 +466,12 @@ class VPDef { unsigned getNumDefinedValues() const { return DefinedValues.size(); } }; +inline void VPValue::assertNotMaterialized() const { + assert((!isa(this) || + !cast(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..c28631a5a3ddb 100644 --- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp @@ -1745,5 +1745,42 @@ TEST_F(VPRecipeTest, CastToVPSingleDefRecipe) { // TODO: check other VPSingleDefRecipes. } +TEST_F(VPInstructionTest, VPSymbolicValueMaterialization) { + 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. + VF->replaceAllUsesWith(Plan.getConstantInt(64, 1)); + + // Now VF should be materialized. + EXPECT_TRUE(Plan.getVF().isMaterialized()); +} + +#if defined(GTEST_HAS_DEATH_TEST) && !defined(NDEBUG) +TEST_F(VPInstructionTest, VPSymbolicValueAddUserAfterMaterialization) { + VPlan &Plan = getPlan(); + + // Materialize VF by replacing all uses. + VPValue *VF = &Plan.getVF(); + VF->replaceAllUsesWith(Plan.getConstantInt(64, 1)); + 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 + } // namespace } // namespace llvm