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] Track or disjoint conditions as add in Assumption/DomCondition Cache #86302

Closed
wants to merge 2 commits into from

Conversation

goldsteinn
Copy link
Contributor

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

…ion/DomCondition Cache

We can definitionally treat `or disjoint` as `add` anywhere.
@goldsteinn goldsteinn requested a review from nikic as a code owner March 22, 2024 16:09
@goldsteinn goldsteinn requested a review from dtcxzyw March 22, 2024 16:09
@goldsteinn goldsteinn changed the title goldsteinn/track disjoint or [ValueTracking] Track or disjoint conditions as add in Assumption/DomCondition Cache Mar 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

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

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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+2-2)
  • (modified) llvm/test/Transforms/InstCombine/known-bits.ll (+51)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 797665cf06c875..b5e8a1d22f264b 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -699,7 +699,7 @@ static void computeKnownBitsFromCmp(const Value *V, CmpInst::Predicate Pred,
   }
   default:
     const APInt *Offset = nullptr;
-    if (match(LHS, m_CombineOr(m_V, m_Add(m_V, m_APInt(Offset)))) &&
+    if (match(LHS, m_CombineOr(m_V, m_AddLike(m_V, m_APInt(Offset)))) &&
         match(RHS, m_APInt(C))) {
       ConstantRange LHSRange = ConstantRange::makeAllowedICmpRegion(Pred, *C);
       if (Offset)
@@ -9285,7 +9285,7 @@ void llvm::findValuesAffectedByCondition(
       } 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())) &&
+        if (match(A, m_AddLike(m_Value(X), m_ConstantInt())) &&
             match(B, m_ConstantInt()))
           AddAffected(X);
 
diff --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index 58c283815cf910..5305c78f691231 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -483,5 +483,56 @@ 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
+}
+
+
+
 declare void @use(i1)
 declare void @sink(i8)

@dtcxzyw
Copy link
Member

dtcxzyw commented Mar 24, 2024

According to the test result, I'd like to merge the remaining part of #86059 first.

@goldsteinn
Copy link
Contributor Author

According to the test result, I'd like to merge the remaining part of #86059 first.

This one isn't really meant to be impactful, imo its just cleanup to use addlike instead of add in as many places as we can. Think there is very little risk involved.

@goldsteinn
Copy link
Contributor Author

According to the test result, I'd like to merge the remaining part of #86059 first.

This one isn't really meant to be impactful, imo its just cleanup to use addlike instead of add in as many places as we can. Think there is very little risk involved.

That okay?

Copy link
Member

@dtcxzyw dtcxzyw 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

3 participants