Skip to content

Commit 02575f8

Browse files
fhahnfhahn
authored
[VPlan] Use VPInstruction for VPScalarPHIRecipe. (NFCI) (#129767)
Now that all phi nodes manage their incoming blocks through the VPlan-predecessors, there should be no need for having a dedicate recipe, it should be sufficient to allow PHI opcodes in VPInstruction. Follow-ups will also migrate VPWidenPHIRecipe and possibly others, building on top of #129388. PR: #129767
1 parent e8e267e commit 02575f8

File tree

12 files changed

+69
-95
lines changed

12 files changed

+69
-95
lines changed

‎llvm/lib/Transforms/Vectorize/VPlan.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,12 +1005,14 @@ void VPlan::execute(VPTransformState *State) {
10051005
continue;
10061006
}
10071007

1008-
auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
1009-
bool NeedsScalar = isa<VPScalarPHIRecipe>(PhiR) ||
1008+
auto *PhiR = cast<VPSingleDefRecipe>(&R);
1009+
// VPInstructions currently model scalar Phis only.
1010+
bool NeedsScalar = isa<VPInstruction>(PhiR) ||
10101011
(isa<VPReductionPHIRecipe>(PhiR) &&
10111012
cast<VPReductionPHIRecipe>(PhiR)->isInLoop());
10121013
Value *Phi = State->get(PhiR, NeedsScalar);
1013-
Value *Val = State->get(PhiR->getBackedgeValue(), NeedsScalar);
1014+
// VPHeaderPHIRecipe supports getBackedgeValue() but VPInstruction does not.
1015+
Value *Val = State->get(PhiR->getOperand(1), NeedsScalar);
10141016
cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB);
10151017
}
10161018
}

‎llvm/lib/Transforms/Vectorize/VPlan.h‎

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,7 @@ class VPRecipeBase : public ilist_node_with_parent<VPRecipeBase, VPBasicBlock>,
441441
bool mayHaveSideEffects() const;
442442

443443
/// Returns true for PHI-like recipes.
444-
bool isPhi() const {
445-
return getVPDefID() >= VPFirstPHISC && getVPDefID() <= VPLastPHISC;
446-
}
444+
bool isPhi() const;
447445

448446
/// Returns true if the recipe may read from memory.
449447
bool mayReadFromMemory() const;
@@ -1879,45 +1877,6 @@ class VPWidenPointerInductionRecipe : public VPWidenInductionRecipe,
18791877
#endif
18801878
};
18811879

1882-
/// Recipe to generate a scalar PHI. Used to generate code for recipes that
1883-
/// produce scalar header phis, including VPCanonicalIVPHIRecipe and
1884-
/// VPEVLBasedIVPHIRecipe.
1885-
class VPScalarPHIRecipe : public VPHeaderPHIRecipe {
1886-
std::string Name;
1887-
1888-
public:
1889-
VPScalarPHIRecipe(VPValue *Start, VPValue *BackedgeValue, DebugLoc DL,
1890-
StringRef Name)
1891-
: VPHeaderPHIRecipe(VPDef::VPScalarPHISC, nullptr, Start, DL),
1892-
Name(Name.str()) {
1893-
addOperand(BackedgeValue);
1894-
}
1895-
1896-
~VPScalarPHIRecipe() override = default;
1897-
1898-
VPScalarPHIRecipe *clone() override {
1899-
llvm_unreachable("cloning not implemented yet");
1900-
}
1901-
1902-
VP_CLASSOF_IMPL(VPDef::VPScalarPHISC)
1903-
1904-
/// Generate the phi/select nodes.
1905-
void execute(VPTransformState &State) override;
1906-
1907-
/// Returns true if the recipe only uses the first lane of operand \p Op.
1908-
bool onlyFirstLaneUsed(const VPValue *Op) const override {
1909-
assert(is_contained(operands(), Op) &&
1910-
"Op must be an operand of the recipe");
1911-
return true;
1912-
}
1913-
1914-
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
1915-
/// Print the recipe.
1916-
void print(raw_ostream &O, const Twine &Indent,
1917-
VPSlotTracker &SlotTracker) const override;
1918-
#endif
1919-
};
1920-
19211880
/// A recipe for widened phis. Incoming values are operands of the recipe and
19221881
/// their operand index corresponds to the incoming predecessor block. If the
19231882
/// recipe is placed in an entry block to a (non-replicate) region, it must have

‎llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
7171
}
7272
case VPInstruction::ExplicitVectorLength:
7373
return Type::getIntNTy(Ctx, 32);
74+
case Instruction::PHI:
75+
// Infer the type of first operand only, as other operands of header phi's
76+
// may lead to infinite recursion.
77+
return inferScalarType(R->getOperand(0));
7478
case VPInstruction::FirstOrderRecurrenceSplice:
7579
case VPInstruction::Not:
7680
case VPInstruction::ResumePhi:
@@ -236,14 +240,14 @@ Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
236240
TypeSwitch<const VPRecipeBase *, Type *>(V->getDefiningRecipe())
237241
.Case<VPActiveLaneMaskPHIRecipe, VPCanonicalIVPHIRecipe,
238242
VPFirstOrderRecurrencePHIRecipe, VPReductionPHIRecipe,
239-
VPWidenPointerInductionRecipe, VPEVLBasedIVPHIRecipe,
240-
VPScalarPHIRecipe>([this](const auto *R) {
241-
// Handle header phi recipes, except VPWidenIntOrFpInduction
242-
// which needs special handling due it being possibly truncated.
243-
// TODO: consider inferring/caching type of siblings, e.g.,
244-
// backedge value, here and in cases below.
245-
return inferScalarType(R->getStartValue());
246-
})
243+
VPWidenPointerInductionRecipe, VPEVLBasedIVPHIRecipe>(
244+
[this](const auto *R) {
245+
// Handle header phi recipes, except VPWidenIntOrFpInduction
246+
// which needs special handling due it being possibly truncated.
247+
// TODO: consider inferring/caching type of siblings, e.g.,
248+
// backedge value, here and in cases below.
249+
return inferScalarType(R->getStartValue());
250+
})
247251
.Case<VPWidenIntOrFpInductionRecipe, VPDerivedIVRecipe>(
248252
[](const auto *R) { return R->getScalarType(); })
249253
.Case<VPReductionRecipe, VPPredInstPHIRecipe, VPWidenPHIRecipe,

‎llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,12 @@ InstructionCost VPRecipeBase::computeCost(ElementCount VF,
277277
llvm_unreachable("subclasses should implement computeCost");
278278
}
279279

280+
bool VPRecipeBase::isPhi() const {
281+
return (getVPDefID() >= VPFirstPHISC && getVPDefID() <= VPLastPHISC) ||
282+
(isa<VPInstruction>(this) &&
283+
cast<VPInstruction>(this)->getOpcode() == Instruction::PHI);
284+
}
285+
280286
InstructionCost
281287
VPPartialReductionRecipe::computeCost(ElementCount VF,
282288
VPCostContext &Ctx) const {
@@ -418,6 +424,7 @@ bool VPInstruction::canGenerateScalarForFirstLane() const {
418424
return true;
419425
switch (Opcode) {
420426
case Instruction::ICmp:
427+
case Instruction::PHI:
421428
case Instruction::Select:
422429
case VPInstruction::BranchOnCond:
423430
case VPInstruction::BranchOnCount:
@@ -467,6 +474,17 @@ Value *VPInstruction::generate(VPTransformState &State) {
467474
Value *B = State.get(getOperand(1), OnlyFirstLaneUsed);
468475
return Builder.CreateCmp(getPredicate(), A, B, Name);
469476
}
477+
case Instruction::PHI: {
478+
assert(getParent() ==
479+
getParent()->getPlan()->getVectorLoopRegion()->getEntry() &&
480+
"VPInstructions with PHI opcodes must be used for header phis only "
481+
"at the moment");
482+
BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
483+
Value *Start = State.get(getOperand(0), VPLane(0));
484+
PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, Name);
485+
Phi->addIncoming(Start, VectorPH);
486+
return Phi;
487+
}
470488
case Instruction::Select: {
471489
bool OnlyFirstLaneUsed = vputils::onlyFirstLaneUsed(this);
472490
Value *Cond = State.get(getOperand(0), OnlyFirstLaneUsed);
@@ -771,7 +789,8 @@ bool VPInstruction::isVectorToScalar() const {
771789
}
772790

773791
bool VPInstruction::isSingleScalar() const {
774-
return getOpcode() == VPInstruction::ResumePhi;
792+
return getOpcode() == VPInstruction::ResumePhi ||
793+
getOpcode() == Instruction::PHI;
775794
}
776795

777796
#if !defined(NDEBUG)
@@ -849,6 +868,8 @@ bool VPInstruction::onlyFirstLaneUsed(const VPValue *Op) const {
849868
switch (getOpcode()) {
850869
default:
851870
return false;
871+
case Instruction::PHI:
872+
return true;
852873
case Instruction::ICmp:
853874
case Instruction::Select:
854875
case Instruction::Or:
@@ -3292,11 +3313,12 @@ void VPWidenPointerInductionRecipe::execute(VPTransformState &State) {
32923313
BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
32933314
PHINode *NewPointerPhi = nullptr;
32943315
if (CurrentPart == 0) {
3295-
auto *IVR = cast<VPHeaderPHIRecipe>(&getParent()
3296-
->getPlan()
3297-
->getVectorLoopRegion()
3298-
->getEntryBasicBlock()
3299-
->front());
3316+
auto *IVR = getParent()
3317+
->getPlan()
3318+
->getVectorLoopRegion()
3319+
->getEntryBasicBlock()
3320+
->front()
3321+
.getVPSingleValue();
33003322
PHINode *CanonicalIV = cast<PHINode>(State.get(IVR, /*IsScalar*/ true));
33013323
NewPointerPhi = PHINode::Create(ScStValueType, 2, "pointer.phi",
33023324
CanonicalIV->getIterator());
@@ -3665,22 +3687,3 @@ void VPEVLBasedIVPHIRecipe::print(raw_ostream &O, const Twine &Indent,
36653687
printOperands(O, SlotTracker);
36663688
}
36673689
#endif
3668-
3669-
void VPScalarPHIRecipe::execute(VPTransformState &State) {
3670-
BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
3671-
Value *Start = State.get(getStartValue(), VPLane(0));
3672-
PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, Name);
3673-
Phi->addIncoming(Start, VectorPH);
3674-
Phi->setDebugLoc(getDebugLoc());
3675-
State.set(this, Phi, /*IsScalar=*/true);
3676-
}
3677-
3678-
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
3679-
void VPScalarPHIRecipe::print(raw_ostream &O, const Twine &Indent,
3680-
VPSlotTracker &SlotTracker) const {
3681-
O << Indent << "SCALAR-PHI ";
3682-
printAsOperand(O, SlotTracker);
3683-
O << " = phi ";
3684-
printOperands(O, SlotTracker);
3685-
}
3686-
#endif

‎llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1747,7 +1747,7 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
17471747

17481748
// Create a scalar phi to track the previous EVL if fixed-order recurrence is
17491749
// contained.
1750-
VPScalarPHIRecipe *PrevEVL = nullptr;
1750+
VPInstruction *PrevEVL = nullptr;
17511751
bool ContainsFORs =
17521752
any_of(Header->phis(), IsaPred<VPFirstOrderRecurrencePHIRecipe>);
17531753
if (ContainsFORs) {
@@ -1762,7 +1762,8 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
17621762
VFSize > 32 ? Instruction::Trunc : Instruction::ZExt, MaxEVL,
17631763
Type::getInt32Ty(Ctx), DebugLoc());
17641764
}
1765-
PrevEVL = new VPScalarPHIRecipe(MaxEVL, &EVL, DebugLoc(), "prev.evl");
1765+
PrevEVL = new VPInstruction(Instruction::PHI, {MaxEVL, &EVL}, DebugLoc(),
1766+
"prev.evl");
17661767
PrevEVL->insertBefore(*Header, Header->getFirstNonPhi());
17671768
}
17681769

@@ -2089,9 +2090,9 @@ void VPlanTransforms::convertToConcreteRecipes(VPlan &Plan) {
20892090
auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
20902091
StringRef Name =
20912092
isa<VPCanonicalIVPHIRecipe>(PhiR) ? "index" : "evl.based.iv";
2092-
auto *ScalarR =
2093-
new VPScalarPHIRecipe(PhiR->getStartValue(), PhiR->getBackedgeValue(),
2094-
PhiR->getDebugLoc(), Name);
2093+
auto *ScalarR = new VPInstruction(
2094+
Instruction::PHI, {PhiR->getStartValue(), PhiR->getBackedgeValue()},
2095+
PhiR->getDebugLoc(), Name);
20952096
ScalarR->insertBefore(PhiR);
20962097
PhiR->replaceAllUsesWith(ScalarR);
20972098
PhiR->eraseFromParent();

‎llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
7777
if (isa<VPActiveLaneMaskPHIRecipe>(RecipeI))
7878
NumActiveLaneMaskPhiRecipes++;
7979

80-
if (IsHeaderVPBB && !isa<VPHeaderPHIRecipe, VPWidenPHIRecipe>(*RecipeI)) {
80+
if (IsHeaderVPBB && !isa<VPHeaderPHIRecipe, VPWidenPHIRecipe>(*RecipeI) &&
81+
!isa<VPInstruction>(*RecipeI) &&
82+
cast<VPInstruction>(RecipeI)->getOpcode() == Instruction::PHI) {
8183
errs() << "Found non-header PHI recipe in header VPBB";
8284
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
8385
errs() << ": ";
@@ -143,12 +145,13 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
143145
})
144146
.Case<VPWidenStoreEVLRecipe, VPReductionEVLRecipe>(
145147
[&](const VPRecipeBase *S) { return VerifyEVLUse(*S, 2); })
146-
.Case<VPWidenLoadEVLRecipe, VPReverseVectorPointerRecipe,
147-
VPScalarPHIRecipe>(
148+
.Case<VPWidenLoadEVLRecipe, VPReverseVectorPointerRecipe>(
148149
[&](const VPRecipeBase *R) { return VerifyEVLUse(*R, 1); })
149150
.Case<VPScalarCastRecipe>(
150151
[&](const VPScalarCastRecipe *S) { return VerifyEVLUse(*S, 0); })
151152
.Case<VPInstruction>([&](const VPInstruction *I) {
153+
if (I->getOpcode() == Instruction::PHI)
154+
return VerifyEVLUse(*I, 1);
152155
if (I->getOpcode() != Instruction::Add) {
153156
errs() << "EVL is used as an operand in non-VPInstruction::Add\n";
154157
return false;
@@ -208,7 +211,9 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
208211
if (!UI ||
209212
isa<VPHeaderPHIRecipe, VPWidenPHIRecipe, VPPredInstPHIRecipe>(UI) ||
210213
(isa<VPIRInstruction>(UI) &&
211-
isa<PHINode>(cast<VPIRInstruction>(UI)->getInstruction())))
214+
isa<PHINode>(cast<VPIRInstruction>(UI)->getInstruction())) ||
215+
(isa<VPInstruction>(UI) &&
216+
cast<VPInstruction>(UI)->getOpcode() == Instruction::PHI))
212217
continue;
213218

214219
// If the user is in the same block, check it comes after R in the

‎llvm/test/Transforms/LoopVectorize/AArch64/vplan-printing.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ define i32 @print_partial_reduction(ptr %a, ptr %b) {
8686
; CHECK-EMPTY:
8787
; CHECK-NEXT: <x1> vector loop: {
8888
; CHECK-NEXT: vector.body:
89-
; CHECK-NEXT: SCALAR-PHI vp<[[EP_IV:%.+]]> = phi ir<0>, vp<%index.next>
89+
; CHECK-NEXT: EMIT vp<[[EP_IV:%.+]]> = phi ir<0>, vp<%index.next>
9090
; CHECK-NEXT: WIDEN-REDUCTION-PHI ir<%accum> = phi ir<0>, ir<%add> (VF scaled by 1/4)
9191
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[EP_IV]]>, ir<1>
9292
; CHECK-NEXT: CLONE ir<%gep.a> = getelementptr ir<%a>, vp<[[STEPS]]>

‎llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ define void @vector_reverse_i64(ptr nocapture noundef writeonly %A, ptr nocaptur
193193
; CHECK-EMPTY:
194194
; CHECK-NEXT: <x1> vector loop: {
195195
; CHECK-NEXT: vector.body:
196-
; CHECK-NEXT: SCALAR-PHI vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
196+
; CHECK-NEXT: EMIT vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
197197
; CHECK-NEXT: vp<[[DEV_IV:%.+]]> = DERIVED-IV ir<%n> + vp<[[CAN_IV]]> * ir<-1>
198198
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[DEV_IV]]>, ir<-1>
199199
; CHECK-NEXT: CLONE ir<%i.0> = add nsw vp<[[STEPS]]>, ir<-1>
@@ -442,7 +442,7 @@ define void @vector_reverse_f32(ptr nocapture noundef writeonly %A, ptr nocaptur
442442
; CHECK-EMPTY:
443443
; CHECK-NEXT: <x1> vector loop: {
444444
; CHECK-NEXT: vector.body:
445-
; CHECK-NEXT: SCALAR-PHI vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
445+
; CHECK-NEXT: EMIT vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
446446
; CHECK-NEXT: vp<[[DEV_IV:%.+]]> = DERIVED-IV ir<%n> + vp<[[CAN_IV]]> * ir<-1>
447447
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[DEV_IV]]>, ir<-1>
448448
; CHECK-NEXT: CLONE ir<%i.0> = add nsw vp<[[STEPS]]>, ir<-1>

‎llvm/test/Transforms/LoopVectorize/RISCV/vplan-vp-intrinsics-fixed-order-recurrence.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ define void @first_order_recurrence(ptr noalias %A, ptr noalias %B, i64 %TC) {
2727
; IF-EVL-NEXT: EMIT vp<[[IV:%[0-9]+]]> = CANONICAL-INDUCTION
2828
; IF-EVL-NEXT: EXPLICIT-VECTOR-LENGTH-BASED-IV-PHI vp<[[EVL_PHI:%[0-9]+]]> = phi ir<0>, vp<[[IV_NEXT:%.+]]>
2929
; IF-EVL-NEXT: FIRST-ORDER-RECURRENCE-PHI ir<[[FOR_PHI:%.+]]> = phi ir<33>, ir<[[LD:%.+]]>
30-
; IF-EVL-NEXT: SCALAR-PHI vp<[[PREV_EVL:%.+]]> = phi vp<[[VF32]]>, vp<[[EVL:%.+]]>
30+
; IF-EVL-NEXT: EMIT vp<[[PREV_EVL:%.+]]> = phi vp<[[VF32]]>, vp<[[EVL:%.+]]>
3131
; IF-EVL-NEXT: EMIT vp<[[AVL:%.+]]> = sub ir<%TC>, vp<[[EVL_PHI]]>
3232
; IF-EVL-NEXT: EMIT vp<[[EVL]]> = EXPLICIT-VECTOR-LENGTH vp<[[AVL]]>
3333
; IF-EVL-NEXT: vp<[[ST:%[0-9]+]]> = SCALAR-STEPS vp<[[EVL_PHI]]>, ir<1>

‎llvm/test/Transforms/LoopVectorize/RISCV/vplan-vp-select-intrinsics.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@
3232

3333
; IF-EVL: <x1> vector loop: {
3434
; IF-EVL-NEXT: vector.body:
35-
; IF-EVL-NEXT: SCALAR-PHI vp<[[IV:%[0-9]+]]> = phi ir<0>, vp<[[IV_NEXT_EXIT:%.+]]>
36-
; IF-EVL-NEXT: SCALAR-PHI vp<[[EVL_PHI:%[0-9]+]]> = phi ir<0>, vp<[[IV_NEX:%.+]]>
35+
; IF-EVL-NEXT: EMIT vp<[[IV:%.+]]> = phi ir<0>, vp<[[IV_NEXT_EXIT:%.+]]>
36+
; IF-EVL-NEXT: EMIT vp<[[EVL_PHI:%.+]]> = phi ir<0>, vp<[[IV_NEX:%.+]]>
3737
; IF-EVL-NEXT: EMIT vp<[[AVL:%.+]]> = sub ir<%N>, vp<[[EVL_PHI]]>
3838
; IF-EVL-NEXT: EMIT vp<[[EVL:%.+]]> = EXPLICIT-VECTOR-LENGTH vp<[[AVL]]>
3939
; IF-EVL-NEXT: vp<[[ST:%[0-9]+]]> = SCALAR-STEPS vp<[[EVL_PHI]]>, ir<1>

0 commit comments

Comments
 (0)