Skip to content

[VPlan] Replace RdxDesc with RecurKind in VPReductionPHIRecipe (NFC). #142322

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 8 commits into from
Jul 6, 2025
Merged

[VPlan] Replace RdxDesc with RecurKind in VPReductionPHIRecipe (NFC). #142322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bc7024c
[VPlan] Replace RdxDesc with RecurKind in VPReductionPHIRecipe (NFC).
fhahn May 30, 2025
5d0c744
!fixup use static cast, add helper.
fhahn Jul 2, 2025
dca0d4b
!fixup unify constructors, use helper
fhahn Jul 3, 2025
4c2e494
Merge remote-tracking branch 'origin/main' into vplan-replace-rdxdesc…
fhahn Jul 6, 2025
0ab71f6
Merge remote-tracking branch 'origin/main' into vplan-replace-rdxdesc…
fhahn Jul 6, 2025
cb4b75d
!fixup address comments, thanks
fhahn Jul 6, 2025
db65c6f
Merge remote-tracking branch 'origin/main' into vplan-replace-rdxdesc…
fhahn Jul 6, 2025
0bd4d2a
!fixup remove unrelated change
fhahn Jul 6, 2025
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
54 changes: 21 additions & 33 deletions llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7266,8 +7266,7 @@ static void fixReductionScalarResumeWhenVectorizingEpilog(

auto *EpiRedHeaderPhi =
cast<VPReductionPHIRecipe>(EpiRedResult->getOperand(0));
const RecurrenceDescriptor &RdxDesc =
EpiRedHeaderPhi->getRecurrenceDescriptor();
RecurKind Kind = EpiRedHeaderPhi->getRecurrenceKind();
Value *MainResumeValue;
if (auto *VPI = dyn_cast<VPInstruction>(EpiRedHeaderPhi->getStartValue())) {
assert((VPI->getOpcode() == VPInstruction::Broadcast ||
Expand All @@ -7276,8 +7275,7 @@ static void fixReductionScalarResumeWhenVectorizingEpilog(
MainResumeValue = VPI->getOperand(0)->getUnderlyingValue();
} else
MainResumeValue = EpiRedHeaderPhi->getStartValue()->getUnderlyingValue();
if (RecurrenceDescriptor::isAnyOfRecurrenceKind(
RdxDesc.getRecurrenceKind())) {
if (RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Independent: can shorten to isAnyOf(), isFindIV(), etc., given that the parameter is a Recur[rence]Kind.

[[maybe_unused]] Value *StartV =
EpiRedResult->getOperand(1)->getLiveInIRValue();
auto *Cmp = cast<ICmpInst>(MainResumeValue);
Expand All @@ -7287,8 +7285,7 @@ static void fixReductionScalarResumeWhenVectorizingEpilog(
"AnyOf expected to start by comparing main resume value to original "
"start value");
MainResumeValue = Cmp->getOperand(0);
} else if (RecurrenceDescriptor::isFindIVRecurrenceKind(
RdxDesc.getRecurrenceKind())) {
} else if (RecurrenceDescriptor::isFindIVRecurrenceKind(Kind)) {
Value *StartV = getStartValueFromReductionResult(EpiRedResult);
Value *SentinelV = EpiRedResult->getOperand(2)->getLiveInIRValue();
using namespace llvm::PatternMatch;
Expand Down Expand Up @@ -8308,7 +8305,7 @@ VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(VPSingleDefRecipe *R,
unsigned ScaleFactor =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Above

      const RecurrenceDescriptor &RdxDesc =
          Legal->getReductionVars().find(Phi)->second;

can be replaced (independently) with

      const RecurrenceDescriptor &RdxDesc =
          Legal->getRecurrenceDescriptor(Phi);

?

Similar candidates in
LoopVectorizationCostModel::collectElementTypesForWidening(), LoopVectorizationCostModel::getReductionPatternCost().

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 in c5fff13, thanks

getScalingForReduction(RdxDesc.getLoopExitInstr()).value_or(1);
PhiRecipe = new VPReductionPHIRecipe(
Phi, RdxDesc, *StartV, CM.isInLoopReduction(Phi),
Phi, RdxDesc.getRecurrenceKind(), *StartV, CM.isInLoopReduction(Phi),
CM.useOrderedReductions(RdxDesc), ScaleFactor);
} else {
// TODO: Currently fixed-order recurrences are modeled as chains of
Expand Down Expand Up @@ -9068,8 +9065,7 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
if (!PhiR || !PhiR->isInLoop() || (MinVF.isScalar() && !PhiR->isOrdered()))
continue;

const RecurrenceDescriptor &RdxDesc = PhiR->getRecurrenceDescriptor();
RecurKind Kind = RdxDesc.getRecurrenceKind();
RecurKind Kind = PhiR->getRecurrenceKind();
assert(
!RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind) &&
!RecurrenceDescriptor::isFindIVRecurrenceKind(Kind) &&
Expand Down Expand Up @@ -9175,6 +9171,9 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
if (CM.blockNeedsPredicationForAnyReason(CurrentLinkI->getParent()))
CondOp = RecipeBuilder.getBlockInMask(CurrentLink->getParent());

// TODO: Retrieve FMFs from recipes directly.
RecurrenceDescriptor RdxDesc = Legal->getRecurrenceDescriptor(
cast<PHINode>(PhiR->getUnderlyingInstr()));
Comment on lines +9175 to +9176
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO to also capture RdxDesc's FMF in PhiR, similar to capturing its Kind, to avoid retrieving RdxDesc here?
Can CM.useOrderedReductions(RdxDesc) below be replaced with PhiR->isOrdered()?

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 TODO, thanks

// Non-FP RdxDescs will have all fast math flags set, so clear them.
FastMathFlags FMFs = isa<FPMathOperator>(CurrentLinkI)
? RdxDesc.getFastMathFlags()
Expand Down Expand Up @@ -9205,7 +9204,8 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
if (!PhiR)
continue;

const RecurrenceDescriptor &RdxDesc = PhiR->getRecurrenceDescriptor();
const RecurrenceDescriptor &RdxDesc = Legal->getRecurrenceDescriptor(
cast<PHINode>(PhiR->getUnderlyingInstr()));
Comment on lines +9207 to +9208
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
const RecurrenceDescriptor &RdxDesc = Legal->getRecurrenceDescriptor(
cast<PHINode>(PhiR->getUnderlyingInstr()));
RecurKind RecurrenceKind = PhiR->getRecurrenceKind();
const RecurrenceDescriptor &RdxDesc = Legal->getRecurrenceDescriptor(
cast<PHINode>(PhiR->getUnderlyingInstr()));

and replace RdxDesc.getRecurrenceKind() below with RecurrenceKind?

Getting rid of RdxDesc here is more difficult: in addition to capturing RdxDesc's FMF in PhiR, code below also uses RdxDesc to retrieve sentinel value, recurrence type, isSigned, Loop Exit Instr, ... ?

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 to use Kind, thanks!

Type *PhiTy = PhiR->getUnderlyingValue()->getType();
// If tail is folded by masking, introduce selects between the phi
// and the users outside the vector region of each reduction, at the
Expand Down Expand Up @@ -9253,24 +9253,23 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
VPInstruction *FinalReductionResult;
VPBuilder::InsertPointGuard Guard(Builder);
Builder.setInsertPoint(MiddleVPBB, IP);
if (RecurrenceDescriptor::isFindIVRecurrenceKind(
RdxDesc.getRecurrenceKind())) {
RecurKind RecurrenceKind = PhiR->getRecurrenceKind();
if (RecurrenceDescriptor::isFindIVRecurrenceKind(RecurrenceKind)) {
VPValue *Start = PhiR->getStartValue();
VPValue *Sentinel = Plan->getOrAddLiveIn(RdxDesc.getSentinelValue());
FinalReductionResult =
Builder.createNaryOp(VPInstruction::ComputeFindIVResult,
{PhiR, Start, Sentinel, NewExitingVPV}, ExitDL);
} else if (RecurrenceDescriptor::isAnyOfRecurrenceKind(
RdxDesc.getRecurrenceKind())) {
} else if (RecurrenceDescriptor::isAnyOfRecurrenceKind(RecurrenceKind)) {
VPValue *Start = PhiR->getStartValue();
FinalReductionResult =
Builder.createNaryOp(VPInstruction::ComputeAnyOfResult,
{PhiR, Start, NewExitingVPV}, ExitDL);
} else {
VPIRFlags Flags = RecurrenceDescriptor::isFloatingPointRecurrenceKind(
RdxDesc.getRecurrenceKind())
? VPIRFlags(RdxDesc.getFastMathFlags())
: VPIRFlags();
VPIRFlags Flags =
RecurrenceDescriptor::isFloatingPointRecurrenceKind(RecurrenceKind)
? VPIRFlags(RdxDesc.getFastMathFlags())
: VPIRFlags();
FinalReductionResult =
Builder.createNaryOp(VPInstruction::ComputeReductionResult,
{PhiR, NewExitingVPV}, Flags, ExitDL);
Expand All @@ -9279,11 +9278,9 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
// then extend the loop exit value to enable InstCombine to evaluate the
// entire expression in the smaller type.
if (MinVF.isVector() && PhiTy != RdxDesc.getRecurrenceType() &&
!RecurrenceDescriptor::isAnyOfRecurrenceKind(
RdxDesc.getRecurrenceKind())) {
!RecurrenceDescriptor::isAnyOfRecurrenceKind(RecurrenceKind)) {
assert(!PhiR->isInLoop() && "Unexpected truncated inloop reduction!");
assert(!RecurrenceDescriptor::isMinMaxRecurrenceKind(
RdxDesc.getRecurrenceKind()) &&
assert(!RecurrenceDescriptor::isMinMaxRecurrenceKind(RecurrenceKind) &&
"Unexpected truncated min-max recurrence!");
Type *RdxTy = RdxDesc.getRecurrenceType();
auto *Trunc =
Expand Down Expand Up @@ -9319,8 +9316,7 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
// with a boolean reduction phi node to check if the condition is true in
// any iteration. The final value is selected by the final
// ComputeReductionResult.
if (RecurrenceDescriptor::isAnyOfRecurrenceKind(
RdxDesc.getRecurrenceKind())) {
if (RecurrenceDescriptor::isAnyOfRecurrenceKind(RecurrenceKind)) {
auto *Select = cast<VPRecipeBase>(*find_if(PhiR->users(), [](VPUser *U) {
return isa<VPWidenSelectRecipe>(U) ||
(isa<VPReplicateRecipe>(U) &&
Expand Down Expand Up @@ -9851,14 +9847,9 @@ preparePlanForEpilogueVectorLoop(VPlan &Plan, Loop *L,
}));
ResumeV = cast<PHINode>(ReductionPhi->getUnderlyingInstr())
->getIncomingValueForBlock(L->getLoopPreheader());
const RecurrenceDescriptor &RdxDesc =
ReductionPhi->getRecurrenceDescriptor();
RecurKind RK = RdxDesc.getRecurrenceKind();
RecurKind RK = ReductionPhi->getRecurrenceKind();
if (RecurrenceDescriptor::isAnyOfRecurrenceKind(RK)) {
Value *StartV = RdxResult->getOperand(1)->getLiveInIRValue();
assert(RdxDesc.getRecurrenceStartValue() == StartV &&
"start value from ComputeAnyOfResult must match");

// VPReductionPHIRecipes for AnyOf reductions expect a boolean as
// start value; compare the final value from the main vector loop
// to the start value.
Expand All @@ -9867,9 +9858,6 @@ preparePlanForEpilogueVectorLoop(VPlan &Plan, Loop *L,
ResumeV = Builder.CreateICmpNE(ResumeV, StartV);
} else if (RecurrenceDescriptor::isFindIVRecurrenceKind(RK)) {
Value *StartV = getStartValueFromReductionResult(RdxResult);
assert(RdxDesc.getRecurrenceStartValue() == StartV &&
"start value from ComputeFinIVResult must match");

ToFrozen[StartV] = cast<PHINode>(ResumeV)->getIncomingValueForBlock(
EPI.MainLoopIterationCountCheck);

Expand Down
25 changes: 11 additions & 14 deletions llvm/lib/Transforms/Vectorize/VPlan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2191,8 +2191,8 @@ struct VPFirstOrderRecurrencePHIRecipe : public VPHeaderPHIRecipe {
/// operand.
class VPReductionPHIRecipe : public VPHeaderPHIRecipe,
public VPUnrollPartAccessor<2> {
/// Descriptor for the reduction.
const RecurrenceDescriptor &RdxDesc;
/// The recurrence kind of the reduction.
const RecurKind Kind;

/// The phi is part of an in-loop reduction.
bool IsInLoop;
Expand All @@ -2205,22 +2205,20 @@ class VPReductionPHIRecipe : public VPHeaderPHIRecipe,
unsigned VFScaleFactor = 1;

public:
/// Create a new VPReductionPHIRecipe for the reduction \p Phi described by \p
/// RdxDesc.
VPReductionPHIRecipe(PHINode *Phi, const RecurrenceDescriptor &RdxDesc,
VPValue &Start, bool IsInLoop = false,
bool IsOrdered = false, unsigned VFScaleFactor = 1)
: VPHeaderPHIRecipe(VPDef::VPReductionPHISC, Phi, &Start),
RdxDesc(RdxDesc), IsInLoop(IsInLoop), IsOrdered(IsOrdered),
VFScaleFactor(VFScaleFactor) {
/// Create a new VPReductionPHIRecipe for the reduction \p Phi.
VPReductionPHIRecipe(PHINode *Phi, RecurKind Kind, VPValue &Start,
bool IsInLoop = false, bool IsOrdered = false,
unsigned VFScaleFactor = 1)
: VPHeaderPHIRecipe(VPDef::VPReductionPHISC, Phi, &Start), Kind(Kind),
IsInLoop(IsInLoop), IsOrdered(IsOrdered), VFScaleFactor(VFScaleFactor) {
assert((!IsOrdered || IsInLoop) && "IsOrdered requires IsInLoop");
}

~VPReductionPHIRecipe() override = default;

VPReductionPHIRecipe *clone() override {
auto *R = new VPReductionPHIRecipe(
dyn_cast_or_null<PHINode>(getUnderlyingValue()), RdxDesc,
dyn_cast_or_null<PHINode>(getUnderlyingValue()), getRecurrenceKind(),
*getOperand(0), IsInLoop, IsOrdered, VFScaleFactor);
R->addOperand(getBackedgeValue());
return R;
Expand All @@ -2240,9 +2238,8 @@ class VPReductionPHIRecipe : public VPHeaderPHIRecipe,
VPSlotTracker &SlotTracker) const override;
#endif

const RecurrenceDescriptor &getRecurrenceDescriptor() const {
return RdxDesc;
}
/// Returns the recurrence kind of the reduction.
RecurKind getRecurrenceKind() const { return Kind; }
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
RecurKind getRecurrenceKind() const { return Kind; }
/// Returns the recurrence kind of the reduction.
RecurKind getRecurrenceKind() const { return Kind; }

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


/// Returns true, if the phi is part of an ordered reduction.
bool isOrdered() const { return IsOrdered; }
Expand Down
12 changes: 5 additions & 7 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,8 +751,7 @@ Value *VPInstruction::generate(VPTransformState &State) {
// and will be removed by breaking up the recipe further.
auto *PhiR = cast<VPReductionPHIRecipe>(getOperand(0));
// Get its reduction variable descriptor.
const RecurrenceDescriptor &RdxDesc = PhiR->getRecurrenceDescriptor();
RecurKind RK = RdxDesc.getRecurrenceKind();
RecurKind RK = PhiR->getRecurrenceKind();
assert(RecurrenceDescriptor::isFindIVRecurrenceKind(RK) &&
"Unexpected reduction kind");
assert(!PhiR->isInLoop() &&
Expand Down Expand Up @@ -786,9 +785,8 @@ Value *VPInstruction::generate(VPTransformState &State) {
// and will be removed by breaking up the recipe further.
auto *PhiR = cast<VPReductionPHIRecipe>(getOperand(0));
// Get its reduction variable descriptor.
const RecurrenceDescriptor &RdxDesc = PhiR->getRecurrenceDescriptor();

RecurKind RK = RdxDesc.getRecurrenceKind();
RecurKind RK = PhiR->getRecurrenceKind();
assert(!RecurrenceDescriptor::isFindIVRecurrenceKind(RK) &&
"should be handled by ComputeFindIVResult");

Expand All @@ -814,9 +812,9 @@ Value *VPInstruction::generate(VPTransformState &State) {
if (RecurrenceDescriptor::isMinMaxRecurrenceKind(RK))
ReducedPartRdx = createMinMaxOp(Builder, RK, ReducedPartRdx, RdxPart);
else
ReducedPartRdx =
Builder.CreateBinOp((Instruction::BinaryOps)RdxDesc.getOpcode(),
RdxPart, ReducedPartRdx, "bin.rdx");
ReducedPartRdx = Builder.CreateBinOp(
(Instruction::BinaryOps)RecurrenceDescriptor::getOpcode(RK),
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this C-style cast to a static_cast?

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 thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1, can be done independently, keeping this patch focused on the RdxDesc.getOpcode() --> getOpcode(RK) transition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do separately, thanks

RdxPart, ReducedPartRdx, "bin.rdx");
}
}

Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1735,8 +1735,7 @@ void VPlanTransforms::clearReductionWrapFlags(VPlan &Plan) {
auto *PhiR = dyn_cast<VPReductionPHIRecipe>(&R);
if (!PhiR)
continue;
const RecurrenceDescriptor &RdxDesc = PhiR->getRecurrenceDescriptor();
RecurKind RK = RdxDesc.getRecurrenceKind();
RecurKind RK = PhiR->getRecurrenceKind();
if (RK != RecurKind::Add && RK != RecurKind::Mul)
continue;

Expand Down
Loading