[llvm] [VPlan] Retain exit conditions and edges in initial VPlan (NFC). (PR #137709)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Wed May 7 13:48:49 PDT 2025
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/137709
>From de0fc2dce15b8cb4f22ea99374b3715c694d6fa1 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Mon, 28 Apr 2025 12:12:55 +0100
Subject: [PATCH 1/9] [VPlan] Retain exit conditions and edges in initial VPlan
(NFC).
Update initial VPlan construction to include exit conditions and
edges.
For now, all early exits are disconnected before forming the regions,
but a follow-up will update uncountable exit handling to also happen
here. This is required to enable VPlan predication and remove the
dependence any IR BBs (https://github.com/llvm/llvm-project/pull/128420).
This includes updates in a few places to use
replaceSuccessor/replacePredecessor to preserve the order of predecessors
and successors, to reduce the need of fixing up phi operand orderings.
This unfortunately required making them public, not sure if there's a
---
llvm/lib/Transforms/Vectorize/VPlan.h | 3 +-
.../Vectorize/VPlanConstruction.cpp | 78 +++++++++----------
.../vplan-printing-outer-loop.ll | 6 +-
3 files changed, 44 insertions(+), 43 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index c77c944c01a36..53332dce7181d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -117,6 +117,7 @@ class VPBlockBase {
Predecessors.erase(Pos);
}
+public:
/// Remove \p Successor from the successors of this block.
void removeSuccessor(VPBlockBase *Successor) {
auto Pos = find(Successors, Successor);
@@ -129,8 +130,6 @@ class VPBlockBase {
void replacePredecessor(VPBlockBase *Old, VPBlockBase *New) {
auto I = find(Predecessors, Old);
assert(I != Predecessors.end());
- assert(Old->getParent() == New->getParent() &&
- "replaced predecessor must have the same parent");
*I = New;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index c7132e84f689c..49edc43fd6179 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -113,6 +113,9 @@ VPBasicBlock *PlainCFGBuilder::getOrCreateVPBB(BasicBlock *BB) {
return VPBB;
}
+ if (!TheLoop->contains(BB))
+ return Plan->getExitBlock(BB);
+
// Create new VPBB.
StringRef Name = BB->getName();
LLVM_DEBUG(dbgs() << "Creating VPBasicBlock for " << Name << "\n");
@@ -146,14 +149,6 @@ bool PlainCFGBuilder::isExternalDef(Value *Val) {
// Instruction definition is in outermost loop PH.
return false;
- // Check whether Instruction definition is in a loop exit.
- SmallVector<BasicBlock *> ExitBlocks;
- TheLoop->getExitBlocks(ExitBlocks);
- if (is_contained(ExitBlocks, InstParent)) {
- // Instruction definition is in outermost loop exit.
- return false;
- }
-
// Check whether Instruction definition is in loop body.
return !TheLoop->contains(Inst);
}
@@ -202,11 +197,6 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
"Instruction shouldn't have been visited.");
if (auto *Br = dyn_cast<BranchInst>(Inst)) {
- if (TheLoop->getLoopLatch() == BB ||
- any_of(successors(BB),
- [this](BasicBlock *Succ) { return !TheLoop->contains(Succ); }))
- continue;
-
// Conditional branch instruction are represented using BranchOnCond
// recipes.
if (Br->isConditional()) {
@@ -296,7 +286,6 @@ std::unique_ptr<VPlan> PlainCFGBuilder::buildPlainCFG(
for (BasicBlock *BB : RPO) {
// Create or retrieve the VPBasicBlock for this BB.
VPBasicBlock *VPBB = getOrCreateVPBB(BB);
- Loop *LoopForBB = LI->getLoopFor(BB);
// Set VPBB predecessors in the same order as they are in the incoming BB.
setVPBBPredsFromBB(VPBB, BB);
@@ -327,24 +316,12 @@ std::unique_ptr<VPlan> PlainCFGBuilder::buildPlainCFG(
BasicBlock *IRSucc1 = BI->getSuccessor(1);
VPBasicBlock *Successor0 = getOrCreateVPBB(IRSucc0);
VPBasicBlock *Successor1 = getOrCreateVPBB(IRSucc1);
-
- // Don't connect any blocks outside the current loop except the latches for
- // inner loops.
- // TODO: Also connect exit blocks during initial VPlan construction.
- if (LoopForBB == TheLoop || BB != LoopForBB->getLoopLatch()) {
- if (!LoopForBB->contains(IRSucc0)) {
- VPBB->setOneSuccessor(Successor1);
- continue;
- }
- if (!LoopForBB->contains(IRSucc1)) {
- VPBB->setOneSuccessor(Successor0);
- continue;
- }
- }
-
VPBB->setTwoSuccessors(Successor0, Successor1);
}
+ for (auto *EB : Plan->getExitBlocks())
+ setVPBBPredsFromBB(EB, EB->getIRBasicBlock());
+
// 2. The whole CFG has been built at this point so all the input Values must
// have a VPlan counterpart. Fix VPlan header phi by adding their
// corresponding VPlan operands.
@@ -447,19 +424,21 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBlockBase *Succ = LatchVPBB->getSingleSuccessor();
assert(LatchVPBB->getNumSuccessors() <= 1 &&
"Latch has more than one successor");
- if (Succ)
- VPBlockUtils::disconnectBlocks(LatchVPBB, Succ);
+ LatchVPBB->removeSuccessor(Succ);
auto *R = Plan.createVPRegionBlock(HeaderVPB, LatchVPBB, "",
false /*isReplicator*/);
// All VPBB's reachable shallowly from HeaderVPB belong to top level loop,
// because VPlan is expected to end at top level latch disconnected above.
+ SmallPtrSet<VPBlockBase *, 2> ExitBlocks(Plan.getExitBlocks().begin(),
+ Plan.getExitBlocks().end());
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPB))
- VPBB->setParent(R);
+ if (!ExitBlocks.contains(VPBB))
+ VPBB->setParent(R);
VPBlockUtils::insertBlockAfter(R, PreheaderVPBB);
- if (Succ)
- VPBlockUtils::connectBlocks(R, Succ);
+ R->setOneSuccessor(Succ);
+ Succ->replacePredecessor(LatchVPBB, R);
}
// Add the necessary canonical IV and branch recipes required to control the
@@ -511,12 +490,33 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
VPBlockUtils::insertBlockAfter(VecPreheader, Plan.getEntry());
VPBasicBlock *MiddleVPBB = Plan.createVPBasicBlock("middle.block");
- VPBlockUtils::connectBlocks(LatchVPB, MiddleVPBB);
- LatchVPB->swapSuccessors();
+ VPBlockBase *LatchExitVPB = LatchVPB->getNumSuccessors() == 2
+ ? LatchVPB->getSuccessors()[0]
+ : nullptr;
+ if (LatchExitVPB) {
+ LatchVPB->getSuccessors()[0] = MiddleVPBB;
+ MiddleVPBB->setPredecessors({LatchVPB});
+ MiddleVPBB->setSuccessors({LatchExitVPB});
+ LatchExitVPB->replacePredecessor(LatchVPB, MiddleVPBB);
+ } else {
+ VPBlockUtils::connectBlocks(LatchVPB, MiddleVPBB);
+ LatchVPB->swapSuccessors();
+ }
addCanonicalIVRecipes(Plan, cast<VPBasicBlock>(HeaderVPB),
cast<VPBasicBlock>(LatchVPB), InductionTy, IVDL);
+ // Disconnect all edges between exit blocks other than from the latch.
+ // TODO: Uncountable exit blocks should be handled here.
+ for (VPBlockBase *EB : to_vector(Plan.getExitBlocks())) {
+ for (VPBlockBase *Pred : to_vector(EB->getPredecessors())) {
+ if (Pred == MiddleVPBB)
+ continue;
+ cast<VPBasicBlock>(Pred)->getTerminator()->eraseFromParent();
+ VPBlockUtils::disconnectBlocks(Pred, EB);
+ }
+ }
+
// Create SCEV and VPValue for the trip count.
// We use the symbolic max backedge-taken-count, which works also when
// vectorizing loops with uncountable early exits.
@@ -541,8 +541,9 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
// Thus if tail is to be folded, we know we don't need to run the
// remainder and we can set the condition to true.
// 3) Otherwise, construct a runtime check.
-
if (!RequiresScalarEpilogueCheck) {
+ if (LatchExitVPB)
+ VPBlockUtils::disconnectBlocks(MiddleVPBB, LatchExitVPB);
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
// The exit blocks are unreachable, remove their recipes to make sure no
// users remain that may pessimize transforms.
@@ -554,9 +555,6 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
}
// The connection order corresponds to the operands of the conditional branch.
- BasicBlock *IRExitBlock = TheLoop->getUniqueLatchExitBlock();
- auto *VPExitBlock = Plan.getExitBlock(IRExitBlock);
- VPBlockUtils::connectBlocks(MiddleVPBB, VPExitBlock);
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
auto *ScalarLatchTerm = TheLoop->getLoopLatch()->getTerminator();
diff --git a/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll b/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
index 91a5ea6b7fe36..fe845ae74cbee 100644
--- a/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
+++ b/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
@@ -31,7 +31,11 @@ define void @foo(i64 %n) {
; CHECK-NEXT: outer.latch:
; CHECK-NEXT: EMIT ir<%outer.iv.next> = add ir<%outer.iv>, ir<1>
; CHECK-NEXT: EMIT ir<%outer.ec> = icmp ir<%outer.iv.next>, ir<8>
-; CHECK-NEXT: Successor(s): outer.header
+; CHECK-NEXT: EMIT branch-on-cond ir<%outer.ec>
+; CHECK-NEXT: Successor(s): ir-bb<exit>, outer.header
+; CHECK-EMPTY:
+; CHECK-NEXT: ir-bb<exit>:
+; CHECK-NEXT: No successors
; CHECK-NEXT: }
entry:
br label %outer.header
>From 55b4d5e5e2bc0ae80f80eac4531c3501b5eee8f5 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 6 May 2025 19:15:47 +0100
Subject: [PATCH 2/9] !fixup address first set of comments, thanks
---
.../Vectorize/VPlanConstruction.cpp | 30 +++++++++++--------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 11f5ee0d2175d..8657f78acd22b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -113,9 +113,6 @@ VPBasicBlock *PlainCFGBuilder::getOrCreateVPBB(BasicBlock *BB) {
return VPBB;
}
- if (!TheLoop->contains(BB))
- return Plan->getExitBlock(BB);
-
// Create new VPBB.
StringRef Name = BB->getName();
LLVM_DEBUG(dbgs() << "Creating VPBasicBlock for " << Name << "\n");
@@ -249,6 +246,8 @@ std::unique_ptr<VPlan> PlainCFGBuilder::buildPlainCFG(
DenseMap<VPBlockBase *, BasicBlock *> &VPB2IRBB) {
VPIRBasicBlock *Entry = cast<VPIRBasicBlock>(Plan->getEntry());
BB2VPBB[Entry->getIRBasicBlock()] = Entry;
+ for (VPIRBasicBlock *ExitVPBB : Plan->getExitBlocks())
+ BB2VPBB[ExitVPBB->getIRBasicBlock()] = ExitVPBB;
// 1. Scan the body of the loop in a topological order to visit each basic
// block after having visited its predecessor basic blocks. Create a VPBB for
@@ -410,9 +409,12 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBlockUtils::disconnectBlocks(PreheaderVPBB, HeaderVPB);
VPBlockUtils::disconnectBlocks(LatchVPBB, HeaderVPB);
VPBlockBase *Succ = LatchVPBB->getSingleSuccessor();
- assert(LatchVPBB->getNumSuccessors() <= 1 &&
- "Latch has more than one successor");
- LatchVPBB->removeSuccessor(Succ);
+ assert(Succ && "Latch expected to be left with a single successor");
+
+ auto *PlaceHolder = Plan.createVPBasicBlock("Region place holder");
+ VPBlockUtils::insertOnEdge(LatchVPBB, Succ, PlaceHolder);
+ VPBlockUtils::disconnectBlocks(LatchVPBB, PlaceHolder);
+ VPBlockUtils::connectBlocks(PreheaderVPBB, PlaceHolder);
auto *R = Plan.createVPRegionBlock(HeaderVPB, LatchVPBB, "",
false /*isReplicator*/);
@@ -425,8 +427,9 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBB->setParent(R);
VPBlockUtils::insertBlockAfter(R, PreheaderVPBB);
- R->setOneSuccessor(Succ);
- Succ->replacePredecessor(LatchVPBB, R);
+ VPBlockUtils::insertOnEdge(PlaceHolder, Succ, R);
+ VPBlockUtils::disconnectBlocks(R, PlaceHolder);
+ VPBlockUtils::disconnectBlocks(PlaceHolder, R);
}
// Add the necessary canonical IV and branch recipes required to control the
@@ -481,11 +484,12 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
VPBlockBase *LatchExitVPB = LatchVPB->getNumSuccessors() == 2
? LatchVPB->getSuccessors()[0]
: nullptr;
- if (LatchExitVPB) {
- LatchVPB->getSuccessors()[0] = MiddleVPBB;
- MiddleVPBB->setPredecessors({LatchVPB});
- MiddleVPBB->setSuccessors({LatchExitVPB});
- LatchExitVPB->replacePredecessor(LatchVPB, MiddleVPBB);
+ // Canonical LatchVPB has header block as last successor. If it has another
+ // successor, the latter is an exit block - insert middle block on its edge.
+ // Otherwise, add middle block as another successor retaining header as last.
+ if (LatchVPB->getNumSuccessors() == 2) {
+ VPBlockBase *LatchExitVPB = LatchVPB->getSuccessors()[0];
+ VPBlockUtils::insertOnEdge(LatchVPB, LatchExitVPB, MiddleVPBB);
} else {
VPBlockUtils::connectBlocks(LatchVPB, MiddleVPBB);
LatchVPB->swapSuccessors();
>From ca45f8822607db7ca22453cd67aed2c49f1bdf5c Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 6 May 2025 20:31:14 +0100
Subject: [PATCH 3/9] !fixup address remaining comments, thanks
---
llvm/lib/Transforms/Vectorize/VPlan.h | 1 -
.../Vectorize/VPlanConstruction.cpp | 23 +++++++++++--------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 53332dce7181d..2febfe3cc7887 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -117,7 +117,6 @@ class VPBlockBase {
Predecessors.erase(Pos);
}
-public:
/// Remove \p Successor from the successors of this block.
void removeSuccessor(VPBlockBase *Successor) {
auto Pos = find(Successors, Successor);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 8657f78acd22b..fd58f802bbe9d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -411,6 +411,8 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBlockBase *Succ = LatchVPBB->getSingleSuccessor();
assert(Succ && "Latch expected to be left with a single successor");
+ // Use a temporary placeholder between LatchVPBB and its successor, to
+ // preserve the original predecessor/successor order of the blocks.
auto *PlaceHolder = Plan.createVPBasicBlock("Region place holder");
VPBlockUtils::insertOnEdge(LatchVPBB, Succ, PlaceHolder);
VPBlockUtils::disconnectBlocks(LatchVPBB, PlaceHolder);
@@ -418,8 +420,8 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
auto *R = Plan.createVPRegionBlock(HeaderVPB, LatchVPBB, "",
false /*isReplicator*/);
- // All VPBB's reachable shallowly from HeaderVPB belong to top level loop,
- // because VPlan is expected to end at top level latch disconnected above.
+ // All VPBB's reachable shallowly from HeaderVPB belong to the current region,
+ // except the exit blocks reachable via non-latch exiting blocks,
SmallPtrSet<VPBlockBase *, 2> ExitBlocks(Plan.getExitBlocks().begin(),
Plan.getExitBlocks().end());
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPB))
@@ -428,6 +430,8 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBlockUtils::insertBlockAfter(R, PreheaderVPBB);
VPBlockUtils::insertOnEdge(PlaceHolder, Succ, R);
+
+ // Remove placeholder block.
VPBlockUtils::disconnectBlocks(R, PlaceHolder);
VPBlockUtils::disconnectBlocks(PlaceHolder, R);
}
@@ -481,9 +485,6 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
VPBlockUtils::insertBlockAfter(VecPreheader, Plan.getEntry());
VPBasicBlock *MiddleVPBB = Plan.createVPBasicBlock("middle.block");
- VPBlockBase *LatchExitVPB = LatchVPB->getNumSuccessors() == 2
- ? LatchVPB->getSuccessors()[0]
- : nullptr;
// Canonical LatchVPB has header block as last successor. If it has another
// successor, the latter is an exit block - insert middle block on its edge.
// Otherwise, add middle block as another successor retaining header as last.
@@ -498,8 +499,10 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
addCanonicalIVRecipes(Plan, cast<VPBasicBlock>(HeaderVPB),
cast<VPBasicBlock>(LatchVPB), InductionTy, IVDL);
- // Disconnect all edges between exit blocks other than from the latch.
- // TODO: Uncountable exit blocks should be handled here.
+ // Disconnect all edges to exit blocks other than from the middle block.
+ // TODO: VPlans with early exits should be explicitly converted to a form only
+ // exiting via the latch here, including adjusting the exit condition, instead
+ // of simplify disconnecting the edges and adjusting the VPlan later.
for (VPBlockBase *EB : to_vector(Plan.getExitBlocks())) {
for (VPBlockBase *Pred : to_vector(EB->getPredecessors())) {
if (Pred == MiddleVPBB)
@@ -533,8 +536,9 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
// Thus if tail is to be folded, we know we don't need to run the
// remainder and we can set the condition to true.
// 3) Otherwise, construct a runtime check.
+
if (!RequiresScalarEpilogueCheck) {
- if (LatchExitVPB)
+ if (auto *LatchExitVPB = MiddleVPBB->getSingleSuccessor())
VPBlockUtils::disconnectBlocks(MiddleVPBB, LatchExitVPB);
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
// The exit blocks are unreachable, remove their recipes to make sure no
@@ -569,7 +573,8 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
void VPlanTransforms::createLoopRegions(VPlan &Plan) {
VPDominatorTree VPDT;
VPDT.recalculate(Plan);
- for (VPBlockBase *HeaderVPB : vp_depth_first_shallow(Plan.getEntry()))
+ for (VPBlockBase *HeaderVPB : post_order(
+ VPBlockShallowTraversalWrapper<VPBlockBase *>(Plan.getEntry())))
if (canonicalHeaderAndLatch(HeaderVPB, VPDT))
createLoopRegion(Plan, HeaderVPB);
>From 1284bc66b23e78f200e9eab8e4178aa4ca4ddda0 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 6 May 2025 20:54:55 +0100
Subject: [PATCH 4/9] !fixup add back assertion in replacePredecessors.
---
llvm/lib/Transforms/Vectorize/VPlan.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 2febfe3cc7887..c77c944c01a36 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -129,6 +129,8 @@ class VPBlockBase {
void replacePredecessor(VPBlockBase *Old, VPBlockBase *New) {
auto I = find(Predecessors, Old);
assert(I != Predecessors.end());
+ assert(Old->getParent() == New->getParent() &&
+ "replaced predecessor must have the same parent");
*I = New;
}
>From 3cc4b31781ca9bd3f820d063225fdafbcbebb25e Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 7 May 2025 09:38:25 +0100
Subject: [PATCH 5/9] !fixup address latest comments, thanks!
---
.../Vectorize/VPlanConstruction.cpp | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 316095d720850..0935b6e61bc31 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -408,31 +408,31 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBlockUtils::disconnectBlocks(PreheaderVPBB, HeaderVPB);
VPBlockUtils::disconnectBlocks(LatchVPBB, HeaderVPB);
- VPBlockBase *Succ = LatchVPBB->getSingleSuccessor();
- assert(Succ && "Latch expected to be left with a single successor");
+ VPBlockBase *LatchExitVPB = LatchVPBB->getSingleSuccessor();
+ assert(LatchExitVPB && "Latch expected to be left with a single successor");
- // Use a temporary placeholder between LatchVPBB and its successor, to
+ // Use a temporary placeholder between LatchVPBB and LatchExitVPB, to
// preserve the original predecessor/successor order of the blocks.
auto *PlaceHolder = Plan.createVPBasicBlock("Region place holder");
- VPBlockUtils::insertOnEdge(LatchVPBB, Succ, PlaceHolder);
+ VPBlockUtils::insertOnEdge(LatchVPBB, LatchExitVPB, PlaceHolder);
VPBlockUtils::disconnectBlocks(LatchVPBB, PlaceHolder);
VPBlockUtils::connectBlocks(PreheaderVPBB, PlaceHolder);
auto *R = Plan.createVPRegionBlock(HeaderVPB, LatchVPBB, "",
false /*isReplicator*/);
// All VPBB's reachable shallowly from HeaderVPB belong to the current region,
- // except the exit blocks reachable via non-latch exiting blocks,
+ // except the exit blocks reachable via non-latch exiting blocks.
SmallPtrSet<VPBlockBase *, 2> ExitBlocks(Plan.getExitBlocks().begin(),
Plan.getExitBlocks().end());
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPB))
if (!ExitBlocks.contains(VPBB))
VPBB->setParent(R);
- VPBlockUtils::insertBlockAfter(R, PreheaderVPBB);
- VPBlockUtils::insertOnEdge(PlaceHolder, Succ, R);
-
- // Remove placeholder block.
+ // Have R replace PlaceHolder as successor of Preheader.
+ VPBlockUtils::insertOnEdge(PreheaderVPBB, PlaceHolder, R);
VPBlockUtils::disconnectBlocks(R, PlaceHolder);
+ // Have R replace PlaceHolder as predecessor of Exit.
+ VPBlockUtils::insertOnEdge(PlaceHolder, LatchExitVPB, R);
VPBlockUtils::disconnectBlocks(PlaceHolder, R);
}
@@ -500,10 +500,10 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
cast<VPBasicBlock>(LatchVPB), InductionTy, IVDL);
// Disconnect all edges to exit blocks other than from the middle block.
- // TODO: VPlans with early exits should be explicitly converted to a form only
- // exiting via the latch here, including adjusting the exit condition, instead
- // of simplify disconnecting the edges and adjusting the VPlan later.
- for (VPBlockBase *EB : to_vector(Plan.getExitBlocks())) {
+ // TODO: VPlans with early exits should be explicitly converted to a form
+ // exiting only via the latch here, including adjusting the exit condition,
+ // instead of simply disconnecting the edges and adjusting the VPlan later.
+ for (VPBlockBase *EB : Plan.getExitBlocks()) {
for (VPBlockBase *Pred : to_vector(EB->getPredecessors())) {
if (Pred == MiddleVPBB)
continue;
>From bbb902ec491de6c861da78f5a967d61b76a9f88d Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 7 May 2025 09:47:52 +0100
Subject: [PATCH 6/9] !fixup only construct one region
---
.../Vectorize/VPlanConstruction.cpp | 28 ++++++++-----------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 0935b6e61bc31..7587eeaeca330 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -411,15 +411,18 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
VPBlockBase *LatchExitVPB = LatchVPBB->getSingleSuccessor();
assert(LatchExitVPB && "Latch expected to be left with a single successor");
- // Use a temporary placeholder between LatchVPBB and LatchExitVPB, to
- // preserve the original predecessor/successor order of the blocks.
- auto *PlaceHolder = Plan.createVPBasicBlock("Region place holder");
- VPBlockUtils::insertOnEdge(LatchVPBB, LatchExitVPB, PlaceHolder);
- VPBlockUtils::disconnectBlocks(LatchVPBB, PlaceHolder);
- VPBlockUtils::connectBlocks(PreheaderVPBB, PlaceHolder);
-
- auto *R = Plan.createVPRegionBlock(HeaderVPB, LatchVPBB, "",
- false /*isReplicator*/);
+ // Create an empty region first and insert it between PreheaderVPBB and
+ // LatchExitVPB, taking care to preserve the original predecessor & successor
+ // order of blocks. Set region entry and exiting after both HeaderVPB and
+ // LatchVPBB have been disconnected from their predecessors/successors.
+ auto *R = Plan.createVPRegionBlock("", false /*isReplicator*/);
+
+ VPBlockUtils::insertOnEdge(LatchVPBB, LatchExitVPB, R);
+ VPBlockUtils::disconnectBlocks(LatchVPBB, R);
+ VPBlockUtils::connectBlocks(PreheaderVPBB, R);
+ R->setEntry(HeaderVPB);
+ R->setExiting(LatchVPBB);
+
// All VPBB's reachable shallowly from HeaderVPB belong to the current region,
// except the exit blocks reachable via non-latch exiting blocks.
SmallPtrSet<VPBlockBase *, 2> ExitBlocks(Plan.getExitBlocks().begin(),
@@ -427,13 +430,6 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPB))
if (!ExitBlocks.contains(VPBB))
VPBB->setParent(R);
-
- // Have R replace PlaceHolder as successor of Preheader.
- VPBlockUtils::insertOnEdge(PreheaderVPBB, PlaceHolder, R);
- VPBlockUtils::disconnectBlocks(R, PlaceHolder);
- // Have R replace PlaceHolder as predecessor of Exit.
- VPBlockUtils::insertOnEdge(PlaceHolder, LatchExitVPB, R);
- VPBlockUtils::disconnectBlocks(PlaceHolder, R);
}
// Add the necessary canonical IV and branch recipes required to control the
>From c1902eda975439d3c68a08d8938e0280940a856e Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 7 May 2025 13:47:53 +0100
Subject: [PATCH 7/9] !fixup update
llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
---
llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
index dbed67a03fdeb..f33e9615d4176 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
@@ -51,7 +51,7 @@ TEST_F(VPlanHCFGTest, testBuildHCFGInnerLoop) {
// Check that the region following the preheader consists of a block for the
// original header and a separate latch.
VPBasicBlock *VecBB = Plan->getVectorLoopRegion()->getEntryBasicBlock();
- EXPECT_EQ(10u, VecBB->size());
+ EXPECT_EQ(11u, VecBB->size());
EXPECT_EQ(0u, VecBB->getNumPredecessors());
EXPECT_EQ(0u, VecBB->getNumSuccessors());
EXPECT_EQ(VecBB->getParent()->getEntryBasicBlock(), VecBB);
@@ -129,6 +129,7 @@ compound=true
" EMIT store ir\<%res\>, ir\<%arr.idx\>\l" +
" EMIT ir\<%indvars.iv.next\> = add ir\<%indvars.iv\>, ir\<1\>\l" +
" EMIT ir\<%exitcond\> = icmp ir\<%indvars.iv.next\>, ir\<%N\>\l" +
+ " EMIT vp\<%3\> = not ir\<%exitcond\>\l" +
" EMIT vp\<%index.next\> = add nuw vp\<%2\>, vp\<%0\>\l" +
" EMIT branch-on-count vp\<%index.next\>, vp\<%1\>\l" +
"No successors\l"
@@ -212,7 +213,7 @@ TEST_F(VPlanHCFGTest, testVPInstructionToVPRecipesInner) {
// Check that the region following the preheader consists of a block for the
// original header and a separate latch.
VPBasicBlock *VecBB = Plan->getVectorLoopRegion()->getEntryBasicBlock();
- EXPECT_EQ(11u, VecBB->size());
+ EXPECT_EQ(12u, VecBB->size());
EXPECT_EQ(0u, VecBB->getNumPredecessors());
EXPECT_EQ(0u, VecBB->getNumSuccessors());
EXPECT_EQ(VecBB->getParent()->getEntryBasicBlock(), VecBB);
@@ -229,6 +230,7 @@ TEST_F(VPlanHCFGTest, testVPInstructionToVPRecipesInner) {
EXPECT_NE(nullptr, dyn_cast<VPInstruction>(&*Iter++));
EXPECT_NE(nullptr, dyn_cast<VPInstruction>(&*Iter++));
EXPECT_NE(nullptr, dyn_cast<VPInstruction>(&*Iter++));
+ EXPECT_NE(nullptr, dyn_cast<VPInstruction>(&*Iter++));
EXPECT_EQ(VecBB->end(), Iter);
}
@@ -302,6 +304,7 @@ compound=true
" EMIT store ir\<%res\>, ir\<%arr.idx\>\l" +
" EMIT ir\<%iv.next\> = add ir\<%iv\>, ir\<1\>\l" +
" EMIT ir\<%exitcond\> = icmp ir\<%iv.next\>, ir\<%N\>\l" +
+ " EMIT vp\<%3\> = not ir\<%exitcond\>\l" +
" EMIT vp\<%index.next\> = add nuw vp\<%2\>, vp\<%0\>\l" +
" EMIT branch-on-count vp\<%index.next\>, vp\<%1\>\l" +
"No successors\l"
>From 0cadcf96fb02282dec6631acc249724279c47c4e Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 7 May 2025 14:35:04 +0100
Subject: [PATCH 8/9] !fixup address comments, thanks!
---
llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 7587eeaeca330..7e7b9046cf623 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -416,7 +416,6 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
// order of blocks. Set region entry and exiting after both HeaderVPB and
// LatchVPBB have been disconnected from their predecessors/successors.
auto *R = Plan.createVPRegionBlock("", false /*isReplicator*/);
-
VPBlockUtils::insertOnEdge(LatchVPBB, LatchExitVPB, R);
VPBlockUtils::disconnectBlocks(LatchVPBB, R);
VPBlockUtils::connectBlocks(PreheaderVPBB, R);
@@ -481,9 +480,10 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
VPBlockUtils::insertBlockAfter(VecPreheader, Plan.getEntry());
VPBasicBlock *MiddleVPBB = Plan.createVPBasicBlock("middle.block");
- // Canonical LatchVPB has header block as last successor. If it has another
- // successor, the latter is an exit block - insert middle block on its edge.
- // Otherwise, add middle block as another successor retaining header as last.
+ // The canonical LatchVPB has the header block as last successor. If it has
+ // another successor, this successor is an exit block - insert middle block on
+ // its edge. Otherwise, add middle block as another successor retaining header
+ // as last.
if (LatchVPB->getNumSuccessors() == 2) {
VPBlockBase *LatchExitVPB = LatchVPB->getSuccessors()[0];
VPBlockUtils::insertOnEdge(LatchVPB, LatchExitVPB, MiddleVPBB);
>From 5d6729d205762af5b41a02d55713a17b8b4fe0ce Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 7 May 2025 21:47:32 +0100
Subject: [PATCH 9/9] !fixup address latest comments, thanks
---
llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 7e7b9046cf623..ffc06c0273ebf 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -422,13 +422,11 @@ static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB) {
R->setEntry(HeaderVPB);
R->setExiting(LatchVPBB);
- // All VPBB's reachable shallowly from HeaderVPB belong to the current region,
- // except the exit blocks reachable via non-latch exiting blocks.
+ // All VPBB's reachable shallowly from HeaderVPB belong to the current region.
SmallPtrSet<VPBlockBase *, 2> ExitBlocks(Plan.getExitBlocks().begin(),
Plan.getExitBlocks().end());
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPB))
- if (!ExitBlocks.contains(VPBB))
- VPBB->setParent(R);
+ VPBB->setParent(R);
}
// Add the necessary canonical IV and branch recipes required to control the
@@ -546,7 +544,8 @@ void VPlanTransforms::prepareForVectorization(VPlan &Plan, Type *InductionTy,
return;
}
- // The connection order corresponds to the operands of the conditional branch.
+ // The connection order corresponds to the operands of the conditional branch,
+ // with the middle block already connected to the exit block.
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH);
auto *ScalarLatchTerm = TheLoop->getLoopLatch()->getTerminator();
More information about the llvm-commits
mailing list