Skip to content

[VPlan] Add VPWidenInduction recipe as common base class (NFC). #120008

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.

Sign up for GitHub

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 6 commits into from
Dec 16, 2024
Merged

[VPlan] Add VPWidenInduction recipe as common base class (NFC). #120008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbdc0bb
[VPlan] Get types and step from VPWidenPointerInductionRecipe (NFC).
fhahn Dec 15, 2024
89a1b3a
[VPlan] Add VPWidenInduction recipe as common base class (NFC).
fhahn Dec 15, 2024
042188a
Merge branch 'main' into vpwideninductionrecipe
fhahn Dec 15, 2024
4fe6ecb
Merge remote-tracking branch 'origin/main' into vpwideninductionrecipe
fhahn Dec 15, 2024
438067d
!fixup address comments, thanks
fhahn Dec 15, 2024
dcba17f
Merge remote-tracking branch 'origin/main' into vpwideninductionrecipe
fhahn Dec 15, 2024
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
12 changes: 2 additions & 10 deletions llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7941,10 +7941,8 @@ BasicBlock *EpilogueVectorizerMainLoop::createEpilogueVectorizedLoopSkeleton(
SmallPtrSet<PHINode *, 4> WideIVs;
for (VPRecipeBase &H :
EPI.EpiloguePlan.getVectorLoopRegion()->getEntryBasicBlock()->phis()) {
if (auto *WideIV = dyn_cast<VPWidenIntOrFpInductionRecipe>(&H))
if (auto *WideIV = dyn_cast<VPWidenInductionRecipe>(&H))
WideIVs.insert(WideIV->getPHINode());
else if (auto *PtrIV = dyn_cast<VPWidenPointerInductionRecipe>(&H))
WideIVs.insert(cast<PHINode>(PtrIV->getUnderlyingValue()));
}
createInductionResumeVPValues(ExpandedSCEVs, nullptr, &WideIVs);

Expand Down Expand Up @@ -10131,13 +10129,7 @@ preparePlanForEpilogueVectorLoop(VPlan &Plan, Loop *L,
} else {
// Retrieve the induction resume values for wide inductions from
// their original phi nodes in the scalar loop.
PHINode *IndPhi = nullptr;
if (auto *Ind = dyn_cast<VPWidenPointerInductionRecipe>(&R)) {
IndPhi = cast<PHINode>(Ind->getUnderlyingValue());
} else {
auto *WidenInd = cast<VPWidenIntOrFpInductionRecipe>(&R);
IndPhi = WidenInd->getPHINode();
}
PHINode *IndPhi = cast<VPWidenInductionRecipe>(&R)->getPHINode();
// Hook up to the PHINode generated by a ResumePhi recipe of main
// loop VPlan, which feeds the scalar loop.
ResumeV = IndPhi->getIncomingValueForBlock(L->getLoopPreheader());
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Vectorize/VPlan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ void VPlan::execute(VPTransformState *State) {
if (isa<VPWidenPHIRecipe>(&R))
continue;

if (isa<VPWidenPointerInductionRecipe, VPWidenIntOrFpInductionRecipe>(&R)) {
if (isa<VPWidenInductionRecipe>(&R)) {
PHINode *Phi = nullptr;
if (isa<VPWidenIntOrFpInductionRecipe>(&R)) {
Phi = cast<PHINode>(State->get(R.getVPSingleValue()));
Expand Down
110 changes: 61 additions & 49 deletions llvm/lib/Transforms/Vectorize/VPlan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2099,38 +2099,81 @@ class VPHeaderPHIRecipe : public VPSingleDefRecipe {
}
};

/// Base class for widened induction (VPWidenIntOrFpInductionRecipe and
Copy link
Collaborator

Choose a reason for hiding this comment

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

Independent: perhaps worth handling Int and FP separately?

/// VPWidenPointerInductionRecipe), providing shared functionality, including
/// retrieving the step value, induction descriptor and original phi node.
class VPWidenInductionRecipe : public VPHeaderPHIRecipe {
const InductionDescriptor &IndDesc;

public:
VPWidenInductionRecipe(unsigned char Kind, PHINode *IV, VPValue *Start,
VPValue *Step, const InductionDescriptor &IndDesc,
DebugLoc DL)
: VPHeaderPHIRecipe(Kind, IV, Start, DL), IndDesc(IndDesc) {
addOperand(Step);
}

static inline bool classof(const VPRecipeBase *R) {
return R->getVPDefID() == VPDef::VPWidenIntOrFpInductionSC ||
R->getVPDefID() == VPDef::VPWidenPointerInductionSC;
}

virtual void execute(VPTransformState &State) override = 0;

/// Returns the step value of the induction.
VPValue *getStepValue() { return getOperand(1); }
const VPValue *getStepValue() const { return getOperand(1); }

PHINode *getPHINode() const { return cast<PHINode>(getUnderlyingValue()); }

/// Returns the induction descriptor for the recipe.
const InductionDescriptor &getInductionDescriptor() const { return IndDesc; }

VPValue *getBackedgeValue() override {
// TODO: All operands of base recipe must exist and be at same index in
// derived recipe.
llvm_unreachable(
"VPWidenIntOrFpInductionRecipe generates its own backedge value");
}

VPRecipeBase &getBackedgeRecipe() override {
// TODO: All operands of base recipe must exist and be at same index in
// derived recipe.
llvm_unreachable(
"VPWidenIntOrFpInductionRecipe generates its own backedge value");
}
};

/// A recipe for handling phi nodes of integer and floating-point inductions,
/// producing their vector values.
class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
PHINode *IV;
class VPWidenIntOrFpInductionRecipe : public VPWidenInductionRecipe {
TruncInst *Trunc;
const InductionDescriptor &IndDesc;

public:
VPWidenIntOrFpInductionRecipe(PHINode *IV, VPValue *Start, VPValue *Step,
VPValue *VF, const InductionDescriptor &IndDesc,
DebugLoc DL)
: VPHeaderPHIRecipe(VPDef::VPWidenIntOrFpInductionSC, IV, Start, DL),
IV(IV), Trunc(nullptr), IndDesc(IndDesc) {
addOperand(Step);
: VPWidenInductionRecipe(VPDef::VPWidenIntOrFpInductionSC, IV, Start,
Step, IndDesc, DL),
Trunc(nullptr) {
addOperand(VF);
}

VPWidenIntOrFpInductionRecipe(PHINode *IV, VPValue *Start, VPValue *Step,
VPValue *VF, const InductionDescriptor &IndDesc,
TruncInst *Trunc, DebugLoc DL)
: VPHeaderPHIRecipe(VPDef::VPWidenIntOrFpInductionSC, Trunc, Start, DL),
IV(IV), Trunc(Trunc), IndDesc(IndDesc) {
addOperand(Step);
: VPWidenInductionRecipe(VPDef::VPWidenIntOrFpInductionSC, IV, Start,
Step, IndDesc, DL),
Comment on lines -2122 to +2166
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not NFC because the underlying instruction changed from Trunc to IV here.

Trunc(Trunc) {
addOperand(VF);
}

~VPWidenIntOrFpInductionRecipe() override = default;

VPWidenIntOrFpInductionRecipe *clone() override {
return new VPWidenIntOrFpInductionRecipe(IV, getStartValue(),
getStepValue(), getVFValue(),
IndDesc, Trunc, getDebugLoc());
return new VPWidenIntOrFpInductionRecipe(
getPHINode(), getStartValue(), getStepValue(), getVFValue(),
getInductionDescriptor(), Trunc, getDebugLoc());
}

VP_CLASSOF_IMPL(VPDef::VPWidenIntOrFpInductionSC)
Expand All @@ -2145,24 +2188,6 @@ class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
VPSlotTracker &SlotTracker) const override;
#endif

VPValue *getBackedgeValue() override {
// TODO: All operands of base recipe must exist and be at same index in
// derived recipe.
llvm_unreachable(
"VPWidenIntOrFpInductionRecipe generates its own backedge value");
}

VPRecipeBase &getBackedgeRecipe() override {
// TODO: All operands of base recipe must exist and be at same index in
// derived recipe.
llvm_unreachable(
"VPWidenIntOrFpInductionRecipe generates its own backedge value");
}

/// Returns the step value of the induction.
VPValue *getStepValue() { return getOperand(1); }
const VPValue *getStepValue() const { return getOperand(1); }

VPValue *getVFValue() { return getOperand(2); }
const VPValue *getVFValue() const { return getOperand(2); }

Expand All @@ -2177,19 +2202,14 @@ class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
TruncInst *getTruncInst() { return Trunc; }
const TruncInst *getTruncInst() const { return Trunc; }

PHINode *getPHINode() { return IV; }

/// Returns the induction descriptor for the recipe.
const InductionDescriptor &getInductionDescriptor() const { return IndDesc; }

/// Returns true if the induction is canonical, i.e. starting at 0 and
/// incremented by UF * VF (= the original IV is incremented by 1) and has the
/// same type as the canonical induction.
bool isCanonical() const;

/// Returns the scalar type of the induction.
Type *getScalarType() const {
return Trunc ? Trunc->getType() : IV->getType();
return Trunc ? Trunc->getType() : getPHINode()->getType();
}

/// Returns the VPValue representing the value of this induction at
Expand All @@ -2200,10 +2220,8 @@ class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
}
};

class VPWidenPointerInductionRecipe : public VPHeaderPHIRecipe,
class VPWidenPointerInductionRecipe : public VPWidenInductionRecipe,
public VPUnrollPartAccessor<3> {
const InductionDescriptor &IndDesc;

bool IsScalarAfterVectorization;

public:
Expand All @@ -2212,19 +2230,16 @@ class VPWidenPointerInductionRecipe : public VPHeaderPHIRecipe,
VPWidenPointerInductionRecipe(PHINode *Phi, VPValue *Start, VPValue *Step,
const InductionDescriptor &IndDesc,
bool IsScalarAfterVectorization, DebugLoc DL)
: VPHeaderPHIRecipe(VPDef::VPWidenPointerInductionSC, Phi, nullptr, DL),
IndDesc(IndDesc),
IsScalarAfterVectorization(IsScalarAfterVectorization) {
addOperand(Start);
addOperand(Step);
}
: VPWidenInductionRecipe(VPDef::VPWidenPointerInductionSC, Phi, Start,
Step, IndDesc, DL),
IsScalarAfterVectorization(IsScalarAfterVectorization) {}

~VPWidenPointerInductionRecipe() override = default;

VPWidenPointerInductionRecipe *clone() override {
return new VPWidenPointerInductionRecipe(
cast<PHINode>(getUnderlyingInstr()), getOperand(0), getOperand(1),
IndDesc, IsScalarAfterVectorization, getDebugLoc());
getInductionDescriptor(), IsScalarAfterVectorization, getDebugLoc());
}

VP_CLASSOF_IMPL(VPDef::VPWidenPointerInductionSC)
Expand All @@ -2235,9 +2250,6 @@ class VPWidenPointerInductionRecipe : public VPHeaderPHIRecipe,
/// Returns true if only scalar values will be generated.
bool onlyScalarsGenerated(bool IsScalable);

/// Returns the induction descriptor for the recipe.
const InductionDescriptor &getInductionDescriptor() const { return IndDesc; }

/// Returns the VPValue representing the value of this induction at
/// the first unrolled part, if it exists. Returns itself if unrolling did not
/// take place.
Expand Down
14 changes: 8 additions & 6 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,12 +1660,13 @@ void VPWidenIntOrFpInductionRecipe::execute(VPTransformState &State) {
const InductionDescriptor &ID = getInductionDescriptor();
TruncInst *Trunc = getTruncInst();
IRBuilderBase &Builder = State.Builder;
assert(IV->getType() == ID.getStartValue()->getType() && "Types must match");
assert(getPHINode()->getType() == ID.getStartValue()->getType() &&
"Types must match");
assert(State.VF.isVector() && "must have vector VF");

// The value from the original loop to which we are mapping the new induction
// variable.
Instruction *EntryVal = Trunc ? cast<Instruction>(Trunc) : IV;
Instruction *EntryVal = Trunc ? cast<Instruction>(Trunc) : getPHINode();

// Fast-math-flags propagate from the original induction instruction.
IRBuilder<>::FastMathFlagGuard FMFG(Builder);
Expand Down Expand Up @@ -3150,7 +3151,8 @@ bool VPWidenPointerInductionRecipe::onlyScalarsGenerated(bool IsScalable) {
}

void VPWidenPointerInductionRecipe::execute(VPTransformState &State) {
assert(IndDesc.getKind() == InductionDescriptor::IK_PtrInduction &&
assert(getInductionDescriptor().getKind() ==
InductionDescriptor::IK_PtrInduction &&
"Not a pointer induction according to InductionDescriptor!");
assert(cast<PHINode>(getUnderlyingInstr())->getType()->isPointerTy() &&
"Unexpected type.");
Expand Down Expand Up @@ -3186,8 +3188,8 @@ void VPWidenPointerInductionRecipe::execute(VPTransformState &State) {

// A pointer induction, performed by using a gep
BasicBlock::iterator InductionLoc = State.Builder.GetInsertPoint();
Value *ScalarStepValue = State.get(getOperand(1), VPLane(0));
Type *PhiType = State.TypeAnalysis.inferScalarType(getOperand(1));
Value *ScalarStepValue = State.get(getStepValue(), VPLane(0));
Type *PhiType = State.TypeAnalysis.inferScalarType(getStepValue());
Value *RuntimeVF = getRuntimeVF(State.Builder, PhiType, State.VF);
// Add induction update using an incorrect block temporarily. The phi node
// will be fixed after VPlan execution. Note that at this point the latch
Expand Down Expand Up @@ -3240,7 +3242,7 @@ void VPWidenPointerInductionRecipe::print(raw_ostream &O, const Twine &Indent,
O << " = WIDEN-POINTER-INDUCTION ";
getStartValue()->printAsOperand(O, SlotTracker);
O << ", ";
getOperand(1)->printAsOperand(O, SlotTracker);
getStepValue()->printAsOperand(O, SlotTracker);
if (getNumOperands() == 4) {
O << ", ";
getOperand(2)->printAsOperand(O, SlotTracker);
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/LoopVectorize/vplan-printing.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,10 +653,10 @@ define void @print_expand_scev(i64 %y, ptr %ptr) {
; CHECK-NEXT: <x1> vector loop: {
; CHECK-NEXT: vector.body:
; CHECK-NEXT: EMIT vp<[[CAN_IV:%.+]]> = CANONICAL-INDUCTION ir<0>, vp<[[CAN_IV_NEXT:%.+]]>
; CHECK-NEXT: ir<%v2> = WIDEN-INDUCTION ir<0>, vp<[[EXP_SCEV]]>, vp<[[VF]]> (truncated to i8)
; CHECK-NEXT: ir<%iv> = WIDEN-INDUCTION ir<0>, vp<[[EXP_SCEV]]>, vp<[[VF]]> (truncated to i8)
; CHECK-NEXT: vp<[[DERIVED_IV:%.+]]> = DERIVED-IV ir<0> + vp<[[CAN_IV]]> * vp<[[EXP_SCEV]]>
; CHECK-NEXT: vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[DERIVED_IV]]>, vp<[[EXP_SCEV]]>
; CHECK-NEXT: WIDEN ir<%v3> = add nuw ir<%v2>, ir<1>
; CHECK-NEXT: WIDEN ir<%v3> = add nuw ir<%iv>, ir<1>
Copy link
Collaborator

Choose a reason for hiding this comment

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

How/is this related? It originally adds %v2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The patch changes VPWidenIntOrFpInductionRecipe to always use the phi node as underlying value (for consistency and to make it easier to retrieve it generically), whereas before the Trunc would be the underlying value.

This wasn't really obvious from the old printing for VPWidenIntOrFpInductionRecipe, which didn't show the result VPValue and VPValue operands properly. Modernized printing in 4304505, hopefully clearer now and more in line with printing for other recipes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, the switch to hold Trunc rather than IV separately is clear. Could Trunc continue to be printed as the result %ir<%v2> of the widened induction even if its no longer the underlying?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately I don't think this would be possible, as at other places we usually print the underlying value of the VPValue, and trying to retrieve the Trunc would add a number of complexities.

; CHECK-NEXT: REPLICATE ir<%gep> = getelementptr inbounds ir<%ptr>, vp<[[STEPS]]>
; CHECK-NEXT: REPLICATE store ir<%v3>, ir<%gep>
; CHECK-NEXT: EMIT vp<[[CAN_IV_NEXT]]> = add nuw vp<[[CAN_IV]]>, vp<[[VFxUF]]>
Expand Down
Loading