[ValueTracking] Add support for sub in isKnownNonEqual

[goldsteinn]

• [ValueTracking] Add tests for sub in isKnownNonEqual; NFC
• [ValueTracking] Add support for sub in isKnownNonEqual

There are several cases we can handle.

1) `-X != X` if `X != 0 && X != INT_MIN`
2) `X - Y != X` if `Y != 0`
3) `Y - X != X` if `Y != 2 * X`

Proofs: https://alive2.llvm.org/ce/z/cuvYV3

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

• [ValueTracking] Add tests for sub in isKnownNonEqual; NFC
• [ValueTracking] Add support for sub in isKnownNonEqual

---

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

2 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+38-4)
• (modified) llvm/test/Transforms/InstSimplify/icmp.ll (+98-4)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 5ad4da43bca7db..dc9490180997c7 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -2456,10 +2456,11 @@ static bool isNonZeroAdd(const APInt &DemandedElts, unsigned Depth,
 static bool isNonZeroSub(const APInt &DemandedElts, unsigned Depth,
                          const SimplifyQuery &Q, unsigned BitWidth, Value *X,
                          Value *Y) {
-  // TODO: Move this case into isKnownNonEqual().
-  if (auto *C = dyn_cast<Constant>(X))
-    if (C->isNullValue() && isKnownNonZero(Y, DemandedElts, Depth, Q))
-      return true;
+  // We duplicate this case with isKnownNonEqual because we have DemandedElts
+  // here but not in isKnownNonEqual.
+  // If we add support for DemandedElts in isKnownNonEqual, drop this case.
+  if (match(X, m_Zero()))
+    return isKnownNonZero(Y, DemandedElts, Depth, Q);
 
   return ::isKnownNonEqual(X, Y, Depth, Q);
 }
@@ -3134,6 +3135,36 @@ static bool isNonEqualShl(const Value *V1, const Value *V2, unsigned Depth,
   return false;
 }
 
+/// Return true if V1 == V2 - X or V1 == X - V2 implies V2 != V1
+static bool isNonEqualSub(const Value *V1, const Value *V2, unsigned Depth,
+                          const SimplifyQuery &Q) {
+  const BinaryOperator *BO = dyn_cast<BinaryOperator>(V1);
+  if (!BO || BO->getOpcode() != Instruction::Sub)
+    return false;
+
+  // -V2 != V2 iff V2 != 0 and V2 != INT_MIN
+  if (match(BO, m_Sub(m_Zero(), m_Specific(V2)))) {
+    const OverflowingBinaryOperator *OBO = cast<OverflowingBinaryOperator>(V1);
+    // nsw implies no INT_MIN case.
+    if (OBO->hasNoSignedWrap())
+      return isKnownNonZero(V2, Depth + 1, Q);
+    // Otherwise check for INT_MIN case directly.
+    KnownBits V2Known = computeKnownBits(V2, Depth + 1, Q);
+    if (V2Known.isNonNegative() ||
+        (!V2Known.One.isZero() && !V2Known.One.isMinSignedValue()))
+      return V2Known.isNonZero() || isKnownNonZero(V2, Depth + 1, Q);
+  }
+
+  // X - V2 != V2 if X != 0
+  if (V2 == BO->getOperand(0))
+    return isKnownNonZero(BO->getOperand(1), Depth + 1, Q);
+  if (V2 == BO->getOperand(1))
+    return isKnownNonZero(BO->getOperand(0), Depth + 1, Q);
+
+  return false;
+}
+
+
 static bool isNonEqualPHIs(const PHINode *PN1, const PHINode *PN2,
                            unsigned Depth, const SimplifyQuery &Q) {
   // Check two PHIs are in same block.
@@ -3274,6 +3305,9 @@ static bool isKnownNonEqual(const Value *V1, const Value *V2, unsigned Depth,
   if (isNonEqualShl(V1, V2, Depth, Q) || isNonEqualShl(V2, V1, Depth, Q))
     return true;
 
+  if (isNonEqualSub(V1, V2, Depth, Q) || isNonEqualSub(V2, V1, Depth, Q))
+    return true;
+
   if (V1->getType()->isIntOrIntVectorTy()) {
     // Are any known bits in V1 contradictory to known bits in V2? If V1
     // has a known zero where V2 has a known one, they must not be equal.
diff --git a/llvm/test/Transforms/InstSimplify/icmp.ll b/llvm/test/Transforms/InstSimplify/icmp.ll
index 3109768bdfe005..699262b074e0d2 100644
--- a/llvm/test/Transforms/InstSimplify/icmp.ll
+++ b/llvm/test/Transforms/InstSimplify/icmp.ll
@@ -250,9 +250,7 @@ define <2 x i1> @sub_odd_poison(<2 x i8> %x) {
 
 define i1 @sub_even(i8 %x) {
 ; CHECK-LABEL: @sub_even(
-; CHECK-NEXT:    [[SUB:%.*]] = sub i8 2, [[X:%.*]]
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i8 [[SUB]], [[X]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 true
 ;
   %sub = sub i8 2, %x
   %cmp = icmp ne i8 %sub, %x
@@ -270,7 +268,7 @@ define i1 @load_ptr(ptr %p) {
 
 define i1 @load_ptr_null_valid(ptr %p) null_pointer_is_valid {
 ; CHECK-LABEL: @load_ptr_null_valid(
-; CHECK-NEXT:    [[LOAD_P:%.*]] = load ptr, ptr [[P:%.*]], align 8, !dereferenceable !0
+; CHECK-NEXT:    [[LOAD_P:%.*]] = load ptr, ptr [[P:%.*]], align 8, !dereferenceable [[META0:![0-9]+]]
 ; CHECK-NEXT:    [[R:%.*]] = icmp ne ptr [[LOAD_P]], null
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
@@ -278,3 +276,99 @@ define i1 @load_ptr_null_valid(ptr %p) null_pointer_is_valid {
   %r = icmp ne ptr %load_p, null
   ret i1 %r
 }
+
+define i1 @non_eq_sub_neg_case_nsw(i8 %x, i8 %yy) {
+; CHECK-LABEL: @non_eq_sub_neg_case_nsw(
+; CHECK-NEXT:    ret i1 false
+;
+  %y = add nuw i8 %yy, 1
+  %lhs = add i8 %x, %y
+  %suby = sub nsw i8 0, %y
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+
+define i1 @non_eq_sub_neg_case_no_intmin(i8 %x, i8 %yy) {
+; CHECK-LABEL: @non_eq_sub_neg_case_no_intmin(
+; CHECK-NEXT:    ret i1 false
+;
+  %y0 = and i8 %yy, 63
+  %y = add nuw i8 %y0, 1
+  %lhs = add i8 %x, %y
+  %suby = sub i8 0, %y
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+
+define i1 @non_eq_sub_neg_case_no_intmin2(i8 %x, i8 %yy) {
+; CHECK-LABEL: @non_eq_sub_neg_case_no_intmin2(
+; CHECK-NEXT:    ret i1 false
+;
+  %y = or i8 %yy, -64
+  %lhs = add i8 %x, %y
+  %suby = sub i8 0, %y
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+
+define i1 @non_eq_sub_neg_case_fail(i8 %x, i8 %yy) {
+; CHECK-LABEL: @non_eq_sub_neg_case_fail(
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[LHS:%.*]] = add i8 [[X:%.*]], [[Y]]
+; CHECK-NEXT:    [[SUBY:%.*]] = sub i8 0, [[Y]]
+; CHECK-NEXT:    [[RHS:%.*]] = add i8 [[X]], [[SUBY]]
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[LHS]], [[RHS]]
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %y = add nuw i8 %yy, 1
+  %lhs = add i8 %x, %y
+  %suby = sub i8 0, %y
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+
+define i1 @non_eq_sub(i8 %x, i8 %yy, i8 %z) {
+; CHECK-LABEL: @non_eq_sub(
+; CHECK-NEXT:    ret i1 false
+;
+  %y = add nuw i8 %yy, 1
+  %lhs = add i8 %x, %z
+  %suby = sub i8 %z, %y
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+
+define i1 @non_eq_sub2(i8 %x, i8 %yy, i8 %z) {
+; CHECK-LABEL: @non_eq_sub2(
+; CHECK-NEXT:    ret i1 false
+;
+  %y = add nuw i8 %yy, 1
+  %lhs = add i8 %x, %z
+  %suby = sub i8 %y, %z
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+
+define i1 @non_eq_sub2_fail(i8 %x, i8 %yy, i8 %z) {
+; CHECK-LABEL: @non_eq_sub2_fail(
+; CHECK-NEXT:    [[Y:%.*]] = add nsw i8 [[YY:%.*]], 1
+; CHECK-NEXT:    [[LHS:%.*]] = add i8 [[X:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[SUBY:%.*]] = sub i8 [[Y]], [[Z]]
+; CHECK-NEXT:    [[RHS:%.*]] = add i8 [[X]], [[SUBY]]
+; CHECK-NEXT:    [[R:%.*]] = icmp eq i8 [[LHS]], [[RHS]]
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %y = add nsw i8 %yy, 1
+  %lhs = add i8 %x, %z
+  %suby = sub i8 %y, %z
+  %rhs = add i8 %x, %suby
+  %r = icmp eq i8 %lhs, %rhs
+  ret i1 %r
+}
+

[github-actions]

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

[nikic]

@dtcxzyw Can you please test this one?

[nikic]

Looks like there is essentially no impact, and the impact there is does not look positive (? not entirely sure on that). As such, I don't think we should add this.

[goldsteinn]

Looks like there is essentially no impact, and the impact there is does not look positive (? not entirely sure on that). As such, I don't think we should add this.

Okay, ill put this on hold unless a motivating case can be found.
I might push the comment in the isNonZeroSub helper.