llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

[ValueTracking] Track `or disjoint` conditions as add in Assumption/DomCondition Cache

Open goldsteinn opened this issue 1 year ago • 2 comments

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

goldsteinn avatar Mar 22 '24 16:03 goldsteinn

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

llvmbot avatar Mar 22 '24 16:03 llvmbot

According to the test result, I'd like to merge the remaining part of https://github.com/llvm/llvm-project/pull/86059 first.

dtcxzyw avatar Mar 24 '24 10:03 dtcxzyw

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 avatar Mar 24 '24 19:03 goldsteinn

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?

goldsteinn avatar Mar 27 '24 18:03 goldsteinn