-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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] Compute knownbits from (icmp upred (add/sub nuw X, Y), C)
#87180
Conversation
(icmp ult/ule (add nuw X, Y), C)
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: None (goldsteinn) Changes
Based on: #87180 Full diff: https://github.com/llvm/llvm-project/pull/87180.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index b5e8a1d22f264b..5bc2d605d3a1b1 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -698,13 +698,19 @@ static void computeKnownBitsFromCmp(const Value *V, CmpInst::Predicate Pred,
break;
}
default:
- const APInt *Offset = nullptr;
- 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)
- 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());
+ }
+ if ((Pred == ICmpInst::ICMP_ULE || Pred == ICmpInst::ICMP_ULT) &&
+ match(LHS, m_c_NUWAdd(m_V, m_Value()))) {
+ Known.Zero.setHighBits(
+ (*C - (Pred == ICmpInst::ICMP_ULT)).countLeadingZeros());
+ }
}
break;
}
@@ -9283,11 +9289,21 @@ 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_AddLike(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_NUWAdd(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 5305c78f691231..0ed63512f8cb49 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -124,7 +124,6 @@ exit:
ret i8 %or2
}
-
define i8 @test_cond_and_bothways(i8 %x) {
; CHECK-LABEL: @test_cond_and_bothways(
; CHECK-NEXT: [[AND:%.*]] = and i8 [[X:%.*]], 91
@@ -181,8 +180,6 @@ exit:
ret i8 %or2
}
-
-
define i8 @test_cond_and_commuted(i8 %x, i1 %c1, i1 %c2) {
; CHECK-LABEL: @test_cond_and_commuted(
; CHECK-NEXT: [[AND:%.*]] = and i8 [[X:%.*]], 3
@@ -343,7 +340,7 @@ exit:
ret i8 %or2
}
-define i32 @test_icmp_trunc1(i32 %x){
+define i32 @test_icmp_trunc1(i32 %x) {
; CHECK-LABEL: @test_icmp_trunc1(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[Y:%.*]] = trunc i32 [[X:%.*]] to i16
@@ -365,7 +362,7 @@ else:
ret i32 0
}
-define i32 @test_icmp_trunc_assume(i32 %x){
+define i32 @test_icmp_trunc_assume(i32 %x) {
; CHECK-LABEL: @test_icmp_trunc_assume(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[Y:%.*]] = trunc i32 [[X:%.*]] to i16
@@ -532,7 +529,103 @@ if.else:
ret i1 %other
}
+define i8 @test_icmp_add(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_add(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[N_ADD:%.*]] = add nuw i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[N_ADD]], 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_add = add nuw i8 %n, %n2
+ %cmp = icmp ult i8 %n_add, 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_add2(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_add2(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[N_ADD:%.*]] = add nuw i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[N_ADD]], 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_add = add nuw i8 %n, %n2
+ %cmp = icmp uge i8 %n_add, 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_add_fail_bad_range(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_add_fail_bad_range(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[N_ADD:%.*]] = add nuw i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[N_ADD]], 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_add = add nuw i8 %n, %n2
+ %cmp = icmp ule i8 %n_add, 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_add_fail_bad_pred(i8 %n, i8 %n2, i8 %other) {
+; CHECK-LABEL: @test_icmp_add_fail_bad_pred(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[N_ADD:%.*]] = add nuw i8 [[N:%.*]], [[N2:%.*]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[N_ADD]], 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_add = add nuw i8 %n, %n2
+ %cmp = icmp ugt i8 %n_add, 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)
|
0dd2548
to
0341e89
Compare
Can you have a look at the regression dtcxzyw/llvm-opt-benchmark#465 (comment)? |
Okay, reduced test:
The issue occurs due to the InstCombine fold at: We produce: Bad:
vs
The issue then arises because we fold Ill add a patch for the |
Two folds unlocked: `(icmp eq/ne (xor x, C0), C1)` -> `(icmp eq/ne x, C2)` `(icmp eq/ne (xor x, y), 0)` -> `(icmp eq/ne x, y)` This fixes regressions assosiated with llvm#87180
Two folds unlocked: `(icmp eq/ne (xor x, C0), C1)` -> `(icmp eq/ne x, C2)` `(icmp eq/ne (xor x, y), 0)` -> `(icmp eq/ne x, y)` This fixes regressions assosiated with llvm#87180
My preference would be to not have this blocked by the missing multi-use xor fold. It seems pretty unrelated that we end up with that pattern w/ this patch but not w/ (in that one case) and handling multi-use xor seems independently valuable. |
Two folds unlocked: `(icmp eq/ne (xor x, C0), C1)` -> `(icmp eq/ne x, C2)` `(icmp eq/ne (xor x, y), 0)` -> `(icmp eq/ne x, y)` This fixes regressions assosiated with llvm#87180
(icmp ult/ule (add nuw X, Y), C)
(icmp upred (add/sub nuw X, Y), C)
0341e89
to
6d6bb29
Compare
@dtcxzyw i update with support for |
6d6bb29
to
7d54e14
Compare
rebased |
Needs another rebase.
Why? |
I guess the |
7d54e14
to
88fa1fa
Compare
ping |
1 similar comment
ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
if.else: | ||
ret i8 %other | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Negative tests without nuw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done + rebased/
…add/sub nuw X, Y), C)`; NFC
…), C)` `(icmp ule/ult (add nuw 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`/`Y`. `(icmp uge/ugt (sub nuw X, Y), C)` implies `(icmp uge/uge X, C)` . We can use this to deduce leading ones in `X`. Proofs: https://alive2.llvm.org/ce/z/sc5k22
88fa1fa
to
1adbd10
Compare
Two folds unlocked: `(icmp eq/ne (xor x, C0), C1)` -> `(icmp eq/ne x, C2)` `(icmp eq/ne (xor x, y), 0)` -> `(icmp eq/ne x, y)` This fixes regressions assosiated with llvm#87180
[ValueTracking] Add tests for computing knownbits from
(icmp upred (add/sub nuw X, Y), C)
; NFC[ValueTracking] Compute knownbits from
(icmp upred (add/sub nuw X, Y), C)
(icmp ule/ult (add nuw X, Y), C)
implies both(icmp ule/ult X, C)
and(icmp ule/ult Y, C)
. We can use this to deduce leading zeros inX
/Y
.(icmp uge/ugt (sub nuw X, Y), C)
implies(icmp uge/uge X, C)
. Wecan use this to deduce leading ones in
X
.Proofs: https://alive2.llvm.org/ce/z/sc5k22