[ValueTracking] Check both operands for being 0 and then the other for isKnownNonZero

[AZero13]

We should check both operands, not just the first one.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: AZero13 (AZero13)

Changes

---

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

1 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+6-5)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 09745ed6eac6a..a8f5728c940b1 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -2880,11 +2880,6 @@ static bool isNonZeroSub(const APInt &DemandedElts, const SimplifyQuery &Q,
   if (matchOpWithOpEqZero(X, Y))
     return true;
 
-  // TODO: Move this case into isKnownNonEqual().
-  if (auto *C = dyn_cast<Constant>(X))
-    if (C->isNullValue() && isKnownNonZero(Y, DemandedElts, Q, Depth))
-      return true;
-
   return ::isKnownNonEqual(X, Y, DemandedElts, Q, Depth);
 }
 
@@ -3892,6 +3887,12 @@ static bool isKnownNonEqual(const Value *V1, const Value *V2,
   if (Depth >= MaxAnalysisRecursionDepth)
     return false;
 
+  // 0 vs known-non-zero => definitely different
+  if (match(V1, m_Zero()) && isKnownNonZero(V2, DemandedElts, Q, Depth + 1))
+    return true;
+  if (match(V2, m_Zero()) && isKnownNonZero(V1, DemandedElts, Q, Depth + 1))
+    return true;
+
   // See if we can recurse through (exactly one of) our operands.  This
   // requires our operation be 1-to-1 and map every input value to exactly
   // one output value.  Such an operation is invertible.

[AZero13]

@dtcxzyw Can you please run the opt-benchmark so I can make tests

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

I don't think this makes sense. x - 0 is just x and should get folded away.