[llvm] Allow MIN/MAX signedness flip when operands are known-negative (PR #174469)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: None (fbrv)

<details>
<summary>Changes</summary>

Extend the existing DAGCombine logic in visitIMINMAX so that signed and unsigned MIN/MAX can be flipped not only when both operands are known non-negative but also when both operands are known negative. This replaces the old SignBitIsZero checks with computeKnownBits and explicit tests for non-negative or negative operands while keeping all existing legality and saturation gating in place. Add regression tests to cover both the known-negative case and the known-non-negative case. Fixes #<!-- -->174325.  
(I used chatgpt to check the grammar for the pr description as english is not my native language)

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


2 Files Affected:

- (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+28-12) 
- (added) llvm/test/CodeGen/X86/dagcombine-iminmax-knownsign.ll (+52) 


``````````diff
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 74d00317c3649..2c7a61385e020 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -6204,26 +6204,42 @@ SDValue DAGCombiner::visitIMINMAX(SDNode *N) {
   if (SDValue RMINMAX = reassociateOps(Opcode, DL, N0, N1, N->getFlags()))
     return RMINMAX;
 
-  // Is sign bits are zero, flip between UMIN/UMAX and SMIN/SMAX.
+  // If both operands are known to have the same sign (both non-negative or both
+  // negative), flip between UMIN/UMAX and SMIN/SMAX.
   // Only do this if:
   // 1. The current op isn't legal and the flipped is.
   // 2. The saturation pattern is broken by canonicalization in InstCombine.
   bool IsOpIllegal = !TLI.isOperationLegal(Opcode, VT);
   bool IsSatBroken = Opcode == ISD::UMIN && N0.getOpcode() == ISD::SMAX;
-  if ((IsSatBroken || IsOpIllegal) && (N0.isUndef() || DAG.SignBitIsZero(N0)) &&
-      (N1.isUndef() || DAG.SignBitIsZero(N1))) {
-    unsigned AltOpcode;
-    switch (Opcode) {
-    case ISD::SMIN: AltOpcode = ISD::UMIN; break;
-    case ISD::SMAX: AltOpcode = ISD::UMAX; break;
-    case ISD::UMIN: AltOpcode = ISD::SMIN; break;
-    case ISD::UMAX: AltOpcode = ISD::SMAX; break;
-    default: llvm_unreachable("Unknown MINMAX opcode");
+
+  if (IsSatBroken || IsOpIllegal) {
+    auto HasKnownSameSign = [&](SDValue A, SDValue B) {
+      if (A.isUndef() || B.isUndef())
+        return true;
+
+      KnownBits KA = DAG.computeKnownBits(A);
+      KnownBits KB = DAG.computeKnownBits(B);
+
+      return (KA.isNonNegative() && KB.isNonNegative()) ||
+            (KA.isNegative() && KB.isNegative());
+    };
+
+    if (HasKnownSameSign(N0, N1)) {
+      unsigned AltOpcode;
+      switch (Opcode) {
+      case ISD::SMIN: AltOpcode = ISD::UMIN; break;
+      case ISD::SMAX: AltOpcode = ISD::UMAX; break;
+      case ISD::UMIN: AltOpcode = ISD::SMIN; break;
+      case ISD::UMAX: AltOpcode = ISD::SMAX; break;
+      default: llvm_unreachable("Unknown MINMAX opcode");
+      }
+
+      if ((IsSatBroken && IsOpIllegal) || TLI.isOperationLegal(AltOpcode, VT))
+        return DAG.getNode(AltOpcode, DL, VT, N0, N1);
     }
-    if ((IsSatBroken && IsOpIllegal) || TLI.isOperationLegal(AltOpcode, VT))
-      return DAG.getNode(AltOpcode, DL, VT, N0, N1);
   }
 
+
   if (Opcode == ISD::SMIN || Opcode == ISD::SMAX)
     if (SDValue S = PerformMinMaxFpToSatCombine(
             N0, N1, N0, N1, Opcode == ISD::SMIN ? ISD::SETLT : ISD::SETGT, DAG))
diff --git a/llvm/test/CodeGen/X86/dagcombine-iminmax-knownsign.ll b/llvm/test/CodeGen/X86/dagcombine-iminmax-knownsign.ll
new file mode 100644
index 0000000000000..71298b5c38b90
--- /dev/null
+++ b/llvm/test/CodeGen/X86/dagcombine-iminmax-knownsign.ll
@@ -0,0 +1,52 @@
+; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+sse2,-sse4.1 < %s | FileCheck %s
+
+declare <16 x i8> @llvm.smax.v16i8(<16 x i8>, <16 x i8>)
+declare <16 x i8> @llvm.smin.v16i8(<16 x i8>, <16 x i8>)
+
+define <16 x i8> @smax_both_negative(<16 x i8> %a, <16 x i8> %b) {
+; CHECK-LABEL: smax_both_negative:
+; CHECK: pmaxub
+  %mask0 = insertelement <16 x i8> poison, i8 -128, i64 0
+  %mask = shufflevector <16 x i8> %mask0, <16 x i8> poison,
+                        <16 x i32> zeroinitializer
+  %a1 = or <16 x i8> %a, %mask
+  %b1 = or <16 x i8> %b, %mask
+  %r = call <16 x i8> @llvm.smax.v16i8(<16 x i8> %a1, <16 x i8> %b1)
+  ret <16 x i8> %r
+}
+
+define <16 x i8> @smin_both_negative(<16 x i8> %a, <16 x i8> %b) {
+; CHECK-LABEL: smin_both_negative:
+; CHECK: pminub
+  %mask0 = insertelement <16 x i8> poison, i8 -128, i64 0
+  %mask = shufflevector <16 x i8> %mask0, <16 x i8> poison,
+                        <16 x i32> zeroinitializer
+  %a1 = or <16 x i8> %a, %mask
+  %b1 = or <16 x i8> %b, %mask
+  %r = call <16 x i8> @llvm.smin.v16i8(<16 x i8> %a1, <16 x i8> %b1)
+  ret <16 x i8> %r
+}
+
+define <16 x i8> @smax_both_nonnegative(<16 x i8> %a, <16 x i8> %b) {
+; CHECK-LABEL: smax_both_nonnegative:
+; CHECK: pmaxub
+  %mask0 = insertelement <16 x i8> poison, i8 127, i64 0
+  %mask = shufflevector <16 x i8> %mask0, <16 x i8> poison,
+                        <16 x i32> zeroinitializer
+  %a1 = and <16 x i8> %a, %mask
+  %b1 = and <16 x i8> %b, %mask
+  %r = call <16 x i8> @llvm.smax.v16i8(<16 x i8> %a1, <16 x i8> %b1)
+  ret <16 x i8> %r
+}
+
+define <16 x i8> @smin_both_nonnegative(<16 x i8> %a, <16 x i8> %b) {
+; CHECK-LABEL: smin_both_nonnegative:
+; CHECK: pminub
+  %mask0 = insertelement <16 x i8> poison, i8 127, i64 0
+  %mask = shufflevector <16 x i8> %mask0, <16 x i8> poison,
+                        <16 x i32> zeroinitializer
+  %a1 = and <16 x i8> %a, %mask
+  %b1 = and <16 x i8> %b, %mask
+  %r = call <16 x i8> @llvm.smin.v16i8(<16 x i8> %a1, <16 x i8> %b1)
+  ret <16 x i8> %r
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/174469