[llvm] [VPlan] Use VPlan predecessors in VPWidenPHIRecipe (NFC). (PR #126388)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 12 12:16:52 PST 2025
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/126388
>From a31d78620bc50417026c76b66cc39ae3fa65d9be Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Sat, 8 Feb 2025 15:00:32 +0000
Subject: [PATCH 1/3] [VPlan] Use VPlan predecessors in VPWidenPHIRecipe (NFC).
Update VPWidenPHIRecipe to use the predecessors in VPlan to determine
the incoming blocks instead of tracking them separately. This brings
VPWidenPHIrecipe in line with the other phi recipes.
---
llvm/lib/Transforms/Vectorize/VPlan.h | 24 +++++--------------
.../Transforms/Vectorize/VPlanHCFGBuilder.cpp | 21 +++++++++-------
.../lib/Transforms/Vectorize/VPlanRecipes.cpp | 15 ++++++++++++
llvm/lib/Transforms/Vectorize/VPlanUtils.h | 10 +-------
.../LoopVectorize/outer-loop-wide-phis.ll | 2 +-
.../Transforms/Vectorize/VPlanTest.cpp | 4 ++--
6 files changed, 37 insertions(+), 39 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index fbbc466f2f7f6..9f84a3483f8cb 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1958,13 +1958,12 @@ class VPScalarPHIRecipe : public VPHeaderPHIRecipe {
#endif
};
-/// A recipe for handling phis that are widened in the vector loop.
-/// In the VPlan native path, all incoming VPValues & VPBasicBlock pairs are
-/// managed in the recipe directly.
+/// A recipe for widened phis. Incoming values are operands of the recipe and
+/// their operand index corresponds to the incoming predeocessor block. If the
+/// recipe is placed in an entry block to a (non-replicate) region, it must have
+/// exactly 2 incoming values, from from the predecessors of the region and one
+/// from the exiting block of the region.
class VPWidenPHIRecipe : public VPSingleDefRecipe {
- /// List of incoming blocks. Only used in the VPlan native path.
- SmallVector<VPBasicBlock *, 2> IncomingBlocks;
-
public:
/// Create a new VPWidenPHIRecipe for \p Phi with start value \p Start and
/// debug location \p DL.
@@ -1991,19 +1990,8 @@ class VPWidenPHIRecipe : public VPSingleDefRecipe {
VPSlotTracker &SlotTracker) const override;
#endif
- /// Adds a pair (\p IncomingV, \p IncomingBlock) to the phi.
- void addIncoming(VPValue *IncomingV, VPBasicBlock *IncomingBlock) {
- addOperand(IncomingV);
- IncomingBlocks.push_back(IncomingBlock);
- }
-
/// Returns the \p I th incoming VPBasicBlock.
- VPBasicBlock *getIncomingBlock(unsigned I) { return IncomingBlocks[I]; }
-
- /// Set the \p I th incoming VPBasicBlock to \p IncomingBlock.
- void setIncomingBlock(unsigned I, VPBasicBlock *IncomingBlock) {
- IncomingBlocks[I] = IncomingBlock;
- }
+ VPBasicBlock *getIncomingBlock(unsigned I);
/// Returns the \p I th incoming VPValue.
VPValue *getIncomingValue(unsigned I) { return getOperand(I); }
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 5a2e5d7cfee48..c94ea35e8112e 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -136,19 +136,22 @@ void PlainCFGBuilder::fixPhiNodes() {
// predecessor is the first operand of the recipe.
assert(Phi->getNumOperands() == 2);
BasicBlock *LoopPred = L->getLoopPredecessor();
- VPPhi->addIncoming(
- getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopPred)),
- BB2VPBB[LoopPred]);
+ VPPhi->addOperand(
+ getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopPred)));
BasicBlock *LoopLatch = L->getLoopLatch();
- VPPhi->addIncoming(
- getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopLatch)),
- BB2VPBB[LoopLatch]);
+ VPPhi->addOperand(
+ getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopLatch)));
continue;
}
- for (unsigned I = 0; I != Phi->getNumOperands(); ++I)
- VPPhi->addIncoming(getOrCreateVPOperand(Phi->getIncomingValue(I)),
- BB2VPBB[Phi->getIncomingBlock(I)]);
+ // Add operands for VPPhi in the order matching its predecessors in VPlan.
+ DenseMap<const VPBasicBlock *, VPValue *> IncomingValues;
+ for (unsigned I = 0; I != Phi->getNumOperands(); ++I) {
+ IncomingValues[BB2VPBB[Phi->getIncomingBlock(I)]] =
+ getOrCreateVPOperand(Phi->getIncomingValue(I));
+ }
+ for (VPBlockBase *Pred : VPPhi->getParent()->getPredecessors())
+ VPPhi->addOperand(IncomingValues.lookup(Pred->getExitingBasicBlock()));
}
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 1855fb67aa54f..45e2e6601c90c 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3603,6 +3603,21 @@ void VPReductionPHIRecipe::print(raw_ostream &O, const Twine &Indent,
}
#endif
+VPBasicBlock *VPWidenPHIRecipe::getIncomingBlock(unsigned I) {
+ VPBasicBlock *Parent = getParent();
+ VPBlockBase *Pred = nullptr;
+ if (Parent->getNumPredecessors() == 0) {
+ auto *R = Parent->getParent();
+ assert(R && R->getEntry() == Parent);
+ assert(I < 2);
+ Pred = I == 0 ? R->getSinglePredecessor() : R;
+ } else {
+ Pred = Parent->getPredecessors()[I];
+ }
+
+ return Pred->getExitingBasicBlock();
+}
+
void VPWidenPHIRecipe::execute(VPTransformState &State) {
assert(EnableVPlanNativePath &&
"Non-native vplans are not expected to have VPWidenPHIRecipes.");
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.h b/llvm/lib/Transforms/Vectorize/VPlanUtils.h
index ac5e1978fcfbe..6ddb88308955f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUtils.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.h
@@ -169,16 +169,8 @@ class VPBlockUtils {
static void reassociateBlocks(VPBlockBase *Old, VPBlockBase *New) {
for (auto *Pred : to_vector(Old->getPredecessors()))
Pred->replaceSuccessor(Old, New);
- for (auto *Succ : to_vector(Old->getSuccessors())) {
+ for (auto *Succ : to_vector(Old->getSuccessors()))
Succ->replacePredecessor(Old, New);
-
- // Replace any references to Old in widened phi incoming blocks.
- for (auto &R : Succ->getEntryBasicBlock()->phis())
- if (auto *WidenPhiR = dyn_cast<VPWidenPHIRecipe>(&R))
- for (unsigned I = 0; I < WidenPhiR->getNumOperands(); I++)
- if (WidenPhiR->getIncomingBlock(I) == Old)
- WidenPhiR->setIncomingBlock(I, cast<VPBasicBlock>(New));
- }
New->setPredecessors(Old->getPredecessors());
New->setSuccessors(Old->getSuccessors());
Old->clearPredecessors();
diff --git a/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll b/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll
index 3f81c0f5c822a..c5d2f6acf85b3 100644
--- a/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll
+++ b/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll
@@ -134,7 +134,7 @@ define void @wide_phi_2_predecessors_phi_ops_swapped(ptr noalias %A, ptr noalias
; CHECK-NEXT: [[WIDE_MASKED_GATHER:%.*]] = call <4 x i64> @llvm.masked.gather.v4i64.v4p0(<4 x ptr> [[TMP1]], i32 8, <4 x i1> splat (i1 true), <4 x i64> poison)
; CHECK-NEXT: br label %[[INNER_LATCH4]]
; CHECK: [[INNER_LATCH4]]:
-; CHECK-NEXT: [[VEC_PHI5:%.*]] = phi <4 x i64> [ zeroinitializer, %[[INNER_HEADER1]] ], [ [[WIDE_MASKED_GATHER]], %[[THEN3]] ]
+; CHECK-NEXT: [[VEC_PHI5:%.*]] = phi <4 x i64> [ [[WIDE_MASKED_GATHER]], %[[THEN3]] ], [ zeroinitializer, %[[INNER_HEADER1]] ]
; CHECK-NEXT: [[TMP2:%.*]] = add nsw <4 x i64> [[VEC_PHI5]], [[VEC_IND]]
; CHECK-NEXT: [[TMP3]] = add nsw <4 x i64> [[TMP2]], [[VEC_PHI2]]
; CHECK-NEXT: [[TMP4]] = add nuw nsw <4 x i64> [[VEC_PHI]], splat (i64 1)
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index 2f37c08bd9f11..b17725382e33c 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -672,7 +672,7 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {
auto *WidenPhi = new VPWidenPHIRecipe(nullptr);
IntegerType *Int32 = IntegerType::get(C, 32);
VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));
- WidenPhi->addIncoming(Val, VPBB1);
+ WidenPhi->addOperand(Val);
VPBB2->appendRecipe(WidenPhi);
VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew");
@@ -693,7 +693,7 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {
auto *WidenPhi = new VPWidenPHIRecipe(nullptr);
IntegerType *Int32 = IntegerType::get(C, 32);
VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));
- WidenPhi->addIncoming(Val, VPBB1);
+ WidenPhi->addOperand(Val);
VPBB2->appendRecipe(WidenPhi);
VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew");
>From d62a4230dc5da7a6354a2d7ff07f795c3b13c772 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 12 Feb 2025 21:15:37 +0100
Subject: [PATCH 2/3] !fixup address latest comments, thanks!
---
llvm/lib/Transforms/Vectorize/VPlan.h | 2 +-
llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp | 6 +++---
llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp | 10 ++++++----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 9f84a3483f8cb..7bad806169796 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1961,7 +1961,7 @@ class VPScalarPHIRecipe : public VPHeaderPHIRecipe {
/// A recipe for widened phis. Incoming values are operands of the recipe and
/// their operand index corresponds to the incoming predeocessor block. If the
/// recipe is placed in an entry block to a (non-replicate) region, it must have
-/// exactly 2 incoming values, from from the predecessors of the region and one
+/// exactly 2 incoming values, the first from the predecessors of the region and the second
/// from the exiting block of the region.
class VPWidenPHIRecipe : public VPSingleDefRecipe {
public:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index c94ea35e8112e..4f9eb0e65ec44 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -145,13 +145,13 @@ void PlainCFGBuilder::fixPhiNodes() {
}
// Add operands for VPPhi in the order matching its predecessors in VPlan.
- DenseMap<const VPBasicBlock *, VPValue *> IncomingValues;
+ DenseMap<const VPBasicBlock *, VPValue *> VPPredToIncomingValue;
for (unsigned I = 0; I != Phi->getNumOperands(); ++I) {
- IncomingValues[BB2VPBB[Phi->getIncomingBlock(I)]] =
+ VPPredToIncomingValue[BB2VPBB[Phi->getIncomingBlock(I)]] =
getOrCreateVPOperand(Phi->getIncomingValue(I));
}
for (VPBlockBase *Pred : VPPhi->getParent()->getPredecessors())
- VPPhi->addOperand(IncomingValues.lookup(Pred->getExitingBasicBlock()));
+ VPPhi->addOperand(VPPredToIncomingValue.lookup(Pred->getExitingBasicBlock()));
}
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 45e2e6601c90c..98e5d375ac776 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3607,10 +3607,12 @@ VPBasicBlock *VPWidenPHIRecipe::getIncomingBlock(unsigned I) {
VPBasicBlock *Parent = getParent();
VPBlockBase *Pred = nullptr;
if (Parent->getNumPredecessors() == 0) {
- auto *R = Parent->getParent();
- assert(R && R->getEntry() == Parent);
- assert(I < 2);
- Pred = I == 0 ? R->getSinglePredecessor() : R;
+ auto *Region = Parent->getParent();
+ assert(Region && !Region->isReplicator() && Region->getEntry() == Parent && "must be in the entry block of a non-replicate region");
+ assert(I < 2 && getNumOperands() == 2 && "when placed in an entry block, only 2 incoming blocks are available");
+
+ // I == 0 selects the predecessor of the region, I == 1 selects the region itself whose exiting block feeds the phi across the backedge.
+ Pred = I == 0 ? Region->getSinglePredecessor() : Region;
} else {
Pred = Parent->getPredecessors()[I];
}
>From ecf91bcff104663b1eaa29da43e6114ae85cbd41 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 12 Feb 2025 21:16:23 +0100
Subject: [PATCH 3/3] !fixup address latest comments, thanks!
---
llvm/lib/Transforms/Vectorize/VPlan.h | 4 ++--
llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp | 3 ++-
llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp | 10 +++++++---
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 7bad806169796..446d544cc1fbd 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1961,8 +1961,8 @@ class VPScalarPHIRecipe : public VPHeaderPHIRecipe {
/// A recipe for widened phis. Incoming values are operands of the recipe and
/// their operand index corresponds to the incoming predeocessor block. If the
/// recipe is placed in an entry block to a (non-replicate) region, it must have
-/// exactly 2 incoming values, the first from the predecessors of the region and the second
-/// from the exiting block of the region.
+/// exactly 2 incoming values, the first from the predecessors of the region and
+/// the second from the exiting block of the region.
class VPWidenPHIRecipe : public VPSingleDefRecipe {
public:
/// Create a new VPWidenPHIRecipe for \p Phi with start value \p Start and
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 4f9eb0e65ec44..33a367a0b65c1 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -151,7 +151,8 @@ void PlainCFGBuilder::fixPhiNodes() {
getOrCreateVPOperand(Phi->getIncomingValue(I));
}
for (VPBlockBase *Pred : VPPhi->getParent()->getPredecessors())
- VPPhi->addOperand(VPPredToIncomingValue.lookup(Pred->getExitingBasicBlock()));
+ VPPhi->addOperand(
+ VPPredToIncomingValue.lookup(Pred->getExitingBasicBlock()));
}
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 98e5d375ac776..d87cee61f1162 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3608,10 +3608,14 @@ VPBasicBlock *VPWidenPHIRecipe::getIncomingBlock(unsigned I) {
VPBlockBase *Pred = nullptr;
if (Parent->getNumPredecessors() == 0) {
auto *Region = Parent->getParent();
- assert(Region && !Region->isReplicator() && Region->getEntry() == Parent && "must be in the entry block of a non-replicate region");
- assert(I < 2 && getNumOperands() == 2 && "when placed in an entry block, only 2 incoming blocks are available");
+ assert(Region && !Region->isReplicator() && Region->getEntry() == Parent &&
+ "must be in the entry block of a non-replicate region");
+ assert(
+ I < 2 && getNumOperands() == 2 &&
+ "when placed in an entry block, only 2 incoming blocks are available");
- // I == 0 selects the predecessor of the region, I == 1 selects the region itself whose exiting block feeds the phi across the backedge.
+ // I == 0 selects the predecessor of the region, I == 1 selects the region
+ // itself whose exiting block feeds the phi across the backedge.
Pred = I == 0 ? Region->getSinglePredecessor() : Region;
} else {
Pred = Parent->getPredecessors()[I];
More information about the llvm-commits
mailing list