[TTI] Consistently pass the pointer type to getAddressComputationCost. NFCI #152657
+36
−30
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
…. NFCI In some places we were passing the type of value being loaded, in other cases we were passing the type of the pointer for the access. The most "involved" user is LoopVectorizationCostModel::getMemInstScalarizationCost, which is the only call site that passes in the SCEV, and it passes along the pointer. This changes call sites to consistently pass the pointer type, and renames the arguments to clarify this. As far as I can see no target actually checks the contents of the type passed, only to see if it's a vector or not, so this shouldn't have an effect.
llvmbot
added
backend:ARM
backend:AArch64
backend:Hexagon
backend:X86
vectorizers
llvm:analysis
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-backend-arm Author: Luke Lau (lukel97) ChangesIn some places we were passing the type of value being accessed, in other cases we were passing the type of the pointer for the access. The most "involved" user is LoopVectorizationCostModel::getMemInstScalarizationCost, which is the only call site that passes in the SCEV, and it passes along the pointer type. This changes call sites to consistently pass the pointer type, and renames the arguments to clarify this. No target actually checks the contents of the type passed, only to see if it's a vector or not, so this shouldn't have an effect. Full diff: https://github.com/llvm/llvm-project/pull/152657.diff 14 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h b/llvm/include/llvm/Analysis/TargetTransformInfo.h
index aa4550de455e0..091797ae3746b 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfo.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h
@@ -1675,13 +1675,14 @@ class TargetTransformInfo {
/// \returns The cost of the address computation. For most targets this can be
/// merged into the instruction indexing mode. Some targets might want to
- /// distinguish between address computation for memory operations on vector
- /// types and scalar types. Such targets should override this function.
- /// The 'SE' parameter holds pointer for the scalar evolution object which
- /// is used in order to get the Ptr step value in case of constant stride.
- /// The 'Ptr' parameter holds SCEV of the access pointer.
- LLVM_ABI InstructionCost getAddressComputationCost(
- Type *Ty, ScalarEvolution *SE = nullptr, const SCEV *Ptr = nullptr) const;
+ /// distinguish between address computation for memory operations with vector
+ /// types and scalar pointer types. Such targets should override this
+ /// function. The 'SE' parameter holds pointer for the scalar evolution object
+ /// which is used in order to get the Ptr step value in case of constant
+ /// stride. The 'Ptr' parameter holds SCEV of the access pointer.
+ LLVM_ABI InstructionCost
+ getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE = nullptr,
+ const SCEV *Ptr = nullptr) const;
/// \returns The cost, if any, of keeping values of the given types alive
/// over a callsite.
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
index abdbca04488db..7330f5173e490 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -933,7 +933,8 @@ class TargetTransformInfoImplBase {
// Assume that we have a register of the right size for the type.
virtual unsigned getNumberOfParts(Type *Tp) const { return 1; }
- virtual InstructionCost getAddressComputationCost(Type *Tp, ScalarEvolution *,
+ virtual InstructionCost getAddressComputationCost(Type *PtrTy,
+ ScalarEvolution *,
const SCEV *) const {
return 0;
}
diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
index 1fcedcd35ee8b..3f39830f05560 100644
--- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h
+++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
@@ -2977,7 +2977,7 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
return LT.first.getValue();
}
- InstructionCost getAddressComputationCost(Type *Ty, ScalarEvolution *,
+ InstructionCost getAddressComputationCost(Type *PtrTy, ScalarEvolution *,
const SCEV *) const override {
return 0;
}
diff --git a/llvm/lib/Analysis/TargetTransformInfo.cpp b/llvm/lib/Analysis/TargetTransformInfo.cpp
index c7eb2ec18c679..4f04209cf4cfc 100644
--- a/llvm/lib/Analysis/TargetTransformInfo.cpp
+++ b/llvm/lib/Analysis/TargetTransformInfo.cpp
@@ -1231,9 +1231,9 @@ unsigned TargetTransformInfo::getNumberOfParts(Type *Tp) const {
}
InstructionCost
-TargetTransformInfo::getAddressComputationCost(Type *Tp, ScalarEvolution *SE,
+TargetTransformInfo::getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE,
const SCEV *Ptr) const {
- InstructionCost Cost = TTIImpl->getAddressComputationCost(Tp, SE, Ptr);
+ InstructionCost Cost = TTIImpl->getAddressComputationCost(PtrTy, SE, Ptr);
assert(Cost >= 0 && "TTI should not produce negative costs!");
return Cost;
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index 9f05add8bc1c1..40d434d5bbda5 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -4293,7 +4293,7 @@ InstructionCost AArch64TTIImpl::getArithmeticInstrCost(
}
InstructionCost
-AArch64TTIImpl::getAddressComputationCost(Type *Ty, ScalarEvolution *SE,
+AArch64TTIImpl::getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE,
const SCEV *Ptr) const {
// Address computations in vectorized code with non-consecutive addresses will
// likely result in more instructions compared to scalar code where the
@@ -4302,7 +4302,7 @@ AArch64TTIImpl::getAddressComputationCost(Type *Ty, ScalarEvolution *SE,
unsigned NumVectorInstToHideOverhead = NeonNonConstStrideOverhead;
int MaxMergeDistance = 64;
- if (Ty->isVectorTy() && SE &&
+ if (PtrTy->isVectorTy() && SE &&
!BaseT::isConstantStridedAccessLessThan(SE, Ptr, MaxMergeDistance + 1))
return NumVectorInstToHideOverhead;
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
index 7f45177437237..f95133687687a 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
@@ -238,7 +238,7 @@ class AArch64TTIImpl final : public BasicTTIImplBase<AArch64TTIImpl> {
ArrayRef<const Value *> Args = {},
const Instruction *CxtI = nullptr) const override;
- InstructionCost getAddressComputationCost(Type *Ty, ScalarEvolution *SE,
+ InstructionCost getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE,
const SCEV *Ptr) const override;
InstructionCost getCmpSelInstrCost(
diff --git a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
index 6f37eca2b00a5..393cf2d973803 100644
--- a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
@@ -1084,7 +1084,7 @@ InstructionCost ARMTTIImpl::getCmpSelInstrCost(
CostKind, Op1Info, Op2Info, I);
}
-InstructionCost ARMTTIImpl::getAddressComputationCost(Type *Ty,
+InstructionCost ARMTTIImpl::getAddressComputationCost(Type *PtrTy,
ScalarEvolution *SE,
const SCEV *Ptr) const {
// Address computations in vectorized code with non-consecutive addresses will
@@ -1095,7 +1095,7 @@ InstructionCost ARMTTIImpl::getAddressComputationCost(Type *Ty,
int MaxMergeDistance = 64;
if (ST->hasNEON()) {
- if (Ty->isVectorTy() && SE &&
+ if (PtrTy->isVectorTy() && SE &&
!BaseT::isConstantStridedAccessLessThan(SE, Ptr, MaxMergeDistance + 1))
return NumVectorInstToHideOverhead;
@@ -1103,7 +1103,7 @@ InstructionCost ARMTTIImpl::getAddressComputationCost(Type *Ty,
// addressing mode.
return 1;
}
- return BaseT::getAddressComputationCost(Ty, SE, Ptr);
+ return BaseT::getAddressComputationCost(PtrTy, SE, Ptr);
}
bool ARMTTIImpl::isProfitableLSRChainElement(Instruction *I) const {
diff --git a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp
index 9fb7d471fd22a..5c212816fbdb3 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp
@@ -156,7 +156,7 @@ HexagonTTIImpl::getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
return BaseT::getIntrinsicInstrCost(ICA, CostKind);
}
-InstructionCost HexagonTTIImpl::getAddressComputationCost(Type *Tp,
+InstructionCost HexagonTTIImpl::getAddressComputationCost(Type *PtrTy,
ScalarEvolution *SE,
const SCEV *S) const {
return 0;
diff --git a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h
index af8dede723083..0a5766d1dadf5 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h
+++ b/llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h
@@ -111,7 +111,7 @@ class HexagonTTIImpl final : public BasicTTIImplBase<HexagonTTIImpl> {
InstructionCost
getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
TTI::TargetCostKind CostKind) const override;
- InstructionCost getAddressComputationCost(Type *Tp, ScalarEvolution *SE,
+ InstructionCost getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE,
const SCEV *S) const override;
InstructionCost getMemoryOpCost(
unsigned Opcode, Type *Src, Align Alignment, unsigned AddressSpace,
diff --git a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
index 90791fc5788f4..9ef21faea2b60 100644
--- a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
+++ b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
@@ -5488,7 +5488,7 @@ InstructionCost X86TTIImpl::getPointersChainCost(
return BaseT::getPointersChainCost(Ptrs, Base, Info, AccessTy, CostKind);
}
-InstructionCost X86TTIImpl::getAddressComputationCost(Type *Ty,
+InstructionCost X86TTIImpl::getAddressComputationCost(Type *PtrTy,
ScalarEvolution *SE,
const SCEV *Ptr) const {
// Address computations in vectorized code with non-consecutive addresses will
@@ -5504,7 +5504,7 @@ InstructionCost X86TTIImpl::getAddressComputationCost(Type *Ty,
// Even in the case of (loop invariant) stride whose value is not known at
// compile time, the address computation will not incur more than one extra
// ADD instruction.
- if (Ty->isVectorTy() && SE && !ST->hasAVX2()) {
+ if (PtrTy->isVectorTy() && SE && !ST->hasAVX2()) {
// TODO: AVX2 is the current cut-off because we don't have correct
// interleaving costs for prior ISA's.
if (!BaseT::isStridedAccess(Ptr))
@@ -5513,7 +5513,7 @@ InstructionCost X86TTIImpl::getAddressComputationCost(Type *Ty,
return 1;
}
- return BaseT::getAddressComputationCost(Ty, SE, Ptr);
+ return BaseT::getAddressComputationCost(PtrTy, SE, Ptr);
}
InstructionCost
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index d9805d8b3ea59..844219a64d2f4 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -2309,8 +2309,7 @@ chainToBasePointerCost(SmallVectorImpl<Instruction *> &Chain,
} else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Instr)) {
// Cost of the address calculation
- Type *ValTy = GEP->getSourceElementType();
- Cost += TTI.getAddressComputationCost(ValTy);
+ Cost += TTI.getAddressComputationCost(GEP->getType());
// And cost of the GEP itself
// TODO: Use TTI->getGEPCost here (it exists, but appears to be not
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index be00fd6a416e5..d39ddc4aa1f17 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -5344,11 +5344,12 @@ LoopVectorizationCostModel::getUniformMemOpCost(Instruction *I,
assert(Legal->isUniformMemOp(*I, VF));
Type *ValTy = getLoadStoreType(I);
+ Type *PtrTy = getLoadStorePointerOperand(I)->getType();
auto *VectorTy = cast<VectorType>(toVectorTy(ValTy, VF));
const Align Alignment = getLoadStoreAlignment(I);
unsigned AS = getLoadStoreAddressSpace(I);
if (isa<LoadInst>(I)) {
- return TTI.getAddressComputationCost(ValTy) +
+ return TTI.getAddressComputationCost(PtrTy) +
TTI.getMemoryOpCost(Instruction::Load, ValTy, Alignment, AS,
CostKind) +
TTI.getShuffleCost(TargetTransformInfo::SK_Broadcast, VectorTy,
@@ -5361,7 +5362,7 @@ LoopVectorizationCostModel::getUniformMemOpCost(Instruction *I,
// VF.getKnownMinValue() - 1 from a scalable vector. This does not represent
// the actual generated code, which involves extracting the last element of
// a scalable vector where the lane to extract is unknown at compile time.
- return TTI.getAddressComputationCost(ValTy) +
+ return TTI.getAddressComputationCost(PtrTy) +
TTI.getMemoryOpCost(Instruction::Store, ValTy, Alignment, AS,
CostKind) +
(IsLoopInvariantStoreValue
@@ -5375,10 +5376,11 @@ LoopVectorizationCostModel::getGatherScatterCost(Instruction *I,
ElementCount VF) {
Type *ValTy = getLoadStoreType(I);
auto *VectorTy = cast<VectorType>(toVectorTy(ValTy, VF));
+ Type *PtrTy = toVectorTy(getLoadStorePointerOperand(I)->getType(), VF);
const Align Alignment = getLoadStoreAlignment(I);
const Value *Ptr = getLoadStorePointerOperand(I);
- return TTI.getAddressComputationCost(VectorTy) +
+ return TTI.getAddressComputationCost(PtrTy) +
TTI.getGatherScatterOpCost(I->getOpcode(), VectorTy, Ptr,
Legal->isMaskRequired(I), Alignment,
CostKind, I);
@@ -5613,11 +5615,12 @@ LoopVectorizationCostModel::getMemoryInstructionCost(Instruction *I,
// moment.
if (VF.isScalar()) {
Type *ValTy = getLoadStoreType(I);
+ Type *PtrTy = getLoadStorePointerOperand(I)->getType();
const Align Alignment = getLoadStoreAlignment(I);
unsigned AS = getLoadStoreAddressSpace(I);
TTI::OperandValueInfo OpInfo = TTI::getOperandInfo(I->getOperand(0));
- return TTI.getAddressComputationCost(ValTy) +
+ return TTI.getAddressComputationCost(PtrTy) +
TTI.getMemoryOpCost(I->getOpcode(), ValTy, Alignment, AS, CostKind,
OpInfo, I);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index e971ba1aac15c..20013608ac31e 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3097,9 +3097,10 @@ InstructionCost VPWidenMemoryRecipe::computeCost(ElementCount VF,
// Currently, ARM will use the underlying IR to calculate gather/scatter
// instruction cost.
const Value *Ptr = getLoadStorePointerOperand(&Ingredient);
+ Type *PtrTy = toVectorTy(Ptr->getType(), VF);
assert(!Reverse &&
"Inconsecutive memory access should not have the order.");
- return Ctx.TTI.getAddressComputationCost(Ty) +
+ return Ctx.TTI.getAddressComputationCost(PtrTy) +
Ctx.TTI.getGatherScatterOpCost(Opcode, Ty, Ptr, IsMasked, Alignment,
Ctx.CostKind, &Ingredient);
}
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 6345b18b809a6..6ebc2f3917fe3 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -1790,7 +1790,8 @@ bool VectorCombine::scalarizeLoadExtract(Instruction &I) {
ScalarizedCost +=
TTI.getMemoryOpCost(Instruction::Load, VecTy->getElementType(),
Align(1), LI->getPointerAddressSpace(), CostKind);
- ScalarizedCost += TTI.getAddressComputationCost(VecTy->getElementType());
+ ScalarizedCost +=
+ TTI.getAddressComputationCost(LI->getPointerOperandType());
}
LLVM_DEBUG(dbgs() << "Found all extractions of a vector load: " << I
|
RKSimon
reviewed
Aug 8, 2025
| /// types and scalar pointer types. Such targets should override this | ||
| /// function. The 'SE' parameter holds pointer for the scalar evolution object | ||
| /// which is used in order to get the Ptr step value in case of constant | ||
| /// stride. The 'Ptr' parameter holds SCEV of the access pointer. |
There was a problem hiding this comment.
Use doxygen style:
/// function. \p SE holds pointer for the scalar evolution object
/// which is used in order to get the Ptr step value in case of constant
/// stride. \p Ptr holds SCEV of the access pointer.
david-arm
reviewed
Aug 8, 2025
| @@ -5375,10 +5376,11 @@ LoopVectorizationCostModel::getGatherScatterCost(Instruction *I, | |||
| ElementCount VF) { | |||
| Type *ValTy = getLoadStoreType(I); | |||
| auto *VectorTy = cast<VectorType>(toVectorTy(ValTy, VF)); | |||
| Type *PtrTy = toVectorTy(getLoadStorePointerOperand(I)->getType(), VF); | |||
| const Align Alignment = getLoadStoreAlignment(I); | |||
| const Value *Ptr = getLoadStorePointerOperand(I); | |||
There was a problem hiding this comment.
Can't you just reuse the Ptr variable like this:
const Align Alignment = getLoadStoreAlignment(I);
const Value *Ptr = getLoadStorePointerOperand(I);
Type *PtrTy = toVectorTy(getLoadStorePointerOperand(I)->getType(), VF);
?
| /// used in order to get the Ptr step value in case of constant stride. \p Ptr | ||
| /// holds the SCEV of the access pointer. | ||
| LLVM_ABI InstructionCost | ||
| getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE = nullptr, |
There was a problem hiding this comment.
If we're passing in the actual pointer type I thought it can only ever be an opaque pointer type, or a vector of opaque pointer types. So can we make the interface even simpler by just passing a boolean saying whether it's a vector or not?
There was a problem hiding this comment.
I guess the pointer type also contains the address space, not that any target currently uses it though. Long term though I think we probably want to change this API anyway to not even pass the type or SCEV? That way we can use it properly from VPlan which doesn't have access to SCEV. Just from looking at the various TTIs we seem to have three costs:
- Scalar address
- Vector of strided addresses (either constant or variable)
- Vector of gathered/scattered addresses
Maybe we could look at splitting this into three separate hooks?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In some places we were passing the type of value being accessed, in other cases we were passing the type of the pointer for the access.
The most "involved" user is LoopVectorizationCostModel::getMemInstScalarizationCost, which is the only call site that passes in the SCEV, and it passes along the pointer type.
This changes call sites to consistently pass the pointer type, and renames the arguments to clarify this.
No target actually checks the contents of the type passed, only to see if it's a vector or not, so this shouldn't have an effect.