Skip to content

[VPlan] Assert vplan-verify-each result and fix LastActiveLane verification #182254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lukel97 merged 12 commits into from
Feb 25, 2026
Merged

[VPlan] Assert vplan-verify-each result and fix LastActiveLane verification #182254

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
896ef73
[VPlan] Assert vplan-verify-each result and get verifier passing
lukel97 Feb 19, 2026
9401b48
Use report_fatal_error
lukel97 Feb 19, 2026
e9059ef
Tighten monotonic IV checks to cases where there's no known overflow
lukel97 Feb 19, 2026
ba01b63
Add comment
lukel97 Feb 19, 2026
db8dbb3
Check for nuw on add
lukel97 Feb 20, 2026
04e4ee7
Merge branch 'main' of github.com:llvm/llvm-project into loop-vectori…
lukel97 Feb 23, 2026
08155c7
Fix accidentally dropping recursive m_Add monotonic check, use isCano…
lukel97 Feb 24, 2026
b503231
Remove noUnsignedWrap check, check for EVL explicitly
lukel97 Feb 24, 2026
90160d8
Merge branch 'main' of github.com:llvm/llvm-project into loop-vectori…
lukel97 Feb 25, 2026
0f62428
Update comment, add unit test
lukel97 Feb 25, 2026
bb065bb
Merge branch 'main' of github.com:llvm/llvm-project into loop-vectori…
lukel97 Feb 25, 2026
84eb197
Undo stray whitespace change
lukel97 Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ cl::opt<bool>
cl::init(false),
#endif
cl::Hidden,
cl::desc("Verfiy VPlans after VPlan transforms."));
cl::desc("Verify VPlans after VPlan transforms."));

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
cl::opt<bool> llvm::VPlanPrintAfterAll(
Expand Down Expand Up @@ -7512,8 +7512,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");

@david-arm david-arm Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this isn't also guarded by EnableVerify like in the vplan code? I was expecting something similar here:

  bool VPlanIsValid = verifyVPlanIsValid(BestVPlan);
  if (EnableVerify && !VPlanIsValid)
    report_fatal_error("Broken VPlan found, compilation aborted!");
  else
    assert(VPlanIsValid && "final VPlan is invalid");

@fhahn fhahn Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an assert that verifies that the plan is valid at this specific point, independent of running the verifier after all transforms. This should mirror how verification is handled for LLVM IR verification

@david-arm david-arm Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in a release build does the IR verifier report a fatal error for unverified IR? If so, then if we mirror that behaviour this should also be a fatal error.

@fhahn fhahn Feb 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, verifyFunction for IR similarly just returns a boolean, instead of reporting a fatal error:

llvm-project/llvm/lib/IR/Verifier.cpp

Line 7885 in 58ac25e

bool llvm::verifyFunction(const Function &f, raw_ostream *OS) {

It is used in asserts so those won't raise a fatal error in release builds (e.g. the assert used in LV). If the verifier is enabled to run after each pass, that raises a fatal error.


// After vectorization, the exit blocks of the original loop will have
// additional predecessors. Invalidate SCEVs for the exit phis in case SE
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,8 @@ inline auto m_c_LogicalOr(const Op0_t &Op0, const Op1_t &Op1) {
return m_c_Select(Op0, m_True(), Op1);
}

inline auto m_CanonicalIV() { return class_match<VPCanonicalIVPHIRecipe>(); }

template <typename Op0_t, typename Op1_t, typename Op2_t>
using VPScalarIVSteps_match = Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t>, 0,
false, VPScalarIVStepsRecipe>;
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Transforms/Vectorize/VPlanTransforms.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ struct VPlanTransforms {
dbgs() << Plan << '\n';
}
#endif
if (VerifyEachVPlan && EnableVerify)
verifyVPlanIsValid(Plan);
if (VerifyEachVPlan && EnableVerify) {
if (!verifyVPlanIsValid(Plan))
report_fatal_error("Broken VPlan found, compilation aborted!");
Comment on lines +76 to +77

@fhahn fhahn Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, it may be good to add a C++ unit test that passes an invalid VPlan to a transform invoked via the macro, to add test coverage for the reporting.

@lukel97 lukel97 Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 0f62428

}
}};

return std::forward<PassTy>(Pass)(Plan, std::forward<ArgsTy>(Args)...);
Expand Down
156 changes: 31 additions & 125 deletions llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ namespace {
class VPlanVerifier {
const VPDominatorTree &VPDT;
VPTypeAnalysis &TypeInfo;
bool VerifyLate;

SmallPtrSet<BasicBlock *, 8> WrappedIRBBs;

Expand All @@ -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;
Comment thread
lukel97 marked this conversation as resolved.
Outdated

/// Verify that \p LastActiveLane's operand is guaranteed to be a prefix-mask.
bool verifyLastActiveLaneRecipe(const VPInstruction &LastActiveLane) const;

Expand All @@ -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);
};
Expand Down Expand Up @@ -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;
}
Expand All @@ -146,106 +139,24 @@ 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;
}
static bool isKnownMonotonic(VPValue *V) {
VPValue *X, *Y;
if (match(V, m_Add(m_VPValue(X), m_VPValue(Y))))
return isKnownMonotonic(X) && isKnownMonotonic(Y);
Comment on lines +145 to +146

@fhahn fhahn Feb 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would also need to check that the Add is NUW, if it would wrap it would not be monotonic, even if both operands are

@lukel97 lukel97 Feb 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in db8dbb3

@lukel97 lukel97 Feb 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this actually causes verifier failures in test/Transforms/LoopVectorize/first-order-recurrence-tail-folding.ll, %10 is used as a prefix mask in a lastactivelane but %step.add doesn't have unsigned wrap:

# | VPlan 'Initial VPlan for VF={2},UF={2}' {
# | Live-in vp<%0> = VF
# | Live-in vp<%1> = VF * UF
# | Live-in vp<%2> = vector-trip-count
# | Live-in vp<%3> = backedge-taken count
# | Live-in ir<%n> = original trip-count
# | 
# | ir-bb<entry>:
# |   EMIT branch-on-cond ir<false>
# | Successor(s): scalar.ph, vector.ph
# | 
# | vector.ph:
# |   EMIT vp<%5> = wide-iv-step vp<%0>, ir<1>
# | Successor(s): vector loop
# | 
# | <x1> vector loop: {
# |   vector.body:
# |     EMIT vp<%6> = CANONICAL-INDUCTION ir<0>, vp<%index.next>
# |     ir<%iv> = WIDEN-INDUCTION ir<0>, ir<1>, vp<%0>, vp<%5>, vp<%step.add>
# |     EMIT vp<%step.add> = add ir<%iv>, vp<%5>
# |     EMIT vp<%10> = icmp ule vp<%step.add>, vp<%3>

Will take a look to see if we're missing a nuw on the step.add from unrolling.

@fhahn fhahn Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be good to add a test case; in general, if the wide IV had nuw, I * think* that should also be preserve-able for the steps, and if it was canonical

@lukel97 lukel97 Feb 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so too. However I'm noticing we actually drop the nuw on the wide IV after #163538. I believe the original reasoning was that with tail folding we might end up with poison lanes because the VTC > TC. But I don't think those lanes are ever used anyway. They're used for computing the header mask, which can't have any poison lanes.

@lukel97 lukel97 Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went down a bit of a rabbit hole here. IIUC we check for the canonical IV overflowing with tail folding because vscale can be a non-power-of-2.

AFAIK the plan is to remove non-power-of-2 vscales in #145098, and if that lands we can remove all the overflow checks so the increments will always have NUW.

Until then I think it's quite tricky to infer NUW. Can we relax the NUW check here and add a TODO to add it back if/when #145098 lands?

if (match(V, m_StepVector()))
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;
});
});
if (auto *WidenIV = dyn_cast<VPWidenIntOrFpInductionRecipe>(V))
Comment thread
eas marked this conversation as resolved.
return match(WidenIV->getStartValue(), m_ZeroInt()) &&
match(WidenIV->getStepValue(), m_One());
Comment thread
lukel97 marked this conversation as resolved.
Outdated
if (auto *Steps = dyn_cast<VPScalarIVStepsRecipe>(V))
return match(Steps->getOperand(0),
m_CombineOr(
m_CanonicalIV(),
m_DerivedIV(m_ZeroInt(), m_CanonicalIV(), m_One()))) &&
match(Steps->getStepValue(), m_One());
if (isa<VPWidenCanonicalIVRecipe>(V))
return true;
return vputils::isUniformAcrossVFsAndUFs(V);
}

bool VPlanVerifier::verifyLastActiveLaneRecipe(
Expand All @@ -259,18 +170,19 @@ bool VPlanVerifier::verifyLastActiveLaneRecipe(
}

const VPlan &Plan = *LastActiveLane.getParent()->getPlan();
// All operands must be prefix-mask. Currently we check for header masks or
// EVL-derived masks, as those are currently the only operands in practice,
// but this may need updating in the future.
// All operands must be prefix-mask. This means an icmp ult/ule LHS, RHS where

@david-arm david-arm Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the changes in this function be done in a separate PR or are they explicitly tied to the removal of the 'VerifyLate' parameter?

@lukel97 lukel97 Feb 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VerifyLate parameter bit was split off and landed in #182799

// the LHS is monotonically increasing and RHS is uniform.

@fhahn fhahn Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the LHS is monotonically increasing and RHS is uniform.
// the LHS is monotonically increasing and RHS is uniform across VF and UF.

@lukel97 lukel97 Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 0f62428

for (VPValue *Op : LastActiveLane.operands()) {
if (vputils::isHeaderMask(Op, Plan))
continue;

// Masks derived from EVL are also fine.
auto BroadcastOrEVL =
m_CombineOr(m_Broadcast(m_EVL(m_VPValue())), m_EVL(m_VPValue()));
if (match(Op, m_CombineOr(m_ICmp(m_StepVector(), BroadcastOrEVL),
m_ICmp(BroadcastOrEVL, m_StepVector()))))
CmpPredicate Pred;
VPValue *LHS, *RHS;
if (match(Op, m_ICmp(Pred, m_VPValue(LHS), m_VPValue(RHS))) &&
(Pred == CmpInst::ICMP_ULE || Pred == CmpInst::ICMP_ULT) &&
isKnownMonotonic(LHS) &&
(vputils::isUniformAcrossVFsAndUFs(RHS) ||

@fhahn fhahn Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the only case that it missing from vputils::isUniformAcrossVFsAndUFs(RHS) EVL? Should EVL be considered uniform-across-VF-and-UFs? Currently we never unroll with EVL so that should be fine I think

@lukel97 lukel97 Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's just EVL. I think it would be nice to avoid having the UF = 1 invariant in isUniformAcrossVFsAndUFs for EVL though. I just restricted the isSingleScalar to EVL in the verifier instead in b503231 if that works for you

vputils::isSingleScalar(RHS)))
Comment thread
eas marked this conversation as resolved.
Outdated
continue;

errs() << "LastActiveLane operand ";
Expand Down Expand Up @@ -372,12 +284,6 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
}
if (const auto *VPI = dyn_cast<VPInstruction>(&R)) {
switch (VPI->getOpcode()) {
case VPInstruction::ExplicitVectorLength:
Comment thread
lukel97 marked this conversation as resolved.
Outdated
if (!verifyEVLRecipe(*VPI)) {
errs() << "EVL VPValue is not used correctly\n";
return false;
}
break;
case VPInstruction::LastActiveLane:
if (!verifyLastActiveLaneRecipe(*VPI))
return false;
Expand Down Expand Up @@ -569,9 +475,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);
}
6 changes: 2 additions & 4 deletions llvm/lib/Transforms/Vectorize/VPlanVerifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,14 @@
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.
/// 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

Expand Down
Loading