[llvm] [VPlan] Use VPlan predecessors in VPWidenPHIRecipe (NFC). (PR #126388)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 17 04:27:24 PST 2025
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/126388
>From ebc7ac635b5901fe3fd8fc896f606027766a19f5 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/4] [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 f5d5e12b1c85d..11222ca05179d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3609,6 +3609,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 29a43e8c6fcb32f88b7cd09e35bd0b18fc1b8a7f 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/4] !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 11222ca05179d..9e3dfcbe854cb 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3613,10 +3613,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 e5c22705b0d7f5b441bc999537aa1b102df3984a 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/4] !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 9e3dfcbe854cb..5489c4c2b6f5f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3614,10 +3614,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];
>From 25b218ccf8b437069f1937c844e2a601f562f37f Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Thu, 13 Feb 2025 13:29:39 +0100
Subject: [PATCH 4/4] !fixup address latest comments, thanks
---
llvm/lib/Transforms/Vectorize/VPlan.h | 4 ++--
llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp | 6 +++---
llvm/unittests/Transforms/Vectorize/VPlanTest.cpp | 1 +
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 446d544cc1fbd..8089cfd1ce802 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1959,9 +1959,9 @@ 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
+/// their operand index corresponds to the incoming predecessor 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
+/// exactly 2 incoming values, the first from the predecessor 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/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 5489c4c2b6f5f..fec498315a63a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3612,7 +3612,9 @@ void VPReductionPHIRecipe::print(raw_ostream &O, const Twine &Indent,
VPBasicBlock *VPWidenPHIRecipe::getIncomingBlock(unsigned I) {
VPBasicBlock *Parent = getParent();
VPBlockBase *Pred = nullptr;
- if (Parent->getNumPredecessors() == 0) {
+ if (Parent->getNumPredecessors() > 0) {
+ Pred = Parent->getPredecessors()[I];
+ } else {
auto *Region = Parent->getParent();
assert(Region && !Region->isReplicator() && Region->getEntry() == Parent &&
"must be in the entry block of a non-replicate region");
@@ -3623,8 +3625,6 @@ VPBasicBlock *VPWidenPHIRecipe::getIncomingBlock(unsigned I) {
// 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];
}
return Pred->getExitingBasicBlock();
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index b17725382e33c..5f73aa43daef9 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -694,6 +694,7 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {
IntegerType *Int32 = IntegerType::get(C, 32);
VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));
WidenPhi->addOperand(Val);
+ WidenPhi->addOperand(Val);
VPBB2->appendRecipe(WidenPhi);
VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew");
More information about the llvm-commits
mailing list