[DAGCombiner] Preserve nneg flag from inner zext when we combine (z/s/aext (zext X)) #82199

topperc · 2024-02-19T02:10:32Z

No description provided.

…/aext (zext X))

llvmbot · 2024-02-19T02:11:08Z

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/82199.diff

2 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+13-6)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+17-6)

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 2a09e44e192979..91ed3d9bd69317 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -13710,8 +13710,12 @@ SDValue DAGCombiner::visitZERO_EXTEND(SDNode *N) {
 
   // fold (zext (zext x)) -> (zext x)
   // fold (zext (aext x)) -> (zext x)
-  if (N0.getOpcode() == ISD::ZERO_EXTEND || N0.getOpcode() == ISD::ANY_EXTEND)
-    return DAG.getNode(ISD::ZERO_EXTEND, DL, VT, N0.getOperand(0));
+  if (N0.getOpcode() == ISD::ZERO_EXTEND || N0.getOpcode() == ISD::ANY_EXTEND) {
+    SDNodeFlags Flags;
+    if (N0.getOpcode() == ISD::ZERO_EXTEND)
+      Flags.setNonNeg(N0->getFlags().hasNonNeg());
+    return DAG.getNode(ISD::ZERO_EXTEND, DL, VT, N0.getOperand(0), Flags);
+  }
 
   // fold (zext (aext_extend_vector_inreg x)) -> (zext_extend_vector_inreg x)
   // fold (zext (zext_extend_vector_inreg x)) -> (zext_extend_vector_inreg x)
@@ -13985,10 +13989,13 @@ SDValue DAGCombiner::visitANY_EXTEND(SDNode *N) {
   // fold (aext (aext x)) -> (aext x)
   // fold (aext (zext x)) -> (zext x)
   // fold (aext (sext x)) -> (sext x)
-  if (N0.getOpcode() == ISD::ANY_EXTEND  ||
-      N0.getOpcode() == ISD::ZERO_EXTEND ||
-      N0.getOpcode() == ISD::SIGN_EXTEND)
-    return DAG.getNode(N0.getOpcode(), DL, VT, N0.getOperand(0));
+  if (N0.getOpcode() == ISD::ANY_EXTEND || N0.getOpcode() == ISD::ZERO_EXTEND ||
+      N0.getOpcode() == ISD::SIGN_EXTEND) {
+    SDNodeFlags Flags;
+    if (N0.getOpcode() == ISD::ZERO_EXTEND)
+      Flags.setNonNeg(N0->getFlags().hasNonNeg());
+    return DAG.getNode(N0.getOpcode(), DL, VT, N0.getOperand(0), Flags);
+  }
 
   // fold (aext (aext_extend_vector_inreg x)) -> (aext_extend_vector_inreg x)
   // fold (aext (zext_extend_vector_inreg x)) -> (zext_extend_vector_inreg x)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 421bb516ad242f..add92cf8b31e44 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -5715,8 +5715,12 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
                                   N1.getValueType().getVectorElementCount()) &&
            "Vector element count mismatch!");
     assert(N1.getValueType().bitsLT(VT) && "Invalid sext node, dst < src!");
-    if (OpOpcode == ISD::SIGN_EXTEND || OpOpcode == ISD::ZERO_EXTEND)
-      return getNode(OpOpcode, DL, VT, N1.getOperand(0));
+    if (OpOpcode == ISD::SIGN_EXTEND || OpOpcode == ISD::ZERO_EXTEND) {
+      SDNodeFlags Flags;
+      if (OpOpcode == ISD::ZERO_EXTEND)
+        Flags.setNonNeg(N1->getFlags().hasNonNeg());
+      return getNode(OpOpcode, DL, VT, N1.getOperand(0), Flags);
+    }
     if (OpOpcode == ISD::UNDEF)
       // sext(undef) = 0, because the top bits will all be the same.
       return getConstant(0, DL, VT);
@@ -5732,8 +5736,11 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
                                   N1.getValueType().getVectorElementCount()) &&
            "Vector element count mismatch!");
     assert(N1.getValueType().bitsLT(VT) && "Invalid zext node, dst < src!");
-    if (OpOpcode == ISD::ZERO_EXTEND) // (zext (zext x)) -> (zext x)
-      return getNode(ISD::ZERO_EXTEND, DL, VT, N1.getOperand(0));
+    if (OpOpcode == ISD::ZERO_EXTEND) { // (zext (zext x)) -> (zext x)
+      SDNodeFlags Flags;
+      Flags.setNonNeg(N1->getFlags().hasNonNeg());
+      return getNode(ISD::ZERO_EXTEND, DL, VT, N1.getOperand(0), Flags);
+    }
     if (OpOpcode == ISD::UNDEF)
       // zext(undef) = 0, because the top bits will be zero.
       return getConstant(0, DL, VT);
@@ -5769,9 +5776,13 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
     assert(N1.getValueType().bitsLT(VT) && "Invalid anyext node, dst < src!");
 
     if (OpOpcode == ISD::ZERO_EXTEND || OpOpcode == ISD::SIGN_EXTEND ||
-        OpOpcode == ISD::ANY_EXTEND)
+        OpOpcode == ISD::ANY_EXTEND) {
+      SDNodeFlags Flags;
+      if (OpOpcode == ISD::ZERO_EXTEND)
+        Flags.setNonNeg(N1->getFlags().hasNonNeg());
       // (ext (zext x)) -> (zext x)  and  (ext (sext x)) -> (sext x)
-      return getNode(OpOpcode, DL, VT, N1.getOperand(0));
+      return getNode(OpOpcode, DL, VT, N1.getOperand(0), Flags);
+    }
     if (OpOpcode == ISD::UNDEF)
       return getUNDEF(VT);

goldsteinn · 2024-02-19T03:33:49Z

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

-    return DAG.getNode(ISD::ZERO_EXTEND, DL, VT, N0.getOperand(0));
+  if (N0.getOpcode() == ISD::ZERO_EXTEND || N0.getOpcode() == ISD::ANY_EXTEND) {
+    SDNodeFlags Flags;
+    if (N0.getOpcode() == ISD::ZERO_EXTEND)


Doesn't zext always imply nneg unless sizeof(dstty) <= sizeof(srcty) in which case its a nop?

nneg is about the sign bit of the input to the zero extend, not the output. It indicates that the zext could be a replaced with a sext.

… sign bits. This treats the zext nneg as sext if X is known to have sufficient sign bits to allow the zext or truncate or both to removed. This code is taken from the same optimization for sext. Test cases are based on a common pattern where a zext nneg is created based on a dominating condition that ensure the value is positive. The value will be exported from the first block sign extended to a legal type. This creates an AssertSExt and truncate in the next block. Treating the zext nneg as a sext allows us to remove the zext. The zext_nneg_crossbb_i32 test will be further improved when this patch combines with llvm#82199.

nikic

The changes LGTM, but is it possible to test it (or at least some parts of it)?

… sign bits. This treats the zext nneg as sext if X is known to have sufficient sign bits to allow the zext or truncate or both to removed. This code is taken from the same optimization for sext. Test cases are based on a common pattern where a zext nneg is created based on a dominating condition that ensure the value is positive. The value will be exported from the first block sign extended to a legal type. This creates an AssertSExt and truncate in the next block. Treating the zext nneg as a sext allows us to remove the zext. The zext_nneg_crossbb_i32 test will be further improved when this patch combines with llvm#82199.

topperc · 2024-02-19T18:57:59Z

The changes LGTM, but is it possible to test it (or at least some parts of it)?

Patch now contains changes to one of the tests from #82227. That tests the code in getNode(ISD::ANY_EXTEND

This adds additional tests for #82199. These tests need us to propagate the nneg flag when we zero/sign extend an existing zext nneg node. For these tests on RV64, call lowering will need to sign extend or zero extend the existing zext nneg to i64. getNode will fold this into a single zext. We should propagate the nneg flag from the original zext nneg. This will allow us to remove the zext nneg based on known sign bits during DAG combine.

topperc · 2024-02-19T19:15:45Z

The changes LGTM, but is it possible to test it (or at least some parts of it)?

Patch now contains changes to one of the tests from #82227. That tests the code in getNode(ISD::ANY_EXTEND

Added two more tests to covert getNode for ISD::SIGN_EXTEND and ISD::ZERO_EXTEND.

nikic

LGTM

[DAGCombiner] Preserve nneg flag from inner zext when we combine (z/s…

8c4bd86

…/aext (zext X))

topperc requested review from arsenm, nikic and RKSimon February 19, 2024 02:10

llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Feb 19, 2024

goldsteinn reviewed Feb 19, 2024

View reviewed changes

topperc mentioned this pull request Feb 19, 2024

[DAGCombiner][RISCV] Optimize (zext nneg (truncate X)) if X has known sign bits. #82227

Merged

nikic reviewed Feb 19, 2024

View reviewed changes

topperc added 2 commits February 19, 2024 10:46

Merge remote-tracking branch 'origin/main' into pr/preserve-nneg

15dc555

fixup! regenerate test.

6d0c644

topperc added 2 commits February 19, 2024 11:13

Merge remote-tracking branch 'origin/main' into pr/preserve-nneg

a56af52

fixup! Update newly added tests.

67fb0d0

nikic approved these changes Feb 19, 2024

View reviewed changes

topperc merged commit f8cbb67 into llvm:main Feb 19, 2024
4 checks passed

topperc deleted the pr/preserve-nneg branch February 19, 2024 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DAGCombiner] Preserve nneg flag from inner zext when we combine (z/s/aext (zext X)) #82199

[DAGCombiner] Preserve nneg flag from inner zext when we combine (z/s/aext (zext X)) #82199

topperc commented Feb 19, 2024

llvmbot commented Feb 19, 2024

goldsteinn Feb 19, 2024

topperc Feb 19, 2024

nikic left a comment

topperc commented Feb 19, 2024

topperc commented Feb 19, 2024

nikic left a comment

[DAGCombiner] Preserve nneg flag from inner zext when we combine (z/s/aext (zext X)) #82199

[DAGCombiner] Preserve nneg flag from inner zext when we combine (z/s/aext (zext X)) #82199

Conversation

topperc commented Feb 19, 2024

llvmbot commented Feb 19, 2024

goldsteinn Feb 19, 2024

Choose a reason for hiding this comment

topperc Feb 19, 2024

Choose a reason for hiding this comment

nikic left a comment

Choose a reason for hiding this comment

topperc commented Feb 19, 2024

topperc commented Feb 19, 2024

nikic left a comment

Choose a reason for hiding this comment