[llvm] [ValueTracking] improve `isKnownNonZero` precision for `smax` (PR #88170)

[via llvm-commits]

https://github.com/goldsteinn updated https://github.com/llvm/llvm-project/pull/88170

>From 86b3c85e23c8572efdb21ea51b1f283d2d3a99c5 Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Tue, 9 Apr 2024 12:36:03 -0500
Subject: [PATCH 1/2] [ValueTracking] Add tests for improving `isKnownNonZero`
 of `smax`; NFC

---
 .../Transforms/InstSimplify/known-non-zero.ll     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/llvm/test/Transforms/InstSimplify/known-non-zero.ll b/llvm/test/Transforms/InstSimplify/known-non-zero.ll
index b647f11af4461d..6ebc4e0f31a9cf 100644
--- a/llvm/test/Transforms/InstSimplify/known-non-zero.ll
+++ b/llvm/test/Transforms/InstSimplify/known-non-zero.ll
@@ -166,3 +166,18 @@ A:
 B:
   ret i1 0
 }
+
+define i1 @smax_non_zero(i8 %xx, i8 %y) {
+; CHECK-LABEL: @smax_non_zero(
+; CHECK-NEXT:    [[X0:%.*]] = and i8 [[XX:%.*]], 63
+; CHECK-NEXT:    [[X:%.*]] = add i8 [[X0]], 1
+; CHECK-NEXT:    [[V:%.*]] = call i8 @llvm.smax.i8(i8 [[X]], i8 [[Y:%.*]])
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[V]], 0
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %x0 = and i8 %xx, 63
+  %x = add i8 %x0, 1
+  %v = call i8 @llvm.smax.i8(i8 %x, i8 %y)
+  %r = icmp eq i8 %v, 0
+  ret i1 %r
+}

>From ba62ff8d963ac2adce4e3bb916e18f23020f480e Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Tue, 9 Apr 2024 11:58:03 -0500
Subject: [PATCH 2/2] [ValueTracking] improve `isKnownNonZero` precision for
 `smax`

Instead of relying on known-bits for strictly positive, use the
`isKnownPositive` API. This will use `isKnownNonZero` which is more
accurate.
---
 llvm/lib/Analysis/ValueTracking.cpp           | 48 ++++++++++++++-----
 .../Transforms/InstSimplify/known-non-zero.ll |  6 +--
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index ca48cfe7738154..9fe7c2e88cb2b4 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -2828,23 +2828,47 @@ static bool isKnownNonZeroFromOperator(const Operator *I,
       case Intrinsic::uadd_sat:
         return isKnownNonZero(II->getArgOperand(1), DemandedElts, Depth, Q) ||
                isKnownNonZero(II->getArgOperand(0), DemandedElts, Depth, Q);
-      case Intrinsic::smin:
       case Intrinsic::smax: {
-        auto KnownOpImpliesNonZero = [&](const KnownBits &K) {
-          return II->getIntrinsicID() == Intrinsic::smin
-                     ? K.isNegative()
-                     : K.isStrictlyPositive();
-        };
-        KnownBits XKnown =
+        // if either arg is strictly positive the result is non-zero. Otherwise
+        // the result is non-zero if both ops are non-zero.
+        KnownBits Op1Known =
+            computeKnownBits(II->getArgOperand(1), DemandedElts, Depth, Q);
+        // Avoid re-computing isKnownNonZero if we already failed once.
+        bool OpsMaybeZero = false;
+        if (Op1Known.isNonNegative()) {
+          if (Op1Known.isNonZero() ||
+              isKnownNonZero(II->getArgOperand(1), DemandedElts, Depth, Q))
+            return true;
+          OpsMaybeZero = true;
+        }
+        KnownBits Op0Known =
             computeKnownBits(II->getArgOperand(0), DemandedElts, Depth, Q);
-        if (KnownOpImpliesNonZero(XKnown))
-          return true;
-        KnownBits YKnown =
+        if (Op0Known.isNonNegative()) {
+          if (Op0Known.isNonZero() ||
+              isKnownNonZero(II->getArgOperand(0), DemandedElts, Depth, Q))
+            return true;
+          OpsMaybeZero = true;
+        }
+        // We already failed at least one call to isKnownNonZero
+        if (OpsMaybeZero)
+          break;
+        return isKnownNonZero(II->getArgOperand(0), DemandedElts, Depth, Q) &&
+               isKnownNonZero(II->getArgOperand(1), DemandedElts, Depth, Q);
+      }
+
+      case Intrinsic::smin: {
+        // if either arg is negative the result is non-zero. Otherwise
+        // the result is non-zero if both ops are non-zero.
+        KnownBits Op1Known =
             computeKnownBits(II->getArgOperand(1), DemandedElts, Depth, Q);
-        if (KnownOpImpliesNonZero(YKnown))
+        if (Op1Known.isNegative())
+          return true;
+        KnownBits Op0Known =
+            computeKnownBits(II->getArgOperand(0), DemandedElts, Depth, Q);
+        if (Op0Known.isNegative())
           return true;
 
-        if (XKnown.isNonZero() && YKnown.isNonZero())
+        if (Op1Known.isNonZero() && Op0Known.isNonZero())
           return true;
       }
         [[fallthrough]];
diff --git a/llvm/test/Transforms/InstSimplify/known-non-zero.ll b/llvm/test/Transforms/InstSimplify/known-non-zero.ll
index 6ebc4e0f31a9cf..51f80f62c2f34c 100644
--- a/llvm/test/Transforms/InstSimplify/known-non-zero.ll
+++ b/llvm/test/Transforms/InstSimplify/known-non-zero.ll
@@ -169,11 +169,7 @@ B:
 
 define i1 @smax_non_zero(i8 %xx, i8 %y) {
 ; CHECK-LABEL: @smax_non_zero(
-; CHECK-NEXT:    [[X0:%.*]] = and i8 [[XX:%.*]], 63
-; CHECK-NEXT:    [[X:%.*]] = add i8 [[X0]], 1
-; CHECK-NEXT:    [[V:%.*]] = call i8 @llvm.smax.i8(i8 [[X]], i8 [[Y:%.*]])
-; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[V]], 0
-; CHECK-NEXT:    ret i1 [[R]]
+; CHECK-NEXT:    ret i1 false
 ;
   %x0 = and i8 %xx, 63
   %x = add i8 %x0, 1