-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LV] Support multiplies by constants when forming scaled reductions. #161092
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
13aeeb7
4c85c89
a30869f
8ec69ae
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 |
---|---|---|
|
@@ -7954,6 +7954,13 @@ bool VPRecipeBuilder::getScaledReductions( | |
auto CollectExtInfo = [this, &Exts, &ExtOpTypes, | ||
&ExtKinds](SmallVectorImpl<Value *> &Ops) -> bool { | ||
for (const auto &[I, OpI] : enumerate(Ops)) { | ||
auto *CI = dyn_cast<ConstantInt>(OpI); | ||
if (I > 0 && CI && | ||
canConstantBeExtended(CI, ExtOpTypes[0], ExtKinds[0])) { | ||
ExtOpTypes[I] = ExtOpTypes[0]; | ||
ExtKinds[I] = ExtKinds[0]; | ||
continue; | ||
} | ||
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. I guess the comment below:
is still valid because the 2nd operand could be a non-constant loop invariant value? 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. I think there are still cases that are covered by the TODO, e.g. we one operand could be a SExt/ZExt outside the loop, which we could support by materializing a new SExt/ZExt in the plan, although that is probably not that easy to do with the current structure. Another case would be non-constant live-ins where we know that only the lowest 7/6 bits are set (depending on the type of extend) |
||
Value *ExtOp; | ||
if (!match(OpI, m_ZExtOrSExt(m_Value(ExtOp)))) | ||
return false; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1753,6 +1753,16 @@ void LoopVectorizationPlanner::printPlans(raw_ostream &O) { | |
} | ||
#endif | ||
|
||
bool llvm::canConstantBeExtended(const ConstantInt *CI, Type *NarrowType, | ||
TTI::PartialReductionExtendKind ExtKind) { | ||
APInt TruncatedVal = CI->getValue().trunc(NarrowType->getScalarSizeInBits()); | ||
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. Do we have a negative partial reduction test for 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. Yep, it should be |
||
unsigned WideSize = CI->getType()->getScalarSizeInBits(); | ||
APInt ExtendedVal = ExtKind == TTI::PR_SignExtend | ||
? TruncatedVal.sext(WideSize) | ||
: TruncatedVal.zext(WideSize); | ||
return ExtendedVal == CI->getValue(); | ||
} | ||
|
||
TargetTransformInfo::OperandValueInfo | ||
VPCostContext::getOperandInfo(VPValue *V) const { | ||
if (!V->isLiveIn()) | ||
|
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.
Would have been good to drop_begin in the enumerate?
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.
We can't drop the first operand, as it needs to be handled below.