-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
[DAGCombiner] Turn (neg (max x, (neg x)))
into (min x, (neg x))
#120666
base: main
Are you sure you want to change the base?
Changes from 5 commits
abf6f85
25c43d8
1abcdc3
55d7531
3e85bc4
b8b253b
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 |
---|---|---|
|
@@ -3949,6 +3949,24 @@ SDValue DAGCombiner::visitSUB(SDNode *N) { | |
if (SDValue Result = TLI.expandABS(N1.getNode(), DAG, true)) | ||
return Result; | ||
|
||
// Similar to the previous rule, but this time targeting an expanded abs. | ||
// (sub 0, (max X, (sub 0, X))) --> (min X, (sub 0, X)) | ||
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. Could also do the converse:
but I don't know if there's a real need for it. 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 it's fine to add it in this patch. It's done now. |
||
// as well as | ||
// (sub 0, (min X, (sub 0, X))) --> (max X, (sub 0, X)) | ||
// Note that these two are applicable to both signed and unsigned min/max. | ||
SDValue X; | ||
SDValue S0; | ||
auto NegPat = m_AllOf(m_Neg(m_Deferred(X)), m_Value(S0)); | ||
if (LegalOperations && | ||
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. Technically you should check the max is legal, but I doubt in practice the legality of min is different than max 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.
This, and even if only max is illegal, I think it'll be expanded (while min is not), which makes this transformation even more profitable. 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. It depends in which combiner phase, eventually only legal operations can be emitted 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. Ah, if you're referring to the max generated by this rule, I already checked its legality (through |
||
sd_match(N1, m_OneUse(m_AnyOf(m_SMax(m_Value(X), NegPat), | ||
m_UMax(m_Value(X), NegPat), | ||
m_SMin(m_Value(X), NegPat), | ||
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 this can preserve flags 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. Good catch. I simply capture and reuse the SDValue of the first sub. It's done now. |
||
m_UMin(m_Value(X), NegPat))))) { | ||
unsigned NewOpc = ISD::getInverseMinMaxOpcode(N1->getOpcode()); | ||
if (hasOperation(NewOpc, VT)) | ||
return DAG.getNode(NewOpc, DL, VT, X, S0); | ||
} | ||
|
||
// Fold neg(splat(neg(x)) -> splat(x) | ||
if (VT.isVector()) { | ||
SDValue N1S = DAG.getSplatValue(N1, true); | ||
|
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.
Document
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.
Done