[LV] Add scalar load/stores to VPReplicateRecipe::computeCost #153218
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: David Sherwood (david-arm) ChangesAvoid calling getLegacyCost for single scalar loads and stores where the cost is trivial to calculate. Full diff: https://github.com/llvm/llvm-project/pull/153218.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index e34cab117f321..b301aee6dad23 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -2975,6 +2975,22 @@ InstructionCost VPReplicateRecipe::computeCost(ElementCount VF,
Op2Info, Operands, UI, &Ctx.TLI) *
(isSingleScalar() ? 1 : VF.getFixedValue());
}
+ case Instruction::Load:
+ case Instruction::Store: {
+ if (isSingleScalar()) {
+ Type *ValTy = getLoadStoreType(UI);
+ Type *ScalarPtrTy = getLoadStorePointerOperand(UI)->getType();
+ const Align Alignment = getLoadStoreAlignment(UI);
+ unsigned AS = getLoadStoreAddressSpace(UI);
+ TTI::OperandValueInfo OpInfo = TTI::getOperandInfo(UI->getOperand(0));
+ InstructionCost ScalarMemOpCost = Ctx.TTI.getMemoryOpCost(
+ UI->getOpcode(), ValTy, Alignment, AS, CostKind, OpInfo, UI);
+ return ScalarMemOpCost + Ctx.TTI.getAddressComputationCost(ScalarPtrTy);
+ }
+ // TODO: See getMemInstScalarizationCost for how to handle vector and
+ // predicated cases.
+ break;
+ }
}
return Ctx.getLegacyCost(UI, VF);
|
Contributor
Author
|
Gentle ping. :) |
fhahn
reviewed
Aug 26, 2025
Contributor
There was a problem hiding this comment.
Suggested change
| // TODO: See getMemInstScalarizationCost for how to handle vector and | |
| // TODO: See getMemInstScalarizationCost for how to handle replicating and |
This code here won't handle vector cases?
Comment on lines
2981
to
2982
Contributor
There was a problem hiding this comment.
Can we get this from the VPlan type analysis?
david-arm
force-pushed
the
vp_rep_costs
branch
from
a4e0e92 to
fd7bf4b
Compare
fhahn
reviewed
Aug 28, 2025
Comment on lines
5208
to
5209
Contributor
There was a problem hiding this comment.
It is indeed confusing and the argument isn't documented, but it looks like in RISCV TTI and X86 TTI (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86TargetTransformInfo.cpp#L5241) the argument is used as it is passed here: stored value for stores, address for loads. AArch64 and ARM TTIs don't use it.
So it seems it is used as expected, even though the documentation of the interface could be improved
Contributor
There was a problem hiding this comment.
Just checking if you saw this comment? Given the above, the comment should be removed here and the documentation for the TTI functions improved/clarified
Contributor
Author
There was a problem hiding this comment.
Yeah, sorry just got caught up with other things. I guess I wasn't really sure whether you wanted me to change the documentation in this PR or were just pointing it out for information purpose?
Contributor
There was a problem hiding this comment.
Probably best to remove the TOOD here and clarify the TTI separately?
Avoid calling getLegacyCost for single scalar loads and stores where the cost is trivial to calculate.
david-arm
force-pushed
the
vp_rep_costs
branch
from
fd7bf4b to
adf9af9
Compare
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/12161 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Avoid calling getLegacyCost for single scalar loads and stores where the cost is trivial to calculate.