Skip to content

[TTI][RISCV] Add cost modelling for intrinsic vp.load.ff #160470

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
Sep 26, 2025
Merged

[TTI][RISCV] Add cost modelling for intrinsic vp.load.ff #160470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0a3596d
[TTI] Add cost modelling for intrinsic vp.load.ff
arcbbb Sep 24, 2025
7ec6084
Replace undef with live-in ptr
arcbbb Sep 24, 2025
3cdef8f
Remove uneeded assertion
arcbbb Sep 25, 2025
b1cc6fb
Rename getFaultOnlyFirstLoadCost getFaultFirstLoadCost
arcbbb Sep 25, 2025
4d0a568
remove tests for regular loads
arcbbb Sep 25, 2025
79e0c3d
Rename isLegalFaultOnlyFirstLoad to isLegalFaultFirstLoad
arcbbb Sep 25, 2025
75b4f56
Rename FaultFirstLoad to FirstFaultLoad
arcbbb Sep 25, 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
5 changes: 5 additions & 0 deletions llvm/include/llvm/Analysis/TargetTransformInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,11 @@ class TargetTransformInfo {
TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput,
bool UseMaskForCond = false, bool UseMaskForGaps = false) const;

/// \return The cost of vp intrinsic vp.load.ff.
LLVM_ABI InstructionCost getFirstFaultLoadCost(
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the benefit of having a separate hook instead of just requesting the cost of the intrinsic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We pass info to getIntrinsicInstrCost via IntrinsicCostAttributes, but attributes like alignment aren’t supported there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In VPlan, we request the cost before emitting vp.load.ff; hence no IntrinsicInst is available for IntrinsicCostAttributes to derive alignment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm that's unfortunate. Would be good to be able to pass all required info to getIntrinsicCost.

But there's also already getVPMemoryOpCost, could it go there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that having a separate hook for every type of memory intrinsic is a lot of boilerplate. But I guess this is also more consistent with how we cost e.g. vp.strided.load/masked.load etc.

Copy link
Contributor

@lukel97 lukel97 Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

TIL that getVPMemoryOpCost actually exists. We don't use it in RISC-V, so it looks like it's dead. I've opened up a PR to remove it: #160838

Given that we're moving away from (non-trivial) VP intrinsics in general, if SVE ever wants to support first-fault loads I presume we'll want to have a non-VP hook anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL that getVPMemoryOpCost actually exists. We don't use it in RISC-V, so it looks like it's dead. I've opened up a PR to remove it: #160838

This is exactly why I would argue we should try to adress the underlying issue, i.e. make getIntrinsicInstrCost powerful enough to serve the use cases, so we don't have to add various very specialized hooks when they are for intrinsics.

@arcbbb would probably have been good to wait a bit with merging while there was still an ongoing discussion

Copy link
Contributor

Choose a reason for hiding this comment

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

i.e. make getIntrinsicInstrCost powerful enough to serve the use cases, so we don't have to add various very specialized hooks when they are for intrinsics.

I think that's reasonable. We should probably also consolidate all the different memory costing hooks that can go into getIntrinsicInstrCost? E.g.

  • getMaskedMemoryOpCost -> llvm.masked.load/store
  • getGatherScatterOpCost -> llvm.masked.gather/scatter
  • getExpandCompressMemoryOpCost -> llvm.masked.expandload/llvm.masked.compressstore
  • getStridedMemoryOpCost -> llvm.experimental.vp.strided.load/store

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fhahn Thanks for flagging; I’ve reverted the change so we can discuss this without rushing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @arcbbb!

I think that's reasonable. We should probably also consolidate all the different memory costing hooks that can go
into getIntrinsicInstrCost? E.g.

getMaskedMemoryOpCost -> llvm.masked.load/store
getGatherScatterOpCost -> llvm.masked.gather/scatter
getExpandCompressMemoryOpCost -> llvm.masked.expandload/llvm.masked.compressstore
getStridedMemoryOpCost -> llvm.experimental.vp.strided.load/store

I think that would be an ideal outcome, but I am not sure how easy it would be to pass the required info through.

Depending on that, it would probably be OK to add another hook like in this PR (at least getVPMemoryOpCost is gone now), but would be great to check if the general direction would be feasible.

Type *DataTy, Align Alignment,
TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput) const;

/// A helper function to determine the type of reduction algorithm used
/// for a given \p Opcode and set of FastMathFlags \p FMF.
static bool requiresOrderedReduction(std::optional<FastMathFlags> FMF) {
Expand Down
6 changes: 6 additions & 0 deletions llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,12 @@ class TargetTransformInfoImplBase {
return 1;
}

virtual InstructionCost
getFirstFaultLoadCost(Type *DataTy, Align Alignment,
TTI::TargetCostKind CostKind) const {
return InstructionCost::getInvalid();
}

virtual InstructionCost
getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
TTI::TargetCostKind CostKind) const {
Expand Down
8 changes: 8 additions & 0 deletions llvm/include/llvm/CodeGen/BasicTTIImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,14 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
}
}

if (ICA.getID() == Intrinsic::vp_load_ff) {
Type *RetTy = ICA.getReturnType();
Type *DataTy = cast<StructType>(RetTy)->getElementType(0);
Align Alignment;
if (auto *VPI = dyn_cast_or_null<VPIntrinsic>(ICA.getInst()))
Alignment = VPI->getPointerAlignment().valueOrOne();
return thisT()->getFirstFaultLoadCost(DataTy, Alignment, CostKind);
}
if (ICA.getID() == Intrinsic::vp_scatter) {
if (ICA.isTypeBasedOnly()) {
IntrinsicCostAttributes MaskedScatter(
Expand Down
9 changes: 9 additions & 0 deletions llvm/lib/Analysis/TargetTransformInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,15 @@ InstructionCost TargetTransformInfo::getInterleavedMemoryOpCost(
return Cost;
}

InstructionCost
TargetTransformInfo::getFirstFaultLoadCost(Type *DataTy, Align Alignment,
TTI::TargetCostKind CostKind) const {
InstructionCost Cost =
TTIImpl->getFirstFaultLoadCost(DataTy, Alignment, CostKind);
assert(Cost >= 0 && "TTI should not produce negative costs!");
return Cost;
}

InstructionCost
TargetTransformInfo::getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
TTI::TargetCostKind CostKind) const {
Expand Down
16 changes: 16 additions & 0 deletions llvm/lib/Target/RISCV/RISCVISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24788,6 +24788,22 @@ bool RISCVTargetLowering::isLegalStridedLoadStore(EVT DataType,
return true;
}

bool RISCVTargetLowering::isLegalFirstFaultLoad(EVT DataType,
Align Alignment) const {
if (!Subtarget.hasVInstructions())
return false;

EVT ScalarType = DataType.getScalarType();
if (!isLegalElementTypeForRVV(ScalarType))
return false;

if (!Subtarget.enableUnalignedVectorMem() &&
Alignment < ScalarType.getStoreSize())
return false;

return true;
}

MachineInstr *
RISCVTargetLowering::EmitKCFICheck(MachineBasicBlock &MBB,
MachineBasicBlock::instr_iterator &MBBI,
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/RISCV/RISCVISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,10 @@ class RISCVTargetLowering : public TargetLowering {
/// alignment is legal.
bool isLegalStridedLoadStore(EVT DataType, Align Alignment) const;

/// Return true if a fault-only-first load of the given result type and
/// alignment is legal.
bool isLegalFirstFaultLoad(EVT DataType, Align Alignment) const;

unsigned getMaxSupportedInterleaveFactor() const override { return 8; }

bool fallBackToDAGISel(const Instruction &Inst) const override;
Expand Down
11 changes: 11 additions & 0 deletions llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,17 @@ InstructionCost RISCVTTIImpl::getInterleavedMemoryOpCost(
return MemCost + ShuffleCost;
}

InstructionCost
RISCVTTIImpl::getFirstFaultLoadCost(Type *DataTy, Align Alignment,
TTI::TargetCostKind CostKind) const {
EVT DataTypeVT = TLI->getValueType(DL, DataTy);
if (!TLI->isLegalFirstFaultLoad(DataTypeVT, Alignment))
return BaseT::getFirstFaultLoadCost(DataTy, Alignment, CostKind);

return getMemoryOpCost(Instruction::Load, DataTy, Alignment, 0, CostKind,
{TTI::OK_AnyValue, TTI::OP_None}, nullptr);
}

InstructionCost RISCVTTIImpl::getGatherScatterOpCost(
unsigned Opcode, Type *DataTy, const Value *Ptr, bool VariableMask,
Align Alignment, TTI::TargetCostKind CostKind, const Instruction *I) const {
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ class RISCVTTIImpl final : public BasicTTIImplBase<RISCVTTIImpl> {
Align Alignment, unsigned AddressSpace, TTI::TargetCostKind CostKind,
bool UseMaskForCond = false, bool UseMaskForGaps = false) const override;

InstructionCost
getFirstFaultLoadCost(Type *DataTy, Align Alignment,
TTI::TargetCostKind CostKind) const override;

InstructionCost getGatherScatterOpCost(unsigned Opcode, Type *DataTy,
const Value *Ptr, bool VariableMask,
Align Alignment,
Expand Down
Loading
Loading