Skip to content

[VPlan] Implement VPReductionRecipe::computeCost(). NFC #107790

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 7 commits into from
Oct 15, 2024
Merged

[VPlan] Implement VPReductionRecipe::computeCost(). NFC #107790

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
324659a
[VPlan] Implment VPReductionRecipe::computeCost(). NFC
ElvisWang123 Sep 5, 2024
91fcf39
Address comments.
ElvisWang123 Sep 25, 2024
4f2cc46
Address comments.
ElvisWang123 Oct 4, 2024
28b82ca
Address comments and add `isInLoopReduction()`.
ElvisWang123 Oct 9, 2024
041f41b
Fixup! Remove legacy model query
ElvisWang123 Oct 9, 2024
7acd294
Fixup! Using cast to avoid warning when compiling without assertion.
ElvisWang123 Oct 14, 2024
80faae5
Fixup! typo
ElvisWang123 Oct 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
4 changes: 4 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2476,6 +2476,10 @@ class VPReductionRecipe : public VPSingleDefRecipe {
/// Generate the reduction in the loop
void execute(VPTransformState &State) override;

/// Return the cost of VPReductionRecipe.
InstructionCost computeCost(ElementCount VF,
VPCostContext &Ctx) const override;

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Print the recipe.
void print(raw_ostream &O, const Twine &Indent,
Expand Down
34 changes: 34 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2071,6 +2071,40 @@ void VPReductionEVLRecipe::execute(VPTransformState &State) {
State.set(this, NewRed, /*IsScalar*/ true);
}

InstructionCost VPReductionRecipe::computeCost(ElementCount VF,
VPCostContext &Ctx) const {
RecurKind RdxKind = RdxDesc.getRecurrenceKind();
Copy link
Contributor

@fhahn fhahn Sep 25, 2024

Choose a reason for hiding this comment

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

This computes the cost for non-in loop and non-any-of reductions, correct? Would be good to add an assert an explanation why (for those the cost needs to be pre-computed at the moment)

Copy link
Contributor Author

@ElvisWang123 ElvisWang123 Oct 4, 2024

Choose a reason for hiding this comment

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

Thanks, I will add an assertion to check it is not an any-of reduction.

I tried to compute the cost of in-loop reductions by the vplan-based cost model.

Copy link
Contributor

@fhahn fhahn Oct 4, 2024

Choose a reason for hiding this comment

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

It would probably be better to add support for in-loop reductions separately, as it adds extra complexity and may introduce new divergences

Copy link
Contributor Author

@ElvisWang123 ElvisWang123 Oct 4, 2024

Choose a reason for hiding this comment

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

Sure, I will open another PR to address in-loop reduction.

Type *ElementTy = Ctx.Types.inferScalarType(this);
auto *VectorTy = cast<VectorType>(ToVectorTy(ElementTy, VF));
Copy link
Contributor

@fhahn fhahn Sep 25, 2024

Choose a reason for hiding this comment

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

infer the type for the recipe instead and assert that the type matches RdxDesc.getRecurrenceType() to ensure consistency?

Copy link
Contributor Author

@ElvisWang123 ElvisWang123 Oct 4, 2024

Choose a reason for hiding this comment

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

Sure, thanks.

TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput;
unsigned Opcode = RdxDesc.getOpcode();

// TODO: Support any-of and in-loop reductions.
assert(
(!RecurrenceDescriptor::isAnyOfRecurrenceKind(RdxKind) ||
ForceTargetInstructionCost.getNumOccurrences() > 0) &&
"Any-of reduction not implemented in VPlan-based cost model currently.");
assert(
(!cast<VPReductionPHIRecipe>(getOperand(0))->isInLoop() ||
ForceTargetInstructionCost.getNumOccurrences() > 0) &&
"In-loop reduction not implemented in VPlan-based cost model currently.");

assert(ElementTy->getTypeID() == RdxDesc.getRecurrenceType()->getTypeID() &&
"Infered type and recurrence type mismatch.");
Copy link
Contributor

@fhahn fhahn Oct 14, 2024

Choose a reason for hiding this comment

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

Suggested change
"Infered type and recurrence type mismatch.");
"Inferred type and recurrence type mismatch.");


// Cost = Reduction cost + BinOp cost
InstructionCost Cost =
Ctx.TTI.getArithmeticInstrCost(Opcode, ElementTy, CostKind);
if (RecurrenceDescriptor::isMinMaxRecurrenceKind(RdxKind)) {
Intrinsic::ID Id = getMinMaxReductionIntrinsicOp(RdxKind);
return Cost + Ctx.TTI.getMinMaxReductionCost(
Id, VectorTy, RdxDesc.getFastMathFlags(), CostKind);
}

return Cost + Ctx.TTI.getArithmeticReductionCost(
Opcode, VectorTy, RdxDesc.getFastMathFlags(), CostKind);
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void VPReductionRecipe::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
Expand Down
Loading