-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[DAG] Canonicalize zero_extend to sign_extend based on target preference #70671
Conversation
We already had code to stop the canonicalization in the other direction, let's add the inverse combine. It turned out we'd missed a least one case which unconditionally created zero_extend, so fix that too to avoid a combine loop. Note that this will most likely be entirely reworked in the near future based on the new nneg flag. This change was triggered by me investigating what that would look like, and asking why the current structure was incomplete.
@llvm/pr-subscribers-llvm-selectiondag Author: Philip Reames (preames) ChangesWe already had code to stop the canonicalization in the other direction, let's add the inverse combine. It turned out we'd missed a least one case which unconditionally created zero_extend, so fix that too to avoid a combine loop. Note that this will most likely be entirely reworked in the near future based on the new nneg flag. This change was triggered by me investigating what that would look like, and asking why the current structure was incomplete. Full diff: https://github.com/llvm/llvm-project/pull/70671.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index ca5bd4952866886..3f186a71256d084 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -13481,7 +13481,8 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
if (SDValue V = foldSextSetcc(N))
return V;
- // fold (sext x) -> (zext x) if the sign bit is known zero.
+ // fold (sext x) -> (zext x) if the sign bit is known zero, and that
+ // is what the target prefers.
if (!TLI.isSExtCheaperThanZExt(N0.getValueType(), VT) &&
(!LegalOperations || TLI.isOperationLegal(ISD::ZERO_EXTEND, VT)) &&
DAG.SignBitIsZero(N0))
@@ -13825,6 +13826,14 @@ SDValue DAGCombiner::visitZERO_EXTEND(SDNode *N) {
return DAG.getNode(ISD::ZERO_EXTEND, DL, VT, SCC);
}
+ // fold (zext x) -> (sext x) if the sign bit is known zero, and
+ // that is what the target prefers.
+ if (TLI.isSExtCheaperThanZExt(N0.getValueType(), VT) &&
+ (!LegalOperations || TLI.isOperationLegal(ISD::SIGN_EXTEND, VT)) &&
+ DAG.SignBitIsZero(N0))
+ return DAG.getNode(ISD::SIGN_EXTEND, DL, VT, N0);
+
+
// (zext (shl (zext x), cst)) -> (shl (zext x), cst)
if ((N0.getOpcode() == ISD::SHL || N0.getOpcode() == ISD::SRL) &&
!TLI.isZExtFree(N0, VT)) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 8b4f3159499122a..ee6f87374823702 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -2419,7 +2419,7 @@ bool TargetLowering::SimplifyDemandedBits(
Known = Known.sext(BitWidth);
// If the sign bit is known zero, convert this to a zero extend.
- if (Known.isNonNegative()) {
+ if (Known.isNonNegative() && !isSExtCheaperThanZExt(SrcVT, VT)) {
unsigned Opc =
IsVecInReg ? ISD::ZERO_EXTEND_VECTOR_INREG : ISD::ZERO_EXTEND;
if (!TLO.LegalOperations() || isOperationLegal(Opc, VT))
|
You can test this locally with the following command:git-clang-format --diff 56dab2cb0733f10df4e9cff8c83dd7081154527b d19c0c56fb97ed9c248e9f6182419a078c1f5761 -- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp View the diff from clang-format here.diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 3f186a71256d..3e2fda23160c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -13833,7 +13833,6 @@ SDValue DAGCombiner::visitZERO_EXTEND(SDNode *N) {
DAG.SignBitIsZero(N0))
return DAG.getNode(ISD::SIGN_EXTEND, DL, VT, N0);
-
// (zext (shl (zext x), cst)) -> (shl (zext x), cst)
if ((N0.getOpcode() == ISD::SHL || N0.getOpcode() == ISD::SRL) &&
!TLI.isZExtFree(N0, VT)) {
|
Test? |
Abandoning as this is no longer applicable given the recent change to zext nneg. The only piece of this which looks keeping, I've moved to it's own review and rewritten in terms of zext nneg. #72281 |
We already had code to stop the canonicalization in the other direction, let's add the inverse combine. It turned out we'd missed a least one case which unconditionally created zero_extend, so fix that too to avoid a combine loop.
Note that this will most likely be entirely reworked in the near future based on the new nneg flag. This change was triggered by me investigating what that would look like, and asking why the current structure was incomplete.