-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[VPlan] Use scalar VPPhi instead of VPWidenPHIRecipe in createPlainCFG. #150847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
fhahn
wants to merge
1
commit into
llvm:main
Choose a base branch
from
fhahn:vplan-use-scalar-phi-in-initial-vplan
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesThe initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes. Full diff: https://github.com/llvm/llvm-project/pull/150847.diff 7 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 6616e61f9bb84..b6f261a1b3cbc 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8237,7 +8237,7 @@ VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(VPSingleDefRecipe *R,
VPRecipeBase *Recipe;
Instruction *Instr = R->getUnderlyingInstr();
SmallVector<VPValue *, 4> Operands(R->operands());
- if (auto *PhiR = dyn_cast<VPWidenPHIRecipe>(R)) {
+ if (auto *PhiR = dyn_cast<VPPhi>(R)) {
VPBasicBlock *Parent = PhiR->getParent();
[[maybe_unused]] VPRegionBlock *LoopRegionOf =
Parent->getEnclosingLoopRegion();
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index a5de5933d5ff1..55930da0de1dc 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1243,8 +1243,20 @@ struct LLVM_ABI_FOR_TEST VPPhi : public VPInstruction, public VPPhiAccessors {
return R && R->getOpcode() == Instruction::PHI;
}
+ static inline bool classof(const VPValue *V) {
+ auto *R = dyn_cast<VPInstruction>(V);
+ return R && R->getOpcode() == Instruction::PHI;
+ }
+
+ static inline bool classof(const VPSingleDefRecipe *R) {
+ auto *VPI = dyn_cast<VPInstruction>(R);
+ return VPI && VPI->getOpcode() == Instruction::PHI;
+ }
+
VPPhi *clone() override {
- return new VPPhi(operands(), getDebugLoc(), getName());
+ auto *PhiR = new VPPhi(operands(), getDebugLoc(), getName());
+ PhiR->setUnderlyingValue(getUnderlyingValue());
+ return PhiR;
}
void execute(VPTransformState &State) override;
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 6c1f53b4eaa24..bd73e502768f9 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -91,17 +91,15 @@ void PlainCFGBuilder::fixHeaderPhis() {
for (auto *Phi : PhisToFix) {
assert(IRDef2VPValue.count(Phi) && "Missing VPInstruction for PHINode.");
VPValue *VPVal = IRDef2VPValue[Phi];
- assert(isa<VPWidenPHIRecipe>(VPVal) &&
- "Expected WidenPHIRecipe for phi node.");
- auto *VPPhi = cast<VPWidenPHIRecipe>(VPVal);
- assert(VPPhi->getNumOperands() == 0 &&
- "Expected VPInstruction with no operands.");
+ assert(isa<VPPhi>(VPVal) && "Expected VPPhi for phi node.");
+ auto *PhiR = cast<VPPhi>(VPVal);
+ assert(PhiR->getNumOperands() == 0 && "Expected VPPhi with no operands.");
assert(isHeaderBB(Phi->getParent(), LI->getLoopFor(Phi->getParent())) &&
"Expected Phi in header block.");
assert(Phi->getNumOperands() == 2 &&
"header phi must have exactly 2 operands");
for (BasicBlock *Pred : predecessors(Phi->getParent()))
- VPPhi->addOperand(
+ PhiR->addOperand(
getOrCreateVPOperand(Phi->getIncomingValueForBlock(Pred)));
}
}
@@ -207,8 +205,8 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
// Phi node's operands may have not been visited at this point. We create
// an empty VPInstruction that we will fix once the whole plain CFG has
// been built.
- NewR = new VPWidenPHIRecipe(Phi, nullptr, Phi->getDebugLoc(), "vec.phi");
- VPBB->appendRecipe(NewR);
+ NewR = VPIRBuilder.createScalarPhi({}, Phi->getDebugLoc(), "vec.phi");
+ NewR->setUnderlyingValue(Phi);
if (isHeaderBB(Phi->getParent(), LI->getLoopFor(Phi->getParent()))) {
// Header phis need to be fixed after the VPBB for the latch has been
// created.
diff --git a/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp b/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp
index 3b3bbc312402e..862b9301e8ca5 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanPredicator.cpp
@@ -227,10 +227,10 @@ void VPPredicator::createSwitchEdgeMasks(VPInstruction *SI) {
}
void VPPredicator::convertPhisToBlends(VPBasicBlock *VPBB) {
- SmallVector<VPWidenPHIRecipe *> Phis;
+ SmallVector<VPPhi *> Phis;
for (VPRecipeBase &R : VPBB->phis())
- Phis.push_back(cast<VPWidenPHIRecipe>(&R));
- for (VPWidenPHIRecipe *PhiR : Phis) {
+ Phis.push_back(cast<VPPhi>(&R));
+ for (VPPhi *PhiR : Phis) {
// The non-header Phi is converted into a Blend recipe below,
// so we don't have to worry about the insertion order and we can just use
// the builder. At this point we generate the predication tree. There may
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 935a4e41bab06..32b8b4f887b47 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -63,17 +63,20 @@ bool VPlanTransforms::tryToConvertVPInstructionsToVPRecipes(
Instruction *Inst = cast<Instruction>(VPV->getUnderlyingValue());
VPRecipeBase *NewRecipe = nullptr;
- if (auto *VPPhi = dyn_cast<VPWidenPHIRecipe>(&Ingredient)) {
- auto *Phi = cast<PHINode>(VPPhi->getUnderlyingValue());
+ if (auto *PhiR = dyn_cast<VPPhi>(&Ingredient)) {
+ auto *Phi = cast<PHINode>(PhiR->getUnderlyingValue());
const auto *II = GetIntOrFpInductionDescriptor(Phi);
- if (!II)
- continue;
-
- VPValue *Start = Plan->getOrAddLiveIn(II->getStartValue());
- VPValue *Step =
- vputils::getOrCreateVPValueForSCEVExpr(*Plan, II->getStep(), SE);
- NewRecipe = new VPWidenIntOrFpInductionRecipe(
- Phi, Start, Step, &Plan->getVF(), *II, Ingredient.getDebugLoc());
+ if (!II) {
+ NewRecipe = new VPWidenPHIRecipe(Phi, nullptr, PhiR->getDebugLoc());
+ for (VPValue *Op : PhiR->operands())
+ NewRecipe->addOperand(Op);
+ } else {
+ VPValue *Start = Plan->getOrAddLiveIn(II->getStartValue());
+ VPValue *Step =
+ vputils::getOrCreateVPValueForSCEVExpr(*Plan, II->getStep(), SE);
+ NewRecipe = new VPWidenIntOrFpInductionRecipe(
+ Phi, Start, Step, &Plan->getVF(), *II, Ingredient.getDebugLoc());
+ }
} else {
assert(isa<VPInstruction>(&Ingredient) &&
"only VPInstructions expected here");
diff --git a/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll b/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll
index 1cf410c359f06..32b1fc4455d37 100644
--- a/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll
+++ b/llvm/test/Transforms/LoopVectorize/outer-loop-vec-phi-predecessor-order.ll
@@ -35,7 +35,7 @@ define void @test(ptr %src, i64 %n) {
; CHECK-NEXT: [[TMP3:%.*]] = icmp eq <4 x i64> [[TMP2]], [[BROADCAST_SPLAT]]
; CHECK-NEXT: [[TMP4:%.*]] = extractelement <4 x i1> [[TMP3]], i32 0
; CHECK-NEXT: br i1 [[TMP4]], label [[LOOP_2_LATCH4]], label [[LOOP_32]]
-; CHECK: loop.2.latch4:
+; CHECK: loop.2.latch3:
; CHECK-NEXT: [[TMP5]] = add nuw nsw <4 x i64> [[VEC_PHI]], splat (i64 1)
; CHECK-NEXT: [[TMP6:%.*]] = icmp eq <4 x i64> [[TMP5]], [[BROADCAST_SPLAT]]
; CHECK-NEXT: [[TMP7:%.*]] = extractelement <4 x i1> [[TMP6]], i32 0
diff --git a/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll b/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
index 6804817c402bd..20676f3702294 100644
--- a/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
+++ b/llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll
@@ -13,14 +13,14 @@ define void @foo(i64 %n) {
; CHECK-NEXT: Successor(s): outer.header
; CHECK-EMPTY:
; CHECK-NEXT: outer.header:
-; CHECK-NEXT: WIDEN-PHI ir<%outer.iv> = phi [ ir<%outer.iv.next>, outer.latch ], [ ir<0>, ir-bb<entry> ]
+; CHECK-NEXT: EMIT-SCALAR ir<%outer.iv> = phi [ ir<%outer.iv.next>, outer.latch ], [ ir<0>, ir-bb<entry> ]
; CHECK-NEXT: EMIT ir<%gep.1> = getelementptr ir<@arr2>, ir<0>, ir<%outer.iv>
; CHECK-NEXT: EMIT store ir<%outer.iv>, ir<%gep.1>
; CHECK-NEXT: EMIT ir<%add> = add ir<%outer.iv>, ir<%n>
; CHECK-NEXT: Successor(s): inner
; CHECK-EMPTY:
; CHECK-NEXT: inner:
-; CHECK-NEXT: WIDEN-PHI ir<%inner.iv> = phi [ ir<%inner.iv.next>, inner ], [ ir<0>, outer.header ]
+; CHECK-NEXT: EMIT-SCALAR ir<%inner.iv> = phi [ ir<%inner.iv.next>, inner ], [ ir<0>, outer.header ]
; CHECK-NEXT: EMIT ir<%gep.2> = getelementptr ir<@arr>, ir<0>, ir<%inner.iv>, ir<%outer.iv>
; CHECK-NEXT: EMIT store ir<%add>, ir<%gep.2>
; CHECK-NEXT: EMIT ir<%inner.iv.next> = add ir<%inner.iv>, ir<1>
|
The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes.
32b433c
to
b2940a8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The initial VPlan closely reflects the original scalar loop, so unsing VPWidenPHIRecipe here is premature. Widened phi recipes should only be introduced together with other widened recipes.