[llvm] [VPlan] Move structural loop-region verification to verifier. (PR #215079)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 9 01:58:57 PDT 2026
https://github.com/fhahn created https://github.com/llvm/llvm-project/pull/215079
VPlan::getVectorLoopRegion() finds the vector loop region by following the last successor of each block, starting at the plan's entry. Verify that plans have the shape this relies on: the entry must have no predecessors, the region must be the only top-level loop region, and each block on the path from the entry to the region must have a single predecessor which has that block as its last successor.
This replaces the EXPENSIVE_CHECKS-only assert by checking in the verifier, allowing better handling for VPlans violating the expectations in unit tests.
Fixes an assertion with expensive checks after
https://github.com/llvm/llvm-project/pull/199437:
https://lab.llvm.org/buildbot/#/builders/187/builds/22485.
>From b7e401ebaed67a0d442cbb692a11e94c633d097f Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Sat, 8 Aug 2026 20:17:56 +0100
Subject: [PATCH] [VPlan] Move structural loop-region verification to verifier.
VPlan::getVectorLoopRegion() finds the vector loop region by following
the last successor of each block, starting at the plan's entry. Verify
that plans have the shape this relies on: the entry must have no
predecessors, the region must be the only top-level loop region, and
each block on the path from the entry to the region must have a single
predecessor which has that block as its last successor.
This replaces the EXPENSIVE_CHECKS-only assert by checking in the
verifier, allowing better handling for VPlans violating the expectations
in unit tests.
Fixes an assertion with expensive checks after
https://github.com/llvm/llvm-project/pull/199437:
https://lab.llvm.org/buildbot/#/builders/187/builds/22485.
---
llvm/lib/Transforms/Vectorize/VPlan.cpp | 36 ++----
.../Transforms/Vectorize/VPlanVerifier.cpp | 18 +++
.../Vectorize/VPlanVerifierTest.cpp | 114 ++++++++++++++++--
3 files changed, 132 insertions(+), 36 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 5b53312c3ebda..f077932cf7217 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1077,14 +1077,14 @@ InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
return Cost;
}
-// Find the vector loop region by following the last successor of each block,
-// starting from the plan's entry. The vector code path is always the last
-// successor of the entry (and of the min-iters bypass block, if present), and
-// every block on the path to the region has a single predecessor. Stop at the
-// first block with multiple predecessors: in a plain CFG that is the loop
-// header (no region exists yet), and in a rolled CFG it is the middle block
-// following the region.
-static VPRegionBlock *findVectorLoopRegion(VPBlockBase *Entry) {
+VPRegionBlock *VPlan::getVectorLoopRegion() {
+ // Find the vector loop region by following the last successor of each block,
+ // starting from the plan's entry. The vector code path is always the last
+ // successor of the entry (and of the min-iters bypass block, if present), and
+ // every block on the path to the region has a single predecessor. Stop at the
+ // first block with multiple predecessors: in a plain CFG that is the loop
+ // header (no region exists yet), and in a rolled CFG it is the middle block
+ // following the region.
for (VPBlockBase *B = Entry; B && B->getNumPredecessors() <= 1;
B = B->hasSuccessors() ? B->getSuccessors().back() : nullptr)
if (auto *R = dyn_cast<VPRegionBlock>(B))
@@ -1092,26 +1092,6 @@ static VPRegionBlock *findVectorLoopRegion(VPBlockBase *Entry) {
return nullptr;
}
-#ifdef EXPENSIVE_CHECKS
-// Reference lookup that scans every top-level block. Used only to validate
-// findVectorLoopRegion() when the invariants of the last-successor walk change.
-static VPRegionBlock *findVectorLoopRegionByScan(VPBlockBase *Entry) {
- for (VPBlockBase *B : vp_depth_first_shallow(Entry))
- if (auto *R = dyn_cast<VPRegionBlock>(B))
- return R->isReplicator() ? nullptr : R;
- return nullptr;
-}
-#endif
-
-VPRegionBlock *VPlan::getVectorLoopRegion() {
- VPRegionBlock *LoopRegion = findVectorLoopRegion(getEntry());
-#ifdef EXPENSIVE_CHECKS
- assert(LoopRegion == findVectorLoopRegionByScan(getEntry()) &&
- "fast vector loop region lookup disagrees with full CFG scan");
-#endif
- return LoopRegion;
-}
-
const VPRegionBlock *VPlan::getVectorLoopRegion() const {
return const_cast<VPlan *>(this)->getVectorLoopRegion();
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index a4d6cc0ecb480..93a6523ec03a8 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -483,7 +483,18 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
[this](const VPBlockBase *VPB) { return !verifyBlock(VPB); }))
return false;
+ // Check that the plan has a single loop region reachable from entry, and it matches the one returned by getVectorLoopRegion.
const VPRegionBlock *TopRegion = Plan.getVectorLoopRegion();
+ if (any_of(VPBlockUtils::blocksOnly<const VPRegionBlock>(
+ vp_depth_first_shallow(Plan.getEntry())),
+ [TopRegion](const VPRegionBlock *R) {
+ return !R->isReplicator() && R != TopRegion;
+ })) {
+ errs() << "VPlan must have a single top-level loop region, reachable from "
+ "the entry by following the last successor of each block\n";
+ return false;
+ }
+
// TODO: Verify all blocks using vp_depth_first_deep iterators.
if (!TopRegion)
return true;
@@ -525,6 +536,13 @@ bool VPlanVerifier::verify(const VPlan &Plan) {
}
bool llvm::verifyVPlanIsValid(const VPlan &Plan) {
+ // The entry must be the root of the plan's top-level CFG: the dominator tree
+ // constructed below and the verifier's block walks all start there.
+ if (Plan.getEntry()->hasPredecessors()) {
+ errs() << "VPlan entry block has predecessors\n";
+ return false;
+ }
+
VPDominatorTree VPDT(const_cast<VPlan &>(Plan));
VPlanVerifier Verifier(VPDT);
return Verifier.verify(Plan);
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
index ebf7a0942ae2c..7ce87be841c2d 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanVerifierTest.cpp
@@ -201,10 +201,6 @@ TEST_F(VPVerifierTest, VPPhiIncomingValueDoesntDominateIncomingBlock) {
TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
VPlan &Plan = getPlan();
- VPIRValue *Zero = Plan.getConstantInt(32, 0);
- VPInstruction *I1 =
- new VPInstruction(Instruction::Add, {Zero, Zero},
- VPIRFlags::getDefaultFlags(Instruction::Add));
VPInstruction *BranchOnCond =
new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
VPInstruction *BranchOnCond2 =
@@ -212,22 +208,124 @@ TEST_F(VPVerifierTest, DuplicateSuccessorsOutsideRegion) {
VPBasicBlock *VPBB1 = Plan.getEntry();
VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+ VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
- VPBB1->appendRecipe(I1);
- VPBB1->appendRecipe(BranchOnCond2);
VPBB2->appendRecipe(BranchOnCond);
+ VPBB3->appendRecipe(BranchOnCond2);
VPRegionBlock *R1 = Plan.createLoopRegion(Type::getInt32Ty(C), DebugLoc(),
"R1", VPBB2, VPBB2);
VPBlockUtils::connectBlocks(VPBB1, R1);
- VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBlockUtils::connectBlocks(R1, VPBB3);
- VPBlockUtils::connectBlocks(R1, Plan.getScalarHeader());
+ // VPBB3 is outside the region and has the scalar header as both successors.
+ VPBlockUtils::connectBlocks(VPBB3, Plan.getScalarHeader());
+ VPBlockUtils::connectBlocks(VPBB3, Plan.getScalarHeader());
+
+ EXPECT_EQ(R1, Plan.getVectorLoopRegion());
// Duplicate successors are allowed for blocks with conditional terminators.
EXPECT_TRUE(verifyVPlanIsValid(Plan));
}
+TEST_F(VPVerifierTest, VectorLoopRegionWithMultiplePredecessors) {
+ VPlan &Plan = getPlan();
+ VPInstruction *BranchOnCond =
+ new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
+ VPInstruction *BranchOnCond2 =
+ new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
+
+ VPBasicBlock *VPBB1 = Plan.getEntry();
+ VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+
+ VPBB1->appendRecipe(BranchOnCond2);
+ VPBB2->appendRecipe(BranchOnCond);
+
+ VPRegionBlock *R1 = Plan.createLoopRegion(Type::getInt32Ty(C), DebugLoc(),
+ "R1", VPBB2, VPBB2);
+ // Connect the region twice, so it does not have a single predecessor and
+ // cannot be reached by following the last successors from the entry.
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBlockUtils::connectBlocks(R1, Plan.getScalarHeader());
+
+#if GTEST_HAS_STREAM_REDIRECTION
+ ::testing::internal::CaptureStderr();
+#endif
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+ EXPECT_STREQ("VPlan must have a single top-level loop region, reachable from "
+ "the entry by following the last successor of each block\n",
+ ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
+TEST_F(VPVerifierTest, MultipleTopLevelLoopRegions) {
+ VPlan &Plan = getPlan();
+ VPInstruction *BranchOnCond =
+ new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
+ VPInstruction *BranchOnCond2 =
+ new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
+
+ VPBasicBlock *VPBB1 = Plan.getEntry();
+ VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+ VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
+
+ VPBB2->appendRecipe(BranchOnCond);
+ VPBB3->appendRecipe(BranchOnCond2);
+
+ VPRegionBlock *R1 = Plan.createLoopRegion(Type::getInt32Ty(C), DebugLoc(),
+ "R1", VPBB2, VPBB2);
+ VPRegionBlock *R2 = Plan.createLoopRegion(Type::getInt32Ty(C), DebugLoc(),
+ "R2", VPBB3, VPBB3);
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBlockUtils::connectBlocks(R1, R2);
+ VPBlockUtils::connectBlocks(R2, Plan.getScalarHeader());
+
+#if GTEST_HAS_STREAM_REDIRECTION
+ ::testing::internal::CaptureStderr();
+#endif
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+ EXPECT_STREQ("VPlan must have a single top-level loop region, reachable from "
+ "the entry by following the last successor of each block\n",
+ ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
+TEST_F(VPVerifierTest, EntryBlockWithPredecessor) {
+ VPlan &Plan = getPlan();
+ VPInstruction *BranchOnCond =
+ new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
+ VPInstruction *BranchOnCond2 =
+ new VPInstruction(VPInstruction::BranchOnCond, {Plan.getFalse()});
+
+ VPBasicBlock *VPBB1 = Plan.getEntry();
+ VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("");
+ VPBasicBlock *VPBB3 = Plan.createVPBasicBlock("");
+
+ VPBB2->appendRecipe(BranchOnCond);
+ VPBB3->appendRecipe(BranchOnCond2);
+
+ VPRegionBlock *R1 = Plan.createLoopRegion(Type::getInt32Ty(C), DebugLoc(),
+ "R1", VPBB2, VPBB2);
+ VPBlockUtils::connectBlocks(VPBB1, R1);
+ VPBlockUtils::connectBlocks(R1, VPBB3);
+ VPBlockUtils::connectBlocks(VPBB3, Plan.getScalarHeader());
+ // Branch back to the entry block, which must be the root of the top-level
+ // CFG.
+ VPBlockUtils::connectBlocks(VPBB3, VPBB1);
+
+#if GTEST_HAS_STREAM_REDIRECTION
+ ::testing::internal::CaptureStderr();
+#endif
+ EXPECT_FALSE(verifyVPlanIsValid(Plan));
+#if GTEST_HAS_STREAM_REDIRECTION
+ EXPECT_STREQ("VPlan entry block has predecessors\n",
+ ::testing::internal::GetCapturedStderr().c_str());
+#endif
+}
+
TEST_F(VPVerifierTest, DuplicateSuccessorsInsideRegion) {
VPlan &Plan = getPlan();
VPIRValue *Zero = Plan.getConstantInt(32, 0);
More information about the llvm-commits
mailing list