[LVI] Add trunc to i1 handling. #124480
Merged
[LVI] Add trunc to i1 handling. #124480
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
llvmbot
added
llvm:analysis
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Andreas Jonson (andjo403) ChangesThink that it is possible to also handle vectors but have not been able to create test for that. Proof: https://alive2.llvm.org/ce/z/fTPEg6 there is one proof that fails but think that it is a fault in alive AliveToolkit/alive2#1164 as the equivalent code with Full diff: https://github.com/llvm/llvm-project/pull/124480.diff 3 Files Affected:
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 20f69a0955f51c..05c66457fdaa3e 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -398,6 +398,8 @@ class LazyValueInfoImpl {
std::optional<ValueLatticeElement>
getValueFromICmpCondition(Value *Val, ICmpInst *ICI, bool isTrueDest,
bool UseBlockValue);
+ std::optional<ValueLatticeElement>
+ getValueFromTrunc(Value *Val, TruncInst *Trunc, bool IsTrueDest);
std::optional<ValueLatticeElement>
getValueFromCondition(Value *Val, Value *Cond, bool IsTrueDest,
@@ -1283,6 +1285,20 @@ std::optional<ValueLatticeElement> LazyValueInfoImpl::getValueFromICmpCondition(
return ValueLatticeElement::getOverdefined();
}
+std::optional<ValueLatticeElement>
+LazyValueInfoImpl::getValueFromTrunc(Value *Val, TruncInst *Trunc,
+ bool IsTrueDest) {
+ assert(Trunc->getType()->isIntegerTy(1));
+
+ Type *Ty = Val->getType();
+ if (!Ty->isIntegerTy() || Trunc->getOperand(0) != Val)
+ return ValueLatticeElement::getOverdefined();
+
+ if (IsTrueDest)
+ return ValueLatticeElement::getNot(Constant::getNullValue(Ty));
+ return ValueLatticeElement::getNot(Constant::getAllOnesValue(Ty));
+}
+
// Handle conditions of the form
// extractvalue(op.with.overflow(%x, C), 1).
static ValueLatticeElement getValueFromOverflowCondition(
@@ -1312,6 +1328,9 @@ LazyValueInfoImpl::getValueFromCondition(Value *Val, Value *Cond,
if (ICmpInst *ICI = dyn_cast<ICmpInst>(Cond))
return getValueFromICmpCondition(Val, ICI, IsTrueDest, UseBlockValue);
+ if (auto *Trunc = dyn_cast<TruncInst>(Cond))
+ return getValueFromTrunc(Val, Trunc, IsTrueDest);
+
if (auto *EVI = dyn_cast<ExtractValueInst>(Cond))
if (auto *WO = dyn_cast<WithOverflowInst>(EVI->getAggregateOperand()))
if (EVI->getNumIndices() == 1 && *EVI->idx_begin() == 1)
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 8e74b8645fad9a..107900cdfdc5d3 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -348,6 +348,21 @@ static bool processCmp(CmpInst *Cmp, LazyValueInfo *LVI) {
return false;
}
+static bool processTrunc(TruncInst *Trunc, LazyValueInfo *LVI) {
+ Type *Ty = Trunc->getType();
+ if (!Ty->isIntegerTy(1))
+ return false;
+
+ Value *Arg = Trunc->getOperand(0);
+ ConstantRange Res = LVI->getConstantRange(Arg, Trunc, false);
+ if (Res.contains(APInt::getZero(Arg->getType()->getScalarSizeInBits())))
+ return false;
+
+ Trunc->replaceAllUsesWith(ConstantInt::getTrue(Ty));
+ Trunc->eraseFromParent();
+ return true;
+}
+
/// Simplify a switch instruction by removing cases which can never fire. If the
/// uselessness of a case could be determined locally then constant propagation
/// would already have figured it out. Instead, walk the predecessors and
@@ -1239,6 +1254,9 @@ static bool runImpl(Function &F, LazyValueInfo *LVI, DominatorTree *DT,
case Instruction::FCmp:
BBChanged |= processCmp(cast<CmpInst>(&II), LVI);
break;
+ case Instruction::Trunc:
+ BBChanged |= processTrunc(cast<TruncInst>(&II), LVI);
+ break;
case Instruction::Call:
case Instruction::Invoke:
BBChanged |= processCallSite(cast<CallBase>(II), LVI);
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/icmp.ll b/llvm/test/Transforms/CorrelatedValuePropagation/icmp.ll
index e4de34c339d2de..1d6ae86cc272e6 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/icmp.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/icmp.ll
@@ -1515,10 +1515,8 @@ define void @test_trunc_bittest(i8 %a) {
; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[A:%.*]] to i1
; CHECK-NEXT: br i1 [[TRUNC]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]]
; CHECK: if.true:
-; CHECK-NEXT: [[CMP1:%.*]] = icmp ne i8 [[A]], 0
-; CHECK-NEXT: call void @check1(i1 [[CMP1]])
-; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i8 [[A]], 0
-; CHECK-NEXT: call void @check1(i1 [[CMP2]])
+; CHECK-NEXT: call void @check1(i1 true)
+; CHECK-NEXT: call void @check1(i1 false)
; CHECK-NEXT: ret void
; CHECK: if.false:
; CHECK-NEXT: ret void
@@ -1543,10 +1541,8 @@ define void @test_trunc_not_bittest(i8 %a) {
; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[TRUNC]], true
; CHECK-NEXT: br i1 [[NOT]], label [[IF_FALSE:%.*]], label [[IF_TRUE:%.*]]
; CHECK: if.true:
-; CHECK-NEXT: [[CMP1:%.*]] = icmp ne i8 [[A]], -1
-; CHECK-NEXT: call void @check1(i1 [[CMP1]])
-; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i8 [[A]], -1
-; CHECK-NEXT: call void @check1(i1 [[CMP2]])
+; CHECK-NEXT: call void @check1(i1 true)
+; CHECK-NEXT: call void @check1(i1 false)
; CHECK-NEXT: ret void
; CHECK: if.false:
; CHECK-NEXT: ret void
@@ -1571,8 +1567,7 @@ define void @test_icmp_trunc(i8 %a) {
; CHECK-NEXT: [[CMP1:%.*]] = icmp ne i8 [[A:%.*]], 0
; CHECK-NEXT: br i1 [[CMP1]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]]
; CHECK: if.true:
-; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[A]] to i1
-; CHECK-NEXT: call void @check1(i1 [[TRUNC]])
+; CHECK-NEXT: call void @check1(i1 true)
; CHECK-NEXT: ret void
; CHECK: if.false:
; CHECK-NEXT: ret void
@@ -1594,9 +1589,8 @@ define void @test_icmp_trunc_not(i8 %a) {
; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i8 [[A:%.*]], -1
; CHECK-NEXT: br i1 [[CMP1]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]]
; CHECK: if.true:
-; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[A]] to i1
-; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[TRUNC]], true
-; CHECK-NEXT: call void @check1(i1 [[TRUNC]])
+; CHECK-NEXT: [[NOT:%.*]] = xor i1 true, true
+; CHECK-NEXT: call void @check1(i1 true)
; CHECK-NEXT: ret void
; CHECK: if.false:
; CHECK-NEXT: ret void
|
andjo403
force-pushed
the
lazyValueInfoTrunci1
branch
2 times, most recently
from
c6999d6 to
137baab
Compare
|
fixed a fault in a test |
nunoplopes
reviewed
Jan 26, 2025
andjo403
force-pushed
the
lazyValueInfoTrunci1
branch
from
137baab to
71da85a
Compare
andjo403
changed the title
andjo403
changed the title
andjo403
force-pushed
the
lazyValueInfoTrunci1
branch
from
71da85a to
2947c0a
Compare
This was referenced Jan 29, 2025
nikic
reviewed
Jan 29, 2025
llvm/lib/Analysis/LazyValueInfo.cpp
Outdated
| std::optional<ValueLatticeElement> | ||
| LazyValueInfoImpl::getValueFromTrunc(Value *Val, TruncInst *Trunc, | ||
| bool IsTrueDest) { | ||
| assert(Trunc->getType()->isIntegerTy(1)); |
There was a problem hiding this comment.
Could you please add a test with a vector select that has a trunc condition? I think it will fail this assertion. This should be isIntOrIntVectorTy.
There was a problem hiding this comment.
assert fixed and test added see and_elide_trunc_cond_vec
llvm/lib/Analysis/LazyValueInfo.cpp
Outdated
| assert(Trunc->getType()->isIntegerTy(1)); | ||
|
|
||
| Type *Ty = Val->getType(); | ||
| if (!Ty->isIntegerTy() || Trunc->getOperand(0) != Val) |
There was a problem hiding this comment.
The type check here looks redundant.
llvm/lib/Analysis/LazyValueInfo.cpp
Outdated
| @@ -1283,6 +1285,20 @@ std::optional<ValueLatticeElement> LazyValueInfoImpl::getValueFromICmpCondition( | |||
| return ValueLatticeElement::getOverdefined(); | |||
| } | |||
|
|
|||
| std::optional<ValueLatticeElement> | |||
There was a problem hiding this comment.
I don't think you need std::optional? You always return a lattice value.
|
|
||
| if (IsTrueDest) | ||
| return ValueLatticeElement::getNot(Constant::getNullValue(Ty)); | ||
| return ValueLatticeElement::getNot(Constant::getAllOnesValue(Ty)); |
There was a problem hiding this comment.
We could further improve these by taking the nowrap flags into account. I think in a lot of cases we'll have trunc nuw %x to i1, in which case if (trunc nuw %x to i1) implies %x == 1, not just %x != 0.
There was a problem hiding this comment.
added handling for nuw and update the proof
| ; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i8 [[A]], 0 | ||
| ; CHECK-NEXT: call void @check1(i1 [[CMP2]]) | ||
| ; CHECK-NEXT: call void @check1(i1 true) | ||
| ; CHECK-NEXT: call void @check1(i1 false) |
There was a problem hiding this comment.
We should also have negative tests, e.g. to show that it does not imply == 1.
andjo403
added a commit
that referenced
this pull request
Jan 31, 2025
andjo403
force-pushed
the
lazyValueInfoTrunci1
branch
from
2947c0a to
abe86fb
Compare
This was referenced Feb 1, 2025
nikic
reviewed
Feb 1, 2025
| if (IsTrueDest) | ||
| return ValueLatticeElement::get( | ||
| Constant::getIntegerValue(Ty, APInt(Ty->getScalarSizeInBits(), 1))); | ||
| return ValueLatticeElement::get(Constant::getNullValue(Ty)); |
There was a problem hiding this comment.
You can use ConstantInt::getBool() here.
There was a problem hiding this comment.
I do not think it is not possible to use ConstantInt::getBool() as the type of val is not i1.
it is possible to swap the true case to ConstantInt::get(Ty, 1)
There was a problem hiding this comment.
You're right, I got confused with the types here. And yes, using ConstantInt::get is preferable. (getIntegerValue is a specialized API if you need to potentially create an inttoptr expression...)
andjo403
force-pushed
the
lazyValueInfoTrunci1
branch
from
4664e01 to
a35e1a9
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12894 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/13956 Here is the relevant piece of the build log for the reference |
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.
Proof: https://alive2.llvm.org/ce/z/yPrRp-