[ValueTracking] Handle assume(trunc x to i1) in ComputeKnownBits #118406
+12
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-analysis Author: Andreas Jonson (andjo403) ChangesHandle assume( trunc x to i1) in ComputeKnownBits and KnownNonZero to avoid regressions when converting more icmp to trunc as the #84628 remove the canonicalization part of #118104 Full diff: https://github.com/llvm/llvm-project/pull/118406.diff 5 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c48068afc04816..7b7eea027c82b9 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -612,11 +612,16 @@ static bool isKnownNonZeroFromAssume(const Value *V, const SimplifyQuery &Q) {
// Warning: This loop can end up being somewhat performance sensitive.
// We're running this loop for once for each value queried resulting in a
// runtime of ~O(#assumes * #values).
+ Value *Arg = I->getArgOperand(0);
+ if (match(Arg, m_TruncOrSelf(m_Specific(V))) &&
+ isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+ return true;
+ }
Value *RHS;
CmpInst::Predicate Pred;
auto m_V = m_CombineOr(m_Specific(V), m_PtrToInt(m_Specific(V)));
- if (!match(I->getArgOperand(0), m_c_ICmp(Pred, m_V, m_Value(RHS))))
+ if (!match(Arg, m_c_ICmp(Pred, m_V, m_Value(RHS))))
continue;
if (cmpExcludesZero(Pred, RHS) && isValidAssumeForContext(I, Q.CxtI, Q.DT))
@@ -809,8 +814,6 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
if (!Q.AC)
return;
- unsigned BitWidth = Known.getBitWidth();
-
// Note that the patterns below need to be kept in sync with the code
// in AssumptionCache::updateAffectedValues.
@@ -844,17 +847,14 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
Value *Arg = I->getArgOperand(0);
- if (Arg == V && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
- assert(BitWidth == 1 && "assume operand is not i1?");
- (void)BitWidth;
- Known.setAllOnes();
+ if (match(Arg, m_TruncOrSelf(m_Specific(V))) &&
+ isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+ Known.One.setBit(0);
return;
}
- if (match(Arg, m_Not(m_Specific(V))) &&
+ if (match(Arg, m_Not(m_TruncOrSelf(m_Specific(V)))) &&
isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
- assert(BitWidth == 1 && "assume operand is not i1?");
- (void)BitWidth;
- Known.setAllZero();
+ Known.Zero.setBit(0);
return;
}
diff --git a/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll b/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
index 2aa95216a66569..fd9d2e4de61ba8 100644
--- a/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
+++ b/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
@@ -130,8 +130,6 @@ exit2:
ret i32 30
}
-; FIXME: We should be able to merge cont into do.
-; FIXME: COND should be replaced with false. This will be fixed by improving LVI.
define i32 @test4(i32 %i, i1 %f, i32 %n) {
; CHECK-LABEL: define {{[^@]+}}@test4
; CHECK-SAME: (i32 [[I:%.*]], i1 [[F:%.*]], i32 [[N:%.*]]) {
@@ -148,8 +146,7 @@ define i32 @test4(i32 %i, i1 %f, i32 %n) {
; CHECK-NEXT: call void @dummy(i1 [[F]]) #[[ATTR2]]
; CHECK-NEXT: [[CONSUME:%.*]] = call i32 @exit()
; CHECK-NEXT: call void @llvm.assume(i1 noundef [[F]])
-; CHECK-NEXT: [[COND:%.*]] = icmp eq i1 [[F]], false
-; CHECK-NEXT: br i1 [[COND]], label [[EXIT]], label [[CONT:%.*]]
+; CHECK-NEXT: br label [[CONT:%.*]]
; CHECK: exit2:
; CHECK-NEXT: ret i32 30
;
diff --git a/llvm/test/Transforms/InstSimplify/assume-non-zero.ll b/llvm/test/Transforms/InstSimplify/assume-non-zero.ll
index 9176b8101da659..331c9504f2da6f 100644
--- a/llvm/test/Transforms/InstSimplify/assume-non-zero.ll
+++ b/llvm/test/Transforms/InstSimplify/assume-non-zero.ll
@@ -231,3 +231,16 @@ define i1 @nonnull17_unknown(i8 %x) {
%q = icmp ne i8 %x, 0
ret i1 %q
}
+
+define i1 @nonnull_trunc_true(i8 %x) {
+; CHECK-LABEL: @nonnull_trunc_true(
+; CHECK-NEXT: [[A:%.*]] = trunc i8 [[X:%.*]] to i1
+; CHECK-NEXT: call void @llvm.assume(i1 [[A]])
+; CHECK-NEXT: ret i1 true
+;
+ %a = trunc i8 %x to i1
+ call void @llvm.assume(i1 %a)
+ %q = icmp ne i8 %x, 0
+ ret i1 %q
+}
+
diff --git a/llvm/test/Transforms/InstSimplify/compare.ll b/llvm/test/Transforms/InstSimplify/compare.ll
index 21653d800dce2d..a2a8fe42e7583b 100644
--- a/llvm/test/Transforms/InstSimplify/compare.ll
+++ b/llvm/test/Transforms/InstSimplify/compare.ll
@@ -3424,6 +3424,20 @@ define i1 @icmp_ult_vscale_false(i8 %x, i8 %y) {
ret i1 %cmp
}
+define i1 @icmp_eq_false_by_trunc(i8 %x) {
+; CHECK-LABEL: @icmp_eq_false_by_trunc(
+; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[X:%.*]] to i1
+; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[TRUNC]], true
+; CHECK-NEXT: call void @llvm.assume(i1 [[NOT]])
+; CHECK-NEXT: ret i1 false
+;
+ %trunc = trunc i8 %x to i1
+ %not = xor i1 %trunc, true
+ call void @llvm.assume(i1 %not)
+ %cmp = icmp eq i8 %x, 1
+ ret i1 %cmp
+}
+
declare i64 @llvm.vscale.i64()
; TODO: Add coverage for global aliases, link once, etc..
diff --git a/llvm/test/Transforms/InstSimplify/shr-nop.ll b/llvm/test/Transforms/InstSimplify/shr-nop.ll
index 29fe222faaae3e..b18a1c6d30e745 100644
--- a/llvm/test/Transforms/InstSimplify/shr-nop.ll
+++ b/llvm/test/Transforms/InstSimplify/shr-nop.ll
@@ -381,6 +381,18 @@ define i32 @exact_lshr_lowbit(i32 %shiftval) {
ret i32 %shr
}
+define i8 @exact_lshr_lowbit_set_assume_trunc(i8 %x) {
+; CHECK-LABEL: @exact_lshr_lowbit_set_assume_trunc(
+; CHECK-NEXT: [[COND:%.*]] = trunc i8 [[X:%.*]] to i1
+; CHECK-NEXT: call void @llvm.assume(i1 [[COND]])
+; CHECK-NEXT: ret i8 [[X]]
+;
+ %cond = trunc i8 %x to i1
+ call void @llvm.assume(i1 %cond)
+ %shr = lshr exact i8 %x, 1
+ ret i8 %shr
+}
+
define i32 @exact_ashr_lowbit(i32 %shiftval) {
; CHECK-LABEL: @exact_ashr_lowbit(
; CHECK-NEXT: ret i32 7
|
|
@llvm/pr-subscribers-llvm-transforms Author: Andreas Jonson (andjo403) ChangesHandle assume( trunc x to i1) in ComputeKnownBits and KnownNonZero to avoid regressions when converting more icmp to trunc as the #84628 remove the canonicalization part of #118104 Full diff: https://github.com/llvm/llvm-project/pull/118406.diff 5 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c48068afc04816..7b7eea027c82b9 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -612,11 +612,16 @@ static bool isKnownNonZeroFromAssume(const Value *V, const SimplifyQuery &Q) {
// Warning: This loop can end up being somewhat performance sensitive.
// We're running this loop for once for each value queried resulting in a
// runtime of ~O(#assumes * #values).
+ Value *Arg = I->getArgOperand(0);
+ if (match(Arg, m_TruncOrSelf(m_Specific(V))) &&
+ isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+ return true;
+ }
Value *RHS;
CmpInst::Predicate Pred;
auto m_V = m_CombineOr(m_Specific(V), m_PtrToInt(m_Specific(V)));
- if (!match(I->getArgOperand(0), m_c_ICmp(Pred, m_V, m_Value(RHS))))
+ if (!match(Arg, m_c_ICmp(Pred, m_V, m_Value(RHS))))
continue;
if (cmpExcludesZero(Pred, RHS) && isValidAssumeForContext(I, Q.CxtI, Q.DT))
@@ -809,8 +814,6 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
if (!Q.AC)
return;
- unsigned BitWidth = Known.getBitWidth();
-
// Note that the patterns below need to be kept in sync with the code
// in AssumptionCache::updateAffectedValues.
@@ -844,17 +847,14 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
Value *Arg = I->getArgOperand(0);
- if (Arg == V && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
- assert(BitWidth == 1 && "assume operand is not i1?");
- (void)BitWidth;
- Known.setAllOnes();
+ if (match(Arg, m_TruncOrSelf(m_Specific(V))) &&
+ isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+ Known.One.setBit(0);
return;
}
- if (match(Arg, m_Not(m_Specific(V))) &&
+ if (match(Arg, m_Not(m_TruncOrSelf(m_Specific(V)))) &&
isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
- assert(BitWidth == 1 && "assume operand is not i1?");
- (void)BitWidth;
- Known.setAllZero();
+ Known.Zero.setBit(0);
return;
}
diff --git a/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll b/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
index 2aa95216a66569..fd9d2e4de61ba8 100644
--- a/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
+++ b/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
@@ -130,8 +130,6 @@ exit2:
ret i32 30
}
-; FIXME: We should be able to merge cont into do.
-; FIXME: COND should be replaced with false. This will be fixed by improving LVI.
define i32 @test4(i32 %i, i1 %f, i32 %n) {
; CHECK-LABEL: define {{[^@]+}}@test4
; CHECK-SAME: (i32 [[I:%.*]], i1 [[F:%.*]], i32 [[N:%.*]]) {
@@ -148,8 +146,7 @@ define i32 @test4(i32 %i, i1 %f, i32 %n) {
; CHECK-NEXT: call void @dummy(i1 [[F]]) #[[ATTR2]]
; CHECK-NEXT: [[CONSUME:%.*]] = call i32 @exit()
; CHECK-NEXT: call void @llvm.assume(i1 noundef [[F]])
-; CHECK-NEXT: [[COND:%.*]] = icmp eq i1 [[F]], false
-; CHECK-NEXT: br i1 [[COND]], label [[EXIT]], label [[CONT:%.*]]
+; CHECK-NEXT: br label [[CONT:%.*]]
; CHECK: exit2:
; CHECK-NEXT: ret i32 30
;
diff --git a/llvm/test/Transforms/InstSimplify/assume-non-zero.ll b/llvm/test/Transforms/InstSimplify/assume-non-zero.ll
index 9176b8101da659..331c9504f2da6f 100644
--- a/llvm/test/Transforms/InstSimplify/assume-non-zero.ll
+++ b/llvm/test/Transforms/InstSimplify/assume-non-zero.ll
@@ -231,3 +231,16 @@ define i1 @nonnull17_unknown(i8 %x) {
%q = icmp ne i8 %x, 0
ret i1 %q
}
+
+define i1 @nonnull_trunc_true(i8 %x) {
+; CHECK-LABEL: @nonnull_trunc_true(
+; CHECK-NEXT: [[A:%.*]] = trunc i8 [[X:%.*]] to i1
+; CHECK-NEXT: call void @llvm.assume(i1 [[A]])
+; CHECK-NEXT: ret i1 true
+;
+ %a = trunc i8 %x to i1
+ call void @llvm.assume(i1 %a)
+ %q = icmp ne i8 %x, 0
+ ret i1 %q
+}
+
diff --git a/llvm/test/Transforms/InstSimplify/compare.ll b/llvm/test/Transforms/InstSimplify/compare.ll
index 21653d800dce2d..a2a8fe42e7583b 100644
--- a/llvm/test/Transforms/InstSimplify/compare.ll
+++ b/llvm/test/Transforms/InstSimplify/compare.ll
@@ -3424,6 +3424,20 @@ define i1 @icmp_ult_vscale_false(i8 %x, i8 %y) {
ret i1 %cmp
}
+define i1 @icmp_eq_false_by_trunc(i8 %x) {
+; CHECK-LABEL: @icmp_eq_false_by_trunc(
+; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[X:%.*]] to i1
+; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[TRUNC]], true
+; CHECK-NEXT: call void @llvm.assume(i1 [[NOT]])
+; CHECK-NEXT: ret i1 false
+;
+ %trunc = trunc i8 %x to i1
+ %not = xor i1 %trunc, true
+ call void @llvm.assume(i1 %not)
+ %cmp = icmp eq i8 %x, 1
+ ret i1 %cmp
+}
+
declare i64 @llvm.vscale.i64()
; TODO: Add coverage for global aliases, link once, etc..
diff --git a/llvm/test/Transforms/InstSimplify/shr-nop.ll b/llvm/test/Transforms/InstSimplify/shr-nop.ll
index 29fe222faaae3e..b18a1c6d30e745 100644
--- a/llvm/test/Transforms/InstSimplify/shr-nop.ll
+++ b/llvm/test/Transforms/InstSimplify/shr-nop.ll
@@ -381,6 +381,18 @@ define i32 @exact_lshr_lowbit(i32 %shiftval) {
ret i32 %shr
}
+define i8 @exact_lshr_lowbit_set_assume_trunc(i8 %x) {
+; CHECK-LABEL: @exact_lshr_lowbit_set_assume_trunc(
+; CHECK-NEXT: [[COND:%.*]] = trunc i8 [[X:%.*]] to i1
+; CHECK-NEXT: call void @llvm.assume(i1 [[COND]])
+; CHECK-NEXT: ret i8 [[X]]
+;
+ %cond = trunc i8 %x to i1
+ call void @llvm.assume(i1 %cond)
+ %shr = lshr exact i8 %x, 1
+ ret i8 %shr
+}
+
define i32 @exact_ashr_lowbit(i32 %shiftval) {
; CHECK-LABEL: @exact_ashr_lowbit(
; CHECK-NEXT: ret i32 7
|
This was referenced Dec 3, 2024
dtcxzyw
reviewed
Dec 3, 2024
llvm/lib/Analysis/ValueTracking.cpp
Outdated
| @@ -612,11 +612,16 @@ static bool isKnownNonZeroFromAssume(const Value *V, const SimplifyQuery &Q) { | |||
| // Warning: This loop can end up being somewhat performance sensitive. | |||
| // We're running this loop for once for each value queried resulting in a | |||
| // runtime of ~O(#assumes * #values). | |||
| Value *Arg = I->getArgOperand(0); | |||
| if (match(Arg, m_TruncOrSelf(m_Specific(V))) && | |||
| isValidAssumeForContext(I, Q.CxtI, Q.DT)) { | |||
There was a problem hiding this comment.
Looks like we hit another "ephemeral value" issue :(
There was a problem hiding this comment.
I do not know what issue you are thinking about is it something that is shown in the llvm-opt-benchmark?
There was a problem hiding this comment.
Yeah. The test diff shows it only removes some (unused?) assumptions.
There was a problem hiding this comment.
I do not know if there is that many assumptions like this right now but was looking at folding some icmp eq X, C to trunc X to i1 when X is known to be 0 or 1 and got lots of regressions due to this.
|
even if there maybe is problems with ephemeral values resulting in not all assumes is used, |
|
The problem is not that the assume isn't used, it's that the assume is used to simplify itself, removing the assume. |
andjo403
commented
Dec 11, 2024
andjo403
added a commit
that referenced
this pull request
Feb 4, 2025
andjo403
force-pushed
the
truncAssumes
branch
from
a32a721 to
59efb40
Compare
|
Have removed the problematic fold, so this shall not have the "ephemeral value" issue anymore |
goldsteinn
reviewed
Feb 4, 2025
goldsteinn
reviewed
Feb 9, 2025
|
Have rebased and fixed the comments so this is ready for a new review. |
dtcxzyw
reviewed
Apr 9, 2025
llvm/lib/Analysis/ValueTracking.cpp
Outdated
| @@ -643,11 +643,16 @@ static bool isKnownNonZeroFromAssume(const Value *V, const SimplifyQuery &Q) { | |||
| // Warning: This loop can end up being somewhat performance sensitive. | |||
| // We're running this loop for once for each value queried resulting in a | |||
| // runtime of ~O(#assumes * #values). | |||
| Value *Arg = I->getArgOperand(0); | |||
There was a problem hiding this comment.
Which test covers this change? InstSimplify can fold the zero check in nonnull_trunc_true even without the change to isKnownNonZeroFromAssume.
There was a problem hiding this comment.
I'm not sure I understand if I remove this code the tests nonnull_trunc_true and test4 fails
There was a problem hiding this comment.
However, InstCombine can simplify both of them after I removed the change to isKnownNonZeroFromAssume.
(the original tests check InstSimplify/LVI instead).
Can you please spill isKnownNonZeroFromAssume changes into a separate PR? Sorry for the inconvenience.
There was a problem hiding this comment.
sure I can split this PR to the two commits.
andjo403
changed the title
|
@dtcxzyw this PR is only the ComputeKnownBits commit now |
dtcxzyw
changed the title
dtcxzyw
approved these changes
Apr 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Handle assume( trunc x to i1) in ComputeKnownBits to avoid regressions when converting more icmp to trunc as the #84628 remove the canonicalization
trunc x to i1 -> icmp ne (and x, 1), 0proof: https://alive2.llvm.org/ce/z/zAspzb