Skip to content

[VPlan] Compute cost of replicating calls in VPlan. (NFCI) #154291

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
fhahn merged 5 commits into from
Aug 26, 2025
Merged

[VPlan] Compute cost of replicating calls in VPlan. (NFCI) #154291

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
93dfdaf
[VPlan] Compute cost of replicating calls in VPlan. (NFCI)
fhahn Aug 18, 2025
d71ccb2
Merge remote-tracking branch 'origin/main' into vplan-compute-scalar-…
fhahn Aug 26, 2025
5fdb4e5
!fixup use clear instead of new smallvector.
fhahn Aug 26, 2025
69f32c9
Merge remote-tracking branch 'origin/main' into vplan-compute-scalar-…
fhahn Aug 26, 2025
ddc132b
!fixup use to_vector to fix stack-use-after-free.
fhahn Aug 26, 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
44 changes: 36 additions & 8 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3002,13 +3002,6 @@ InstructionCost VPReplicateRecipe::computeCost(ElementCount VF,
// instruction cost.
return 0;
case Instruction::Call: {
if (!isSingleScalar()) {
// TODO: Handle remaining call costs here as well.
if (VF.isScalable())
return InstructionCost::getInvalid();
break;
}

auto *CalledFn =
cast<Function>(getOperand(getNumOperands() - 1)->getLiveInIRValue());
if (CalledFn->isIntrinsic())
Expand All @@ -3017,8 +3010,43 @@ InstructionCost VPReplicateRecipe::computeCost(ElementCount VF,
SmallVector<Type *, 4> Tys;
for (VPValue *ArgOp : drop_end(operands()))
Tys.push_back(Ctx.Types.inferScalarType(ArgOp));

Copy link
Contributor

@david-arm david-arm Aug 21, 2025

Choose a reason for hiding this comment

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

nit: unrelated whitespace

Copy link
Contributor Author

@fhahn fhahn Aug 26, 2025

Choose a reason for hiding this comment

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

Removed thanks

Type *ResultTy = Ctx.Types.inferScalarType(this);
return Ctx.TTI.getCallInstrCost(CalledFn, ResultTy, Tys, Ctx.CostKind);
InstructionCost ScalarCallCost =
Ctx.TTI.getCallInstrCost(CalledFn, ResultTy, Tys, Ctx.CostKind);
Copy link
Contributor

@david-arm david-arm Aug 21, 2025

Choose a reason for hiding this comment

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

I wonder if the old code was even correct? It looks like in the legacy cost model (LoopVectorizationCostModel::getVectorCallCost) we selected the min of getVectorIntrinsicCost and TTI.getCallInstrCost if we're invoking an intrinsic.

Copy link
Contributor Author

@fhahn fhahn Aug 21, 2025

Choose a reason for hiding this comment

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

The current code does not handle intrinsics for now (see bail-out at line 3008).

The twist with intrinsics is that if we chose the intrinsic cost, we only create VPReplicateRecipe for various pseudo-intrinsics. I'll add support for intrinsics as follow-up

Copy link
Contributor

@david-arm david-arm Aug 21, 2025

Choose a reason for hiding this comment

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

Ah yes, sorry I missed that! OK great and thanks for doing this.

if (isSingleScalar())
return ScalarCallCost;

if (VF.isScalable())
return InstructionCost::getInvalid();

// Compute the cost of scalarizing the result and operands if needed.
InstructionCost ScalarizationCost = 0;
if (VF.isVector()) {
if (!ResultTy->isVoidTy()) {
for (Type *VectorTy : getContainedTypes(toVectorizedTy(ResultTy, VF))) {
ScalarizationCost += Ctx.TTI.getScalarizationOverhead(
cast<VectorType>(VectorTy), APInt::getAllOnes(VF.getFixedValue()),
/*Insert=*/true,
/*Extract=*/false, Ctx.CostKind);
}
}
// Skip operands that do not require extraction/scalarization and do not
// incur any overhead.
SmallVector<Type *> Tys;
Copy link
Contributor

@david-arm david-arm Aug 21, 2025

Choose a reason for hiding this comment

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

nit: Should this be SmallVector<Type *, 4> to be consistent with UniqueOperands?

Copy link
Collaborator

@huntergr-arm huntergr-arm Aug 21, 2025

Choose a reason for hiding this comment

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

Maybe the existing Tys SmallVector could be reused, with a clear() call? Or do you think you'll need the scalar types again?

Copy link
Contributor Author

@fhahn fhahn Aug 26, 2025

Choose a reason for hiding this comment

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

Used clear for now, thanks!

SmallPtrSet<const VPValue *, 4> UniqueOperands;
for (auto *Op : drop_end(operands())) {
if (Op->isLiveIn() || isa<VPReplicateRecipe, VPPredInstPHIRecipe>(Op) ||
!UniqueOperands.insert(Op).second)
continue;
Tys.push_back(toVectorizedTy(Ctx.Types.inferScalarType(Op), VF));
}
ScalarizationCost +=
Ctx.TTI.getOperandsScalarizationOverhead(Tys, Ctx.CostKind);
}

return ScalarCallCost * (isSingleScalar() ? 1 : VF.getFixedValue()) +
ScalarizationCost;
Comment on lines +3093 to +3094
Copy link
Collaborator

@SamTebbs33 SamTebbs33 Aug 19, 2025

Choose a reason for hiding this comment

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

I could be mis-reading the indentation here, but if it was single scalar, wouldn't this case have exited early at line 2992?

Copy link
Contributor Author

@fhahn fhahn Aug 20, 2025

Choose a reason for hiding this comment

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

yep, removed, thanks

}
case Instruction::Add:
case Instruction::Sub:
Expand Down
Loading