[VPlan] Remove verifyLate from VPlanVerifier. NFC#182799
Merged
Conversation
In llvm#182254 we want to start aborting compilation when the verifier fails between passes, but currently we run into various EVL related failures. The EVL is used in quite a few more places than when the verification was originally added, all of which need to be handled by the verifier. I think this is also exacerbated by the fact that many recipes nowadays are converted to concrete recipes later in the pipeline which duplicates the number of patterns we need to match. The EVL transform itself has also changed much since its original implementation, i.e. non-trapping recipes don't use EVL (llvm#127180) and VP recipes are generated via pattern matching instead of unconditionally (llvm#155394), so I'm not sure if the verification is as relevant today. Rather than try to add more patterns this PR removes the verification to reduce the maintainence cost. Split off from llvm#182254
We can instead just check if the VPlan has been unrolled. Stacked on llvm#182798
Member
|
@llvm/pr-subscribers-vectorizers Author: Luke Lau (lukel97) ChangesWe can instead just check if the VPlan has been unrolled. Stacked on #182798 Full diff: https://github.com/llvm/llvm-project/pull/182799.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index b28c3d949c96a..2342c8bfa502e 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7508,8 +7508,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
State.CFG.PrevBB->getSingleSuccessor(), &BestVPlan);
VPlanTransforms::removeDeadRecipes(BestVPlan);
- assert(verifyVPlanIsValid(BestVPlan, true /*VerifyLate*/) &&
- "final VPlan is invalid");
+ assert(verifyVPlanIsValid(BestVPlan) && "final VPlan is invalid");
// After vectorization, the exit blocks of the original loop will have
// additional predecessors. Invalidate SCEVs for the exit phis in case SE
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index e4b334c3eba49..35518c069bb3c 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -31,7 +31,6 @@ namespace {
class VPlanVerifier {
const VPDominatorTree &VPDT;
VPTypeAnalysis &TypeInfo;
- bool VerifyLate;
SmallPtrSet<BasicBlock *, 8> WrappedIRBBs;
@@ -40,11 +39,6 @@ class VPlanVerifier {
// VPHeaderPHIRecipes.
bool verifyPhiRecipes(const VPBasicBlock *VPBB);
- /// Verify that \p EVL is used correctly. The user must be either in
- /// EVL-based recipes as a last operand or VPInstruction::Add which is
- /// incoming value into EVL's recipe.
- bool verifyEVLRecipe(const VPInstruction &EVL) const;
-
/// Verify that \p LastActiveLane's operand is guaranteed to be a prefix-mask.
bool verifyLastActiveLaneRecipe(const VPInstruction &LastActiveLane) const;
@@ -67,9 +61,8 @@ class VPlanVerifier {
bool verifyRegionRec(const VPRegionBlock *Region);
public:
- VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo,
- bool VerifyLate)
- : VPDT(VPDT), TypeInfo(TypeInfo), VerifyLate(VerifyLate) {}
+ VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo)
+ : VPDT(VPDT), TypeInfo(TypeInfo) {}
bool verify(const VPlan &Plan);
};
@@ -124,7 +117,7 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
RecipeI++;
}
- if (!VerifyLate && NumActiveLaneMaskPhiRecipes > 1) {
+ if (!VPBB->getPlan()->isUnrolled() && NumActiveLaneMaskPhiRecipes > 1) {
errs() << "There should be no more than one VPActiveLaneMaskPHIRecipe";
return false;
}
@@ -146,108 +139,6 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
return true;
}
-bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
- if (EVL.getOpcode() != VPInstruction::ExplicitVectorLength) {
- errs() << "verifyEVLRecipe should only be called on "
- "VPInstruction::ExplicitVectorLength\n";
- return false;
- }
- auto VerifyEVLUse = [&](const VPRecipeBase &R,
- const unsigned ExpectedIdx) -> bool {
- SmallVector<const VPValue *> Ops(R.operands());
- unsigned UseCount = count(Ops, &EVL);
- if (UseCount != 1 || Ops[ExpectedIdx] != &EVL) {
- errs() << "EVL is used as non-last operand in EVL-based recipe\n";
- return false;
- }
- return true;
- };
- auto VerifyEVLUseInVecEndPtr = [&EVL](auto &VEPRs) {
- if (all_of(VEPRs, [&EVL](VPUser *U) {
- auto *VEPR = cast<VPVectorEndPointerRecipe>(U);
- return match(VEPR->getOffset(),
- m_c_Mul(m_SpecificSInt(VEPR->getStride()),
- m_Sub(m_Specific(&EVL), m_One())));
- }))
- return true;
- errs() << "Expected VectorEndPointer with EVL operand\n";
- return false;
- };
- return all_of(EVL.users(), [&](VPUser *U) {
- return TypeSwitch<const VPUser *, bool>(U)
- .Case([&](const VPWidenIntrinsicRecipe *S) {
- return VerifyEVLUse(*S, S->getNumOperands() - 1);
- })
- .Case<VPWidenStoreEVLRecipe, VPReductionEVLRecipe,
- VPWidenIntOrFpInductionRecipe, VPWidenPointerInductionRecipe>(
- [&](const VPRecipeBase *S) { return VerifyEVLUse(*S, 2); })
- .Case([&](const VPScalarIVStepsRecipe *R) {
- if (R->getNumOperands() != 3) {
- errs() << "Unrolling with EVL tail folding not yet supported\n";
- return false;
- }
- return VerifyEVLUse(*R, 2);
- })
- .Case<VPWidenLoadEVLRecipe, VPVectorEndPointerRecipe,
- VPInterleaveEVLRecipe>(
- [&](const VPRecipeBase *R) { return VerifyEVLUse(*R, 1); })
- .Case(
- [&](const VPInstructionWithType *S) { return VerifyEVLUse(*S, 0); })
- .Case([&](const VPInstruction *I) {
- if (I->getOpcode() == Instruction::PHI ||
- I->getOpcode() == Instruction::ICmp)
- return VerifyEVLUse(*I, 1);
- if (I->getOpcode() == Instruction::Sub) {
- // If Sub has a single user that's a SingleDefRecipe (which is
- // expected to be a Mul), filter its users, in turn, to get
- // VectorEndPointerRecipes, and verify that all the offsets match
- // (EVL - 1) * Stride.
- if (auto *Def = dyn_cast_if_present<VPSingleDefRecipe>(
- I->getSingleUser())) {
- auto VEPRs = make_filter_range(Def->users(),
- IsaPred<VPVectorEndPointerRecipe>);
- if (!VEPRs.empty())
- return VerifyEVLUseInVecEndPtr(VEPRs);
- }
- return VerifyEVLUse(*I, 1);
- }
- switch (I->getOpcode()) {
- case Instruction::Add:
- break;
- case Instruction::UIToFP:
- case Instruction::Trunc:
- case Instruction::ZExt:
- case Instruction::Mul:
- case Instruction::Shl:
- case Instruction::FMul:
- case VPInstruction::Broadcast:
- case VPInstruction::PtrAdd:
- // Opcodes above can only use EVL after wide inductions have been
- // expanded.
- if (!VerifyLate) {
- errs() << "EVL used by unexpected VPInstruction\n";
- return false;
- }
- break;
- default:
- errs() << "EVL used by unexpected VPInstruction\n";
- return false;
- }
- if (!VerifyLate &&
- !isa<VPCurrentIterationPHIRecipe>(*I->users().begin())) {
- errs() << "Result of VPInstruction::Add with EVL operand is "
- "not used by VPCurrentIterationPHIRecipe\n";
- return false;
- }
- return true;
- })
- .Default([&](const VPUser *U) {
- errs() << "EVL has unexpected user\n";
- return false;
- });
- });
-}
-
bool VPlanVerifier::verifyLastActiveLaneRecipe(
const VPInstruction &LastActiveLane) const {
assert(LastActiveLane.getOpcode() == VPInstruction::LastActiveLane &&
@@ -372,12 +263,6 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
}
if (const auto *VPI = dyn_cast<VPInstruction>(&R)) {
switch (VPI->getOpcode()) {
- case VPInstruction::ExplicitVectorLength:
- if (!verifyEVLRecipe(*VPI)) {
- errs() << "EVL VPValue is not used correctly\n";
- return false;
- }
- break;
case VPInstruction::LastActiveLane:
if (!verifyLastActiveLaneRecipe(*VPI))
return false;
@@ -569,9 +454,9 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
return true;
}
-bool llvm::verifyVPlanIsValid(const VPlan &Plan, bool VerifyLate) {
+bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
VPDominatorTree VPDT(const_cast<VPlan &>(Plan));
VPTypeAnalysis TypeInfo(Plan);
- VPlanVerifier Verifier(VPDT, TypeInfo, VerifyLate);
+ VPlanVerifier Verifier(VPDT, TypeInfo);
return Verifier.verify(Plan);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.h b/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
index ccf79e8e5c985..ba17312d03799 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
@@ -29,16 +29,13 @@
namespace llvm {
class VPlan;
-/// Verify invariants for general VPlans. If \p VerifyLate is passed, skip some
-/// checks that are not applicable at later stages of the transform pipeline.
-/// Currently it checks the following:
+/// Verify invariants for general VPlans. Currently it checks the following:
/// 1. Region/Block verification: Check the Region/Block verification
/// invariants for every region in the H-CFG.
/// 2. all phi-like recipes must be at the beginning of a block, with no other
/// recipes in between. Note that currently there is still an exception for
/// VPBlendRecipes.
-LLVM_ABI_FOR_TEST bool verifyVPlanIsValid(const VPlan &Plan,
- bool VerifyLate = false);
+LLVM_ABI_FOR_TEST bool verifyVPlanIsValid(const VPlan &Plan);
} // namespace llvm
|
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Luke Lau (lukel97) ChangesWe can instead just check if the VPlan has been unrolled. Stacked on #182798 Full diff: https://github.com/llvm/llvm-project/pull/182799.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index b28c3d949c96a..2342c8bfa502e 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7508,8 +7508,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
State.CFG.PrevBB->getSingleSuccessor(), &BestVPlan);
VPlanTransforms::removeDeadRecipes(BestVPlan);
- assert(verifyVPlanIsValid(BestVPlan, true /*VerifyLate*/) &&
- "final VPlan is invalid");
+ assert(verifyVPlanIsValid(BestVPlan) && "final VPlan is invalid");
// After vectorization, the exit blocks of the original loop will have
// additional predecessors. Invalidate SCEVs for the exit phis in case SE
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index e4b334c3eba49..35518c069bb3c 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -31,7 +31,6 @@ namespace {
class VPlanVerifier {
const VPDominatorTree &VPDT;
VPTypeAnalysis &TypeInfo;
- bool VerifyLate;
SmallPtrSet<BasicBlock *, 8> WrappedIRBBs;
@@ -40,11 +39,6 @@ class VPlanVerifier {
// VPHeaderPHIRecipes.
bool verifyPhiRecipes(const VPBasicBlock *VPBB);
- /// Verify that \p EVL is used correctly. The user must be either in
- /// EVL-based recipes as a last operand or VPInstruction::Add which is
- /// incoming value into EVL's recipe.
- bool verifyEVLRecipe(const VPInstruction &EVL) const;
-
/// Verify that \p LastActiveLane's operand is guaranteed to be a prefix-mask.
bool verifyLastActiveLaneRecipe(const VPInstruction &LastActiveLane) const;
@@ -67,9 +61,8 @@ class VPlanVerifier {
bool verifyRegionRec(const VPRegionBlock *Region);
public:
- VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo,
- bool VerifyLate)
- : VPDT(VPDT), TypeInfo(TypeInfo), VerifyLate(VerifyLate) {}
+ VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo)
+ : VPDT(VPDT), TypeInfo(TypeInfo) {}
bool verify(const VPlan &Plan);
};
@@ -124,7 +117,7 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
RecipeI++;
}
- if (!VerifyLate && NumActiveLaneMaskPhiRecipes > 1) {
+ if (!VPBB->getPlan()->isUnrolled() && NumActiveLaneMaskPhiRecipes > 1) {
errs() << "There should be no more than one VPActiveLaneMaskPHIRecipe";
return false;
}
@@ -146,108 +139,6 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
return true;
}
-bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
- if (EVL.getOpcode() != VPInstruction::ExplicitVectorLength) {
- errs() << "verifyEVLRecipe should only be called on "
- "VPInstruction::ExplicitVectorLength\n";
- return false;
- }
- auto VerifyEVLUse = [&](const VPRecipeBase &R,
- const unsigned ExpectedIdx) -> bool {
- SmallVector<const VPValue *> Ops(R.operands());
- unsigned UseCount = count(Ops, &EVL);
- if (UseCount != 1 || Ops[ExpectedIdx] != &EVL) {
- errs() << "EVL is used as non-last operand in EVL-based recipe\n";
- return false;
- }
- return true;
- };
- auto VerifyEVLUseInVecEndPtr = [&EVL](auto &VEPRs) {
- if (all_of(VEPRs, [&EVL](VPUser *U) {
- auto *VEPR = cast<VPVectorEndPointerRecipe>(U);
- return match(VEPR->getOffset(),
- m_c_Mul(m_SpecificSInt(VEPR->getStride()),
- m_Sub(m_Specific(&EVL), m_One())));
- }))
- return true;
- errs() << "Expected VectorEndPointer with EVL operand\n";
- return false;
- };
- return all_of(EVL.users(), [&](VPUser *U) {
- return TypeSwitch<const VPUser *, bool>(U)
- .Case([&](const VPWidenIntrinsicRecipe *S) {
- return VerifyEVLUse(*S, S->getNumOperands() - 1);
- })
- .Case<VPWidenStoreEVLRecipe, VPReductionEVLRecipe,
- VPWidenIntOrFpInductionRecipe, VPWidenPointerInductionRecipe>(
- [&](const VPRecipeBase *S) { return VerifyEVLUse(*S, 2); })
- .Case([&](const VPScalarIVStepsRecipe *R) {
- if (R->getNumOperands() != 3) {
- errs() << "Unrolling with EVL tail folding not yet supported\n";
- return false;
- }
- return VerifyEVLUse(*R, 2);
- })
- .Case<VPWidenLoadEVLRecipe, VPVectorEndPointerRecipe,
- VPInterleaveEVLRecipe>(
- [&](const VPRecipeBase *R) { return VerifyEVLUse(*R, 1); })
- .Case(
- [&](const VPInstructionWithType *S) { return VerifyEVLUse(*S, 0); })
- .Case([&](const VPInstruction *I) {
- if (I->getOpcode() == Instruction::PHI ||
- I->getOpcode() == Instruction::ICmp)
- return VerifyEVLUse(*I, 1);
- if (I->getOpcode() == Instruction::Sub) {
- // If Sub has a single user that's a SingleDefRecipe (which is
- // expected to be a Mul), filter its users, in turn, to get
- // VectorEndPointerRecipes, and verify that all the offsets match
- // (EVL - 1) * Stride.
- if (auto *Def = dyn_cast_if_present<VPSingleDefRecipe>(
- I->getSingleUser())) {
- auto VEPRs = make_filter_range(Def->users(),
- IsaPred<VPVectorEndPointerRecipe>);
- if (!VEPRs.empty())
- return VerifyEVLUseInVecEndPtr(VEPRs);
- }
- return VerifyEVLUse(*I, 1);
- }
- switch (I->getOpcode()) {
- case Instruction::Add:
- break;
- case Instruction::UIToFP:
- case Instruction::Trunc:
- case Instruction::ZExt:
- case Instruction::Mul:
- case Instruction::Shl:
- case Instruction::FMul:
- case VPInstruction::Broadcast:
- case VPInstruction::PtrAdd:
- // Opcodes above can only use EVL after wide inductions have been
- // expanded.
- if (!VerifyLate) {
- errs() << "EVL used by unexpected VPInstruction\n";
- return false;
- }
- break;
- default:
- errs() << "EVL used by unexpected VPInstruction\n";
- return false;
- }
- if (!VerifyLate &&
- !isa<VPCurrentIterationPHIRecipe>(*I->users().begin())) {
- errs() << "Result of VPInstruction::Add with EVL operand is "
- "not used by VPCurrentIterationPHIRecipe\n";
- return false;
- }
- return true;
- })
- .Default([&](const VPUser *U) {
- errs() << "EVL has unexpected user\n";
- return false;
- });
- });
-}
-
bool VPlanVerifier::verifyLastActiveLaneRecipe(
const VPInstruction &LastActiveLane) const {
assert(LastActiveLane.getOpcode() == VPInstruction::LastActiveLane &&
@@ -372,12 +263,6 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
}
if (const auto *VPI = dyn_cast<VPInstruction>(&R)) {
switch (VPI->getOpcode()) {
- case VPInstruction::ExplicitVectorLength:
- if (!verifyEVLRecipe(*VPI)) {
- errs() << "EVL VPValue is not used correctly\n";
- return false;
- }
- break;
case VPInstruction::LastActiveLane:
if (!verifyLastActiveLaneRecipe(*VPI))
return false;
@@ -569,9 +454,9 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
return true;
}
-bool llvm::verifyVPlanIsValid(const VPlan &Plan, bool VerifyLate) {
+bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
VPDominatorTree VPDT(const_cast<VPlan &>(Plan));
VPTypeAnalysis TypeInfo(Plan);
- VPlanVerifier Verifier(VPDT, TypeInfo, VerifyLate);
+ VPlanVerifier Verifier(VPDT, TypeInfo);
return Verifier.verify(Plan);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.h b/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
index ccf79e8e5c985..ba17312d03799 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.h
@@ -29,16 +29,13 @@
namespace llvm {
class VPlan;
-/// Verify invariants for general VPlans. If \p VerifyLate is passed, skip some
-/// checks that are not applicable at later stages of the transform pipeline.
-/// Currently it checks the following:
+/// Verify invariants for general VPlans. Currently it checks the following:
/// 1. Region/Block verification: Check the Region/Block verification
/// invariants for every region in the H-CFG.
/// 2. all phi-like recipes must be at the beginning of a block, with no other
/// recipes in between. Note that currently there is still an exception for
/// VPBlendRecipes.
-LLVM_ABI_FOR_TEST bool verifyVPlanIsValid(const VPlan &Plan,
- bool VerifyLate = false);
+LLVM_ABI_FOR_TEST bool verifyVPlanIsValid(const VPlan &Plan);
} // namespace llvm
|
Contributor
Author
|
Unstacked now that #182798 is landed |
HendrikHuebner
pushed a commit
to HendrikHuebner/llvm-project
that referenced
this pull request
Mar 10, 2026
We can instead just check if the VPlan has been unrolled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We can instead just check if the VPlan has been unrolled.