Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ValueTracking] Improve knownbits from conditions where Val is used by and/or. #86059

Closed

Conversation

goldsteinn
Copy link
Contributor

@goldsteinn goldsteinn commented Mar 21, 2024

  • [ValueTracking] Add tests for computing knownbits from (icmp upred X (and/or X, Y)); NFC
  • [ValueTracking] compute knownbits from (icmp upred X (and/or X, Y)); NFC

Proofs: https://alive2.llvm.org/ce/z/K4vFn-

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 21, 2024

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes
  • [ValueTracking] Add basic tests tracking or disjoint conditions as add; NFC
  • [ValueTracking] Tracking or disjoint conditions as add in Assumption/DomCondition Cache
  • [ValueTracking] Add tests for computing knownbits from (icmp upred X (and/or X, Y)); NFC
  • [ValueTracking] compute knownbits from (icmp upred X (and/or X, Y)); NFC

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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+36-12)
  • (modified) llvm/test/Transforms/InstCombine/known-bits.ll (+246)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 797665cf06c875..ade8fef385ec15 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -698,13 +698,26 @@ static void computeKnownBitsFromCmp(const Value *V, CmpInst::Predicate Pred,
     break;
   }
   default:
-    const APInt *Offset = nullptr;
-    if (match(LHS, m_CombineOr(m_V, m_Add(m_V, m_APInt(Offset)))) &&
-        match(RHS, m_APInt(C))) {
-      ConstantRange LHSRange = ConstantRange::makeAllowedICmpRegion(Pred, *C);
-      if (Offset)
-        LHSRange = LHSRange.sub(*Offset);
-      Known = Known.unionWith(LHSRange.toKnownBits());
+    if (match(RHS, m_APInt(C))) {
+      const APInt *Offset = nullptr;
+      if (match(LHS, m_CombineOr(m_V, m_AddLike(m_V, m_APInt(Offset))))) {
+        ConstantRange LHSRange = ConstantRange::makeAllowedICmpRegion(Pred, *C);
+        if (Offset)
+          LHSRange = LHSRange.sub(*Offset);
+        Known = Known.unionWith(LHSRange.toKnownBits());
+      }
+      // X & Y u> C -> X u> C && Y u> C
+      if ((Pred == ICmpInst::ICMP_UGT || Pred == ICmpInst::ICMP_UGE) &&
+          match(LHS, m_c_And(m_V, m_Value()))) {
+        Known.One.setHighBits(
+            (*C + (Pred == ICmpInst::ICMP_UGT)).countLeadingOnes());
+      }
+      // X | Y u< C -> X u< C && Y u< C
+      if ((Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_ULE) &&
+          match(LHS, m_c_Or(m_V, m_Value()))) {
+        Known.Zero.setHighBits(
+            (*C - (Pred == ICmpInst::ICMP_ULT)).countLeadingZeros());
+      }
     }
     break;
   }
@@ -9283,11 +9296,22 @@ void llvm::findValuesAffectedByCondition(
             AddAffected(X);
         }
       } else {
-        // Handle (A + C1) u< C2, which is the canonical form of
-        // A > C3 && A < C4.
-        if (match(A, m_Add(m_Value(X), m_ConstantInt())) &&
-            match(B, m_ConstantInt()))
-          AddAffected(X);
+        if (match(B, m_ConstantInt())) {
+          // Handle (A + C1) u< C2, which is the canonical form of
+          // A > C3 && A < C4.
+          if (match(A, m_AddLike(m_Value(X), m_ConstantInt())))
+            AddAffected(X);
+
+          Value *Y;
+          // X & Y u> C -> X >u C && Y >u C
+          // X | Y u< C -> X u< C && Y u< C
+          if (ICmpInst::isUnsigned(Pred) &&
+              (match(A, m_And(m_Value(X), m_Value(Y))) ||
+               match(A, m_Or(m_Value(X), m_Value(Y))))) {
+            AddAffected(X);
+            AddAffected(Y);
+          }
+        }
 
         // Handle icmp slt/sgt (bitcast X to int), 0/-1, which is supported
         // by computeKnownFPClass().
diff --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index 58c283815cf910..ad0c2c06eea91d 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -483,5 +483,251 @@ if.else:
   ret i64 13
 }
 
+define i1 @test_icmp_or_distjoint(i8 %n, i1 %other) {
+; CHECK-LABEL: @test_icmp_or_distjoint(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_OR:%.*]] = or disjoint i8 [[N:%.*]], 16
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_OR]], -111
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    ret i1 true
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i1 [[OTHER:%.*]]
+;
+entry:
+  %n_or = or disjoint i8 %n, 16
+  %cmp = icmp ugt i8 %n_or, 145
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = icmp slt i8 %n, 0
+  ret i1 %r
+
+if.else:
+  ret i1 %other
+}
+
+define i1 @test_icmp_or_fail_missing_disjoint(i8 %n, i1 %other) {
+; CHECK-LABEL: @test_icmp_or_fail_missing_disjoint(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_OR:%.*]] = or i8 [[N:%.*]], 16
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_OR]], -111
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[R:%.*]] = icmp slt i8 [[N]], 0
+; CHECK-NEXT:    ret i1 [[R]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i1 [[OTHER:%.*]]
+;
+entry:
+  %n_or = or i8 %n, 16
+  %cmp = icmp ugt i8 %n_or, 145
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = icmp slt i8 %n, 0
+  ret i1 %r
+
+if.else:
+  ret i1 %other
+}
+
+define i8 @test_icmp_or(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_or(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_OR:%.*]] = or i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i8 [[N_OR]], 32
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    ret i8 0
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+;
+entry:
+  %n_or = or i8 %n, %n2
+  %cmp = icmp ult i8 %n_or, 32
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+if.else:
+  ret i8 %other
+}
+
+define i8 @test_icmp_or2(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_or2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_OR:%.*]] = or i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_OR]], 14
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 0
+;
+entry:
+  %n_or = or i8 %n, %n2
+  %cmp = icmp uge i8 %n_or, 15
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  ret i8 %other
+if.else:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+}
+
+define i8 @test_icmp_or_fail_bad_range(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_or_fail_bad_range(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_OR:%.*]] = or i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i8 [[N_OR]], 33
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[R:%.*]] = and i8 [[N]], 32
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+;
+entry:
+  %n_or = or i8 %n, %n2
+  %cmp = icmp ule i8 %n_or, 32
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+if.else:
+  ret i8 %other
+}
+
+define i8 @test_icmp_or_fail_bad_pred(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_or_fail_bad_pred(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_OR:%.*]] = or i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_OR]], 32
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[R:%.*]] = and i8 [[N]], 32
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+;
+entry:
+  %n_or = or i8 %n, %n2
+  %cmp = icmp ugt i8 %n_or, 32
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+if.else:
+  ret i8 %other
+}
+
+define i8 @test_icmp_and(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_and(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_AND:%.*]] = and i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_AND]], -33
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    ret i8 32
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+;
+entry:
+  %n_and = and i8 %n, %n2
+  %cmp = icmp ugt i8 %n_and, 223
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+if.else:
+  ret i8 %other
+}
+
+define i8 @test_icmp_and2(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_and2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_AND:%.*]] = and i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i8 [[N_AND]], -31
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 32
+;
+entry:
+  %n_and = and i8 %n, %n2
+  %cmp = icmp ule i8 %n_and, 224
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  ret i8 %other
+if.else:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+}
+
+define i8 @test_icmp_and_fail_bad_range(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_and_fail_bad_range(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_AND:%.*]] = and i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i8 [[N_AND]], -34
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[R:%.*]] = and i8 [[N]], 32
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+;
+entry:
+  %n_and = and i8 %n, %n2
+  %cmp = icmp uge i8 %n_and, 223
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+if.else:
+  ret i8 %other
+}
+
+define i8 @test_icmp_and_fail_bad_pred(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_and_fail_bad_pred(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N_AND:%.*]] = and i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8 [[N_AND]], 32
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[R:%.*]] = and i8 [[N]], 32
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       if.else:
+; CHECK-NEXT:    ret i8 [[OTHER:%.*]]
+;
+entry:
+  %n_and = and i8 %n, %n2
+  %cmp = icmp eq i8 %n_and, 32
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %r = and i8 %n, 32
+  ret i8 %r
+
+if.else:
+  ret i8 %other
+}
+
+
 declare void @use(i1)
 declare void @sink(i8)

@goldsteinn goldsteinn changed the title goldsteinn/more or and support [ValueTracking] Improve knownbits from conditions where Val is used by and/or. Mar 21, 2024
@goldsteinn goldsteinn requested a review from dtcxzyw March 21, 2024 02:29
@dtcxzyw
Copy link
Member

dtcxzyw commented Mar 22, 2024

[ValueTracking] Add basic tests tracking or disjoint conditions as add; NFC
[ValueTracking] Tracking or disjoint conditions as add in Assumption/DomCondition Cache

I would like to file a separate PR for the above commits :)

@goldsteinn
Copy link
Contributor Author

[ValueTracking] Add basic tests tracking or disjoint conditions as add; NFC
[ValueTracking] Tracking or disjoint conditions as add in Assumption/DomCondition Cache

I would like to file a separate PR for the above commits :)

Done, see: #86302

Once that gets in ill rebase this one (merge conflicts otherwise).

@goldsteinn
Copy link
Contributor Author

Rebased

@goldsteinn
Copy link
Contributor Author

ping

…; NFC

`(icmp uge/ugt (and X, Y), C)` implies both `(icmp uge/ugt X, C)` and
`(icmp uge/ugt Y, C)`. We can use this to deduce leading ones in `X`.

`(icmp ule/ult (or X, Y), C)` implies both `(icmp ule/ult X, C)` and
`(icmp ule/ult Y, C)`. We can use this to deduce leading zeros in `X`.
@goldsteinn
Copy link
Contributor Author

rebased

@goldsteinn goldsteinn force-pushed the goldsteinn/more-or-and-support branch from 5d2bee5 to cba358c Compare April 5, 2024 01:26
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks ok to me. I'll leave it to @dtcxzyw to decide if we really need this. Diffs don't look particularly compelling to me.

@goldsteinn
Copy link
Contributor Author

The implementation looks ok to me. I'll leave it to @dtcxzyw to decide if we really need this. Diffs don't look particularly compelling to me.

Yeah I guess an empty diff isn't really all that exciting. For my money, if there is no-compile time impact, its better to have it covered than not, especially since I think there will end up being minimal code complexity added since the add nuw/sub nuw cases have a positive diff so if those get in, the impl of the two cases will overlap. But LMK either way.

@goldsteinn
Copy link
Contributor Author

@nikic, going to push this unless you have any objections.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants