Skip to content

Commit e81fab6

Browse files
committed
[VPlan] Verify final VPlan, just before execution. (NFC)
Add additional verifier call just before execution, to make sure the final VPlan is valid. Note that this currently requires disabling a small number of checks when running late.
1 parent 17db75e commit e81fab6

File tree

3 files changed

+39
-24
lines changed

3 files changed

+39
-24
lines changed

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7802,7 +7802,6 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
78027802
VPlanTransforms::removeDeadRecipes(BestVPlan);
78037803
VPlanTransforms::convertToConcreteRecipes(BestVPlan,
78047804
*Legal->getWidestInductionType());
7805-
78067805
// Perform the actual loop transformation.
78077806
VPTransformState State(&TTI, BestVF, LI, DT, ILV.AC, ILV.Builder, &BestVPlan,
78087807
OrigLoop->getParentLoop(),
@@ -7843,6 +7842,9 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
78437842
if (VectorizingEpilogue)
78447843
VPlanTransforms::removeDeadRecipes(BestVPlan);
78457844

7845+
assert(verifyVPlanIsValid(BestVPlan, true /*VerifyLate*/) &&
7846+
"final VPlan is invalid");
7847+
78467848
ILV.printDebugTracesAtStart();
78477849

78487850
//===------------------------------------------------===//

llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ namespace {
2828
class VPlanVerifier {
2929
const VPDominatorTree &VPDT;
3030
VPTypeAnalysis &TypeInfo;
31+
bool VerifyLate;
3132

3233
SmallPtrSet<BasicBlock *, 8> WrappedIRBBs;
3334

@@ -60,8 +61,9 @@ class VPlanVerifier {
6061
bool verifyRegionRec(const VPRegionBlock *Region);
6162

6263
public:
63-
VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo)
64-
: VPDT(VPDT), TypeInfo(TypeInfo) {}
64+
VPlanVerifier(VPDominatorTree &VPDT, VPTypeAnalysis &TypeInfo,
65+
bool VerifyLate)
66+
: VPDT(VPDT), TypeInfo(TypeInfo), VerifyLate(VerifyLate) {}
6567

6668
bool verify(const VPlan &Plan);
6769
};
@@ -113,7 +115,7 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
113115
RecipeI++;
114116
}
115117

116-
if (NumActiveLaneMaskPhiRecipes > 1) {
118+
if (!VerifyLate && NumActiveLaneMaskPhiRecipes > 1) {
117119
errs() << "There should be no more than one VPActiveLaneMaskPHIRecipe";
118120
return false;
119121
}
@@ -151,7 +153,7 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
151153
}
152154
return true;
153155
};
154-
return all_of(EVL.users(), [&VerifyEVLUse](VPUser *U) {
156+
return all_of(EVL.users(), [this, &VerifyEVLUse](VPUser *U) {
155157
return TypeSwitch<const VPUser *, bool>(U)
156158
.Case<VPWidenIntrinsicRecipe>([&](const VPWidenIntrinsicRecipe *S) {
157159
return VerifyEVLUse(*S, S->getNumOperands() - 1);
@@ -174,7 +176,7 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
174176
"users\n";
175177
return false;
176178
}
177-
if (!isa<VPEVLBasedIVPHIRecipe>(*I->users().begin())) {
179+
if (!VerifyLate && !isa<VPEVLBasedIVPHIRecipe>(*I->users().begin())) {
178180
errs() << "Result of VPInstruction::Add with EVL operand is "
179181
"not used by VPEVLBasedIVPHIRecipe\n";
180182
return false;
@@ -243,7 +245,9 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
243245
continue;
244246
}
245247
// TODO: Also verify VPPredInstPHIRecipe.
246-
if (isa<VPPredInstPHIRecipe>(UI))
248+
if (isa<VPPredInstPHIRecipe>(UI) ||
249+
(isa<VPInstruction>(UI) && (cast<VPInstruction>(UI)->getOpcode() ==
250+
VPInstruction::ResumePhi)))
247251
continue;
248252

249253
// If the user is in the same block, check it comes after R in the
@@ -302,18 +306,20 @@ static bool hasDuplicates(const SmallVectorImpl<VPBlockBase *> &VPBlockVec) {
302306
bool VPlanVerifier::verifyBlock(const VPBlockBase *VPB) {
303307
auto *VPBB = dyn_cast<VPBasicBlock>(VPB);
304308
// Check block's condition bit.
305-
if (VPB->getNumSuccessors() > 1 ||
306-
(VPBB && VPBB->getParent() && VPBB->isExiting() &&
307-
!VPBB->getParent()->isReplicator())) {
308-
if (!VPBB || !VPBB->getTerminator()) {
309-
errs() << "Block has multiple successors but doesn't "
310-
"have a proper branch recipe!\n";
311-
return false;
312-
}
313-
} else {
314-
if (VPBB && VPBB->getTerminator()) {
315-
errs() << "Unexpected branch recipe!\n";
316-
return false;
309+
if (!isa<VPIRBasicBlock>(VPB)) {
310+
if (VPB->getNumSuccessors() > 1 ||
311+
(VPBB && VPBB->getParent() && VPBB->isExiting() &&
312+
!VPBB->getParent()->isReplicator())) {
313+
if (!VPBB || !VPBB->getTerminator()) {
314+
errs() << "Block has multiple successors but doesn't "
315+
"have a proper branch recipe!\n";
316+
return false;
317+
}
318+
} else {
319+
if (VPBB && VPBB->getTerminator()) {
320+
errs() << "Unexpected branch recipe!\n";
321+
return false;
322+
}
317323
}
318324
}
319325

@@ -409,6 +415,10 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
409415
return false;
410416

411417
const VPRegionBlock *TopRegion = Plan.getVectorLoopRegion();
418+
// TODO: Verify all blocks using vp_depth_first_deep iterators.
419+
if (!TopRegion)
420+
return true;
421+
412422
if (!verifyRegionRec(TopRegion))
413423
return false;
414424

@@ -423,7 +433,8 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
423433
return false;
424434
}
425435

426-
if (!isa<VPCanonicalIVPHIRecipe>(&*Entry->begin())) {
436+
// TODO: Remove once loop regions are dissolved before execution.
437+
if (!VerifyLate && !isa<VPCanonicalIVPHIRecipe>(&*Entry->begin())) {
427438
errs() << "VPlan vector loop header does not start with a "
428439
"VPCanonicalIVPHIRecipe\n";
429440
return false;
@@ -452,10 +463,10 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
452463
return true;
453464
}
454465

455-
bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
466+
bool llvm::verifyVPlanIsValid(const VPlan &Plan, bool VerifyLate) {
456467
VPDominatorTree VPDT;
457468
VPDT.recalculate(const_cast<VPlan &>(Plan));
458469
VPTypeAnalysis TypeInfo(Plan);
459-
VPlanVerifier Verifier(VPDT, TypeInfo);
470+
VPlanVerifier Verifier(VPDT, TypeInfo, VerifyLate);
460471
return Verifier.verify(Plan);
461472
}

llvm/lib/Transforms/Vectorize/VPlanVerifier.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@
2727
namespace llvm {
2828
class VPlan;
2929

30-
/// Verify invariants for general VPlans. Currently it checks the following:
30+
/// Verify invariants for general VPlans. If \p VerifyLate is passed, skip some
31+
/// checks that are not applicable at later stages of the transform pipeline.
32+
/// Currently it checks the following:
3133
/// 1. Region/Block verification: Check the Region/Block verification
3234
/// invariants for every region in the H-CFG.
3335
/// 2. all phi-like recipes must be at the beginning of a block, with no other
3436
/// recipes in between. Note that currently there is still an exception for
3537
/// VPBlendRecipes.
36-
bool verifyVPlanIsValid(const VPlan &Plan);
38+
bool verifyVPlanIsValid(const VPlan &Plan, bool VerifyLate = false);
3739

3840
} // namespace llvm
3941

0 commit comments

Comments
 (0)