Skip to content

[VPlan] Use VPInstruction for VPScalarPHIRecipe. (NFCI) #129767

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

Merged
merged 7 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion llvm/lib/Transforms/Vectorize/VPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1074,9 +1074,15 @@ void VPlan::execute(VPTransformState *State) {
Inc->setOperand(0, State->get(IV->getLastUnrolledPartOperand()));
continue;
}
if (auto *VPI = dyn_cast<VPInstruction>(&R)) {
Value *Phi = State->get(VPI, true);
Value *Val = State->get(VPI->getOperand(1), true);
cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB);
continue;
}

auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suffice to

Suggested change
auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
auto *PhiR = cast<VPSingleDefRecipe>(&R);

so State->get() works for both VPHeaderPHIRecipes and VPInstructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

bool NeedsScalar = isa<VPScalarPHIRecipe>(PhiR) ||
bool NeedsScalar =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool NeedsScalar =
// VPInstructions currently model scalar Phis only.
bool NeedsScalar = isa<VPInstruction>(PhiR) ||

(w/o it can drop enclosing ()).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks

(isa<VPReductionPHIRecipe>(PhiR) &&
cast<VPReductionPHIRecipe>(PhiR)->isInLoop());
Value *Phi = State->get(PhiR, NeedsScalar);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Value *Phi = State->get(PhiR, NeedsScalar);
// VPHeaderPHIRecipe supports getBackedgeValue() but VPInstruction does not.
Value *BackedgeValue = State->get(PhiR>getOperand(1), NeedsScalar);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks

Expand Down
50 changes: 8 additions & 42 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,7 @@ class VPRecipeBase : public ilist_node_with_parent<VPRecipeBase, VPBasicBlock>,
bool mayHaveSideEffects() const;

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

/// Returns true if the recipe may read from memory.
bool mayReadFromMemory() const;
Expand Down Expand Up @@ -1887,45 +1885,6 @@ class VPWidenPointerInductionRecipe : public VPWidenInductionRecipe,
#endif
};

/// Recipe to generate a scalar PHI. Used to generate code for recipes that
/// produce scalar header phis, including VPCanonicalIVPHIRecipe and
/// VPEVLBasedIVPHIRecipe.
class VPScalarPHIRecipe : public VPHeaderPHIRecipe {
std::string Name;

public:
VPScalarPHIRecipe(VPValue *Start, VPValue *BackedgeValue, DebugLoc DL,
StringRef Name)
: VPHeaderPHIRecipe(VPDef::VPScalarPHISC, nullptr, Start, DL),
Name(Name.str()) {
addOperand(BackedgeValue);
}

~VPScalarPHIRecipe() override = default;

VPScalarPHIRecipe *clone() override {
llvm_unreachable("cloning not implemented yet");
}

VP_CLASSOF_IMPL(VPDef::VPScalarPHISC)

/// Generate the phi/select nodes.
void execute(VPTransformState &State) override;

/// Returns true if the recipe only uses the first lane of operand \p Op.
bool onlyFirstLaneUsed(const VPValue *Op) const override {
assert(is_contained(operands(), Op) &&
"Op must be an operand of the recipe");
return true;
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the recipe.
void print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const override;
#endif
};

/// A recipe for widened phis. Incoming values are operands of the recipe and
/// 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
Expand Down Expand Up @@ -3004,6 +2963,13 @@ class VPWidenCanonicalIVRecipe : public VPSingleDefRecipe,
return 0;
}

/// Returns true if the recipe only uses the first lane of operand \p Op.
bool onlyFirstLaneUsed(const VPValue *Op) const override {
assert(is_contained(operands(), Op) &&
"Op must be an operand of the recipe");
return true;
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the recipe.
void print(raw_ostream &O, const Twine &Indent,
Expand Down
20 changes: 12 additions & 8 deletions llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
}
case VPInstruction::ExplicitVectorLength:
return Type::getIntNTy(Ctx, 32);
case Instruction::PHI:
// Avoid inferring the type for other operands, as this may lead to infinite
// recursions for cycles.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Avoid inferring the type for other operands, as this may lead to infinite
// recursions for cycles.
// Infer the type of first operand only, as other operands of header phi's may
// lead to infinite recursion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks!

return inferScalarType(R->getOperand(0));
case VPInstruction::FirstOrderRecurrenceSplice:
case VPInstruction::Not:
case VPInstruction::ResumePhi:
Expand Down Expand Up @@ -236,14 +240,14 @@ Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
TypeSwitch<const VPRecipeBase *, Type *>(V->getDefiningRecipe())
.Case<VPActiveLaneMaskPHIRecipe, VPCanonicalIVPHIRecipe,
VPFirstOrderRecurrencePHIRecipe, VPReductionPHIRecipe,
VPWidenPointerInductionRecipe, VPEVLBasedIVPHIRecipe,
VPScalarPHIRecipe>([this](const auto *R) {
// Handle header phi recipes, except VPWidenIntOrFpInduction
// which needs special handling due it being possibly truncated.
// TODO: consider inferring/caching type of siblings, e.g.,
// backedge value, here and in cases below.
return inferScalarType(R->getStartValue());
})
VPWidenPointerInductionRecipe, VPEVLBasedIVPHIRecipe>(
[this](const auto *R) {
// Handle header phi recipes, except VPWidenIntOrFpInduction
// which needs special handling due it being possibly truncated.
// TODO: consider inferring/caching type of siblings, e.g.,
// backedge value, here and in cases below.
return inferScalarType(R->getStartValue());
})
.Case<VPWidenIntOrFpInductionRecipe, VPDerivedIVRecipe>(
[](const auto *R) { return R->getScalarType(); })
.Case<VPReductionRecipe, VPPredInstPHIRecipe, VPWidenPHIRecipe,
Expand Down
45 changes: 21 additions & 24 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,11 @@ InstructionCost VPRecipeBase::computeCost(ElementCount VF,
VPCostContext &Ctx) const {
llvm_unreachable("subclasses should implement computeCost");
}
bool VPRecipeBase::isPhi() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, is this missing a newline above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, thanks

return (getVPDefID() >= VPFirstPHISC && getVPDefID() <= VPLastPHISC) ||
(isa<VPInstruction>(this) &&
cast<VPInstruction>(this)->getOpcode() == Instruction::PHI);
}

InstructionCost
VPPartialReductionRecipe::computeCost(ElementCount VF,
Expand Down Expand Up @@ -418,6 +423,7 @@ bool VPInstruction::canGenerateScalarForFirstLane() const {
if (isSingleScalar() || isVectorToScalar())
return true;
switch (Opcode) {
case Instruction::PHI:
case Instruction::ICmp:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case Instruction::PHI:
case Instruction::ICmp:
case Instruction::ICmp:
case Instruction::PHI:

lex order?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks!

case Instruction::Select:
case VPInstruction::BranchOnCond:
Expand Down Expand Up @@ -458,6 +464,13 @@ Value *VPInstruction::generate(VPTransformState &State) {
}

switch (getOpcode()) {
case Instruction::PHI: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhere it should be documented that VPInstructions of PHI opcode are used to model header phi's only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment here, thanks

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Perhaps worth an assert? VPInstructions with phi opcodes could be used for non-header phi's prior to predication. Eventually also for uniform branches, and out-of-loop branches.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added assert, thanks!

Will generalize this using #129389 once it lands

BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
Value *Start = State.get(getOperand(0), VPLane(0));
PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, Name);
Phi->addIncoming(Start, VectorPH);
return Phi;
}
case VPInstruction::Not: {
Value *A = State.get(getOperand(0));
return Builder.CreateNot(A, Name);
Expand Down Expand Up @@ -838,6 +851,8 @@ bool VPInstruction::onlyFirstLaneUsed(const VPValue *Op) const {
switch (getOpcode()) {
default:
return false;
case Instruction::PHI:
return true;
case Instruction::ICmp:
case Instruction::Select:
case Instruction::Or:
Expand Down Expand Up @@ -3283,11 +3298,12 @@ void VPWidenPointerInductionRecipe::execute(VPTransformState &State) {
BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
PHINode *NewPointerPhi = nullptr;
if (CurrentPart == 0) {
auto *IVR = cast<VPHeaderPHIRecipe>(&getParent()
->getPlan()
->getVectorLoopRegion()
->getEntryBasicBlock()
->front());
auto *IVR = getParent()
->getPlan()
->getVectorLoopRegion()
->getEntryBasicBlock()
->front()
.getVPSingleValue();
Comment on lines +3316 to +3321
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independently: is there a better way to retrieve IVR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be part of the operands. Will look into this separately.

PHINode *CanonicalIV = cast<PHINode>(State.get(IVR, /*IsScalar*/ true));
NewPointerPhi = PHINode::Create(ScStValueType, 2, "pointer.phi",
CanonicalIV->getIterator());
Expand Down Expand Up @@ -3666,22 +3682,3 @@ void VPEVLBasedIVPHIRecipe::print(raw_ostream &O, const Twine &Indent,
printOperands(O, SlotTracker);
}
#endif

void VPScalarPHIRecipe::execute(VPTransformState &State) {
BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
Value *Start = State.get(getStartValue(), VPLane(0));
PHINode *Phi = State.Builder.CreatePHI(Start->getType(), 2, Name);
Phi->addIncoming(Start, VectorPH);
Phi->setDebugLoc(getDebugLoc());
State.set(this, Phi, /*IsScalar=*/true);
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void VPScalarPHIRecipe::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
O << Indent << "SCALAR-PHI ";
printAsOperand(O, SlotTracker);
O << " = phi ";
printOperands(O, SlotTracker);
}
#endif
11 changes: 6 additions & 5 deletions llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1738,7 +1738,7 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {

// Create a scalar phi to track the previous EVL if fixed-order recurrence is
// contained.
VPScalarPHIRecipe *PrevEVL = nullptr;
VPInstruction *PrevEVL = nullptr;
bool ContainsFORs =
any_of(Header->phis(), IsaPred<VPFirstOrderRecurrencePHIRecipe>);
if (ContainsFORs) {
Expand All @@ -1753,7 +1753,8 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
VFSize > 32 ? Instruction::Trunc : Instruction::ZExt, MaxEVL,
Type::getInt32Ty(Ctx), DebugLoc());
}
PrevEVL = new VPScalarPHIRecipe(MaxEVL, &EVL, DebugLoc(), "prev.evl");
PrevEVL = new VPInstruction(Instruction::PHI, {MaxEVL, &EVL}, DebugLoc(),
"prev.evl");
PrevEVL->insertBefore(*Header, Header->getFirstNonPhi());
}

Expand Down Expand Up @@ -2080,9 +2081,9 @@ void VPlanTransforms::convertToConcreteRecipes(VPlan &Plan) {
auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
StringRef Name =
isa<VPCanonicalIVPHIRecipe>(PhiR) ? "index" : "evl.based.iv";
auto *ScalarR =
new VPScalarPHIRecipe(PhiR->getStartValue(), PhiR->getBackedgeValue(),
PhiR->getDebugLoc(), Name);
auto *ScalarR = new VPInstruction(
Instruction::PHI, {PhiR->getStartValue(), PhiR->getBackedgeValue()},
PhiR->getDebugLoc(), Name);
ScalarR->insertBefore(PhiR);
PhiR->replaceAllUsesWith(ScalarR);
PhiR->eraseFromParent();
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,13 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
})
.Case<VPWidenStoreEVLRecipe, VPReductionEVLRecipe>(
[&](const VPRecipeBase *S) { return VerifyEVLUse(*S, 2); })
.Case<VPWidenLoadEVLRecipe, VPReverseVectorPointerRecipe,
VPScalarPHIRecipe>(
.Case<VPWidenLoadEVLRecipe, VPReverseVectorPointerRecipe>(
[&](const VPRecipeBase *R) { return VerifyEVLUse(*R, 1); })
.Case<VPScalarCastRecipe>(
[&](const VPScalarCastRecipe *S) { return VerifyEVLUse(*S, 0); })
.Case<VPInstruction>([&](const VPInstruction *I) {
if (I->getOpcode() == Instruction::PHI)
return VerifyEVLUse(*I, I->getNumOperands() - 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using I->getNumOperands() - 1 is better than using 1 as done for VPScalarPHIRecipe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use 1, thanks!

if (I->getOpcode() != Instruction::Add) {
errs() << "EVL is used as an operand in non-VPInstruction::Add\n";
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ define i32 @print_partial_reduction(ptr %a, ptr %b) {
; CHECK-EMPTY:
; CHECK-NEXT: <x1> vector loop: {
; CHECK-NEXT: vector.body:
; CHECK-NEXT: SCALAR-PHI vp<[[EP_IV:%.+]]> = phi ir<0>, vp<%index.next>
; CHECK-NEXT: EMIT vp<[[EP_IV:%.+]]> = phi ir<0>, vp<%index.next>
; CHECK-NEXT: WIDEN-REDUCTION-PHI ir<%accum> = phi ir<0>, ir<%add> (VF scaled by 1/4)
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[EP_IV]]>, ir<1>
; CHECK-NEXT: CLONE ir<%gep.a> = getelementptr ir<%a>, vp<[[STEPS]]>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ define void @vector_reverse_i64(ptr nocapture noundef writeonly %A, ptr nocaptur
; CHECK-EMPTY:
; CHECK-NEXT: <x1> vector loop: {
; CHECK-NEXT: vector.body:
; CHECK-NEXT: SCALAR-PHI vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: EMIT vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: vp<[[DEV_IV:%.+]]> = DERIVED-IV ir<%n> + vp<[[CAN_IV]]> * ir<-1>
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[DEV_IV]]>, ir<-1>
; CHECK-NEXT: CLONE ir<%i.0> = add nsw vp<[[STEPS]]>, ir<-1>
Expand Down Expand Up @@ -442,7 +442,7 @@ define void @vector_reverse_f32(ptr nocapture noundef writeonly %A, ptr nocaptur
; CHECK-EMPTY:
; CHECK-NEXT: <x1> vector loop: {
; CHECK-NEXT: vector.body:
; CHECK-NEXT: SCALAR-PHI vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: EMIT vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: vp<[[DEV_IV:%.+]]> = DERIVED-IV ir<%n> + vp<[[CAN_IV]]> * ir<-1>
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[DEV_IV]]>, ir<-1>
; CHECK-NEXT: CLONE ir<%i.0> = add nsw vp<[[STEPS]]>, ir<-1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@

; IF-EVL: <x1> vector loop: {
; IF-EVL-NEXT: vector.body:
; IF-EVL-NEXT: SCALAR-PHI vp<[[IV:%[0-9]+]]> = phi ir<0>, vp<[[IV_NEXT_EXIT:%.+]]>
; IF-EVL-NEXT: SCALAR-PHI vp<[[EVL_PHI:%[0-9]+]]> = phi ir<0>, vp<[[IV_NEX:%.+]]>
; IF-EVL-NEXT: EMIT vp<[[IV:%.+]]> = phi ir<0>, vp<[[IV_NEXT_EXIT:%.+]]>
; IF-EVL-NEXT: EMIT vp<[[EVL_PHI:%.+]]> = phi ir<0>, vp<[[IV_NEX:%.+]]>
; IF-EVL-NEXT: EMIT vp<[[AVL:%.+]]> = sub ir<%N>, vp<[[EVL_PHI]]>
; IF-EVL-NEXT: EMIT vp<[[EVL:%.+]]> = EXPLICIT-VECTOR-LENGTH vp<[[AVL]]>
; IF-EVL-NEXT: vp<[[ST:%[0-9]+]]> = SCALAR-STEPS vp<[[EVL_PHI]]>, ir<1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ define void @switch4_default_common_dest_with_case(ptr %start, ptr %end) {
; CHECK-EMPTY:
; CHECK-NEXT: <x1> vector loop: {
; CHECK-NEXT: vector.body:
; CHECK-NEXT: SCALAR-PHI vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: EMIT vp<[[CAN_IV:%.+]]> = phi ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[CAN_IV]]>, ir<1>
; CHECK-NEXT: EMIT vp<[[PTR:%.+]]> = ptradd ir<%start>, vp<[[STEPS]]>
; CHECK-NEXT: vp<[[WIDE_PTR:%.+]]> = vector-pointer vp<[[PTR]]>
Expand Down
Loading