-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AMDGPU] prevent shrinking udiv/urem if either operand is in (SignedMax,UnsignedMax] #116733
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 |
---|---|---|
|
@@ -1193,19 +1193,35 @@ int AMDGPUCodeGenPrepareImpl::getDivNumBits(BinaryOperator &I, Value *Num, | |
Value *Den, unsigned AtLeast, | ||
bool IsSigned) const { | ||
const DataLayout &DL = Mod->getDataLayout(); | ||
unsigned LHSSignBits = ComputeNumSignBits(Num, DL, 0, AC, &I); | ||
if (LHSSignBits < AtLeast) | ||
return -1; | ||
|
||
unsigned RHSSignBits = ComputeNumSignBits(Den, DL, 0, AC, &I); | ||
if (RHSSignBits < AtLeast) | ||
return -1; | ||
|
||
unsigned SignBits = std::min(LHSSignBits, RHSSignBits); | ||
unsigned DivBits = Num->getType()->getScalarSizeInBits() - SignBits; | ||
if (IsSigned) | ||
++DivBits; | ||
return DivBits; | ||
if (IsSigned) { | ||
unsigned LHSSignBits = ComputeNumSignBits(Num, DL, 0, AC, &I); | ||
if (LHSSignBits < AtLeast) | ||
return -1; | ||
|
||
unsigned RHSSignBits = ComputeNumSignBits(Den, DL, 0, AC, &I); | ||
if (RHSSignBits < AtLeast) | ||
return -1; | ||
|
||
unsigned SignBits = std::min(LHSSignBits, RHSSignBits); | ||
unsigned DivBits = Num->getType()->getScalarSizeInBits() - SignBits; | ||
return DivBits + 1; | ||
} else { | ||
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. No else after return |
||
KnownBits Known = computeKnownBits(Num, DL, 0, AC, &I); | ||
// We know all bits are used for division for Num or Den in range | ||
// (SignedMax, UnsignedMax] | ||
if (Known.isNegative() || !Known.isNonNegative()) | ||
return -1; | ||
Comment on lines
+1210
to
+1213
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. Is this reproducing the logic of computeKnownBits for division? Can you just do KnownLHS.udiv/urem(KnowRHS)? 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. Unfortunately we need the KnownBits of the operands for shrinking as div/rem gives us the known of the final output. |
||
unsigned LHSSignBits = Known.countMinLeadingZeros(); | ||
|
||
Known = computeKnownBits(Den, DL, 0, AC, &I); | ||
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. Try RHS first |
||
if (Known.isNegative() || !Known.isNonNegative()) | ||
return -1; | ||
unsigned RHSSignBits = Known.countMinLeadingZeros(); | ||
|
||
unsigned SignBits = std::min(LHSSignBits, RHSSignBits); | ||
unsigned DivBits = Num->getType()->getScalarSizeInBits() - SignBits; | ||
return DivBits; | ||
} | ||
} | ||
|
||
// The fractional part of a float is enough to accurately represent up to | ||
|
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.
Should try RHS first as it's canonically simpler