-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[RISCV][TTI] Implement getPartialReductionCost for the vqdotq cases #140974
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -294,6 +294,29 @@ RISCVTTIImpl::getPopcntSupport(unsigned TyWidth) const { | |
: TTI::PSK_Software; | ||
} | ||
|
||
InstructionCost RISCVTTIImpl::getPartialReductionCost( | ||
unsigned Opcode, Type *InputTypeA, Type *InputTypeB, Type *AccumType, | ||
ElementCount VF, TTI::PartialReductionExtendKind OpAExtend, | ||
TTI::PartialReductionExtendKind OpBExtend, | ||
std::optional<unsigned> BinOp) const { | ||
|
||
// zve32x is broken for partial_reduce_umla, but let's make sure we | ||
// don't generate them. | ||
if (!ST->hasStdExtZvqdotq() || ST->getELen() < 64 || | ||
Opcode != Instruction::Add || !BinOp || *BinOp != Instruction::Mul || | ||
InputTypeA != InputTypeB || !InputTypeA->isIntegerTy(8) || | ||
OpAExtend != OpBExtend || !AccumType->isIntegerTy(32) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add a TODO to remove the OpAExtend != OpBExtend restriction once there's a signed-unsigned SD node that we're able to lower to vqdotsu? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want, I can add the TODO, but I didn't feel it added anything with the existing TODO on that topic already existing in lowering. |
||
!VF.isKnownMultipleOf(4) || !VF.isScalable()) | ||
return InstructionCost::getInvalid(); | ||
|
||
Type *Tp = VectorType::get(AccumType, VF); | ||
std::pair<InstructionCost, MVT> LT = getTypeLegalizationCost(Tp); | ||
// Note: Asuming all vqdot* variants are equal cost | ||
// TODO: Thread CostKind through this API | ||
return LT.first * getRISCVInstructionCost(RISCV::VQDOT_VV, LT.second, | ||
TTI::TCK_RecipThroughput); | ||
} | ||
|
||
bool RISCVTTIImpl::shouldExpandReduction(const IntrinsicInst *II) const { | ||
// Currently, the ExpandReductions pass can't expand scalable-vector | ||
// reductions, but we still request expansion as RVV doesn't support certain | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does comparing the std::optional directly work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this would crash as that's an implicit assertion the optional has a value. It may not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would unwrap the optional, the == operator should be defined between an optional and a non-optional: https://en.cppreference.com/w/cpp/utility/optional/operator_cmp