-
Notifications
You must be signed in to change notification settings - Fork 14.2k
[InstCombine] Use m_NotForbidPoison
when folding (X u< Y) ? -1 : (~X + Y) --> uadd.sat(~X, Y)
#114345
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
Conversation
…~X + Y) --> uadd.sat(~X, Y)`
@llvm/pr-subscribers-llvm-transforms Author: Yingwei Zheng (dtcxzyw) ChangesAlive2: https://alive2.llvm.org/ce/z/mTGCo- Fixes #113869. Full diff: https://github.com/llvm/llvm-project/pull/114345.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index c5f39a4c381ed1..b442e79377740d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1051,7 +1051,7 @@ static Value *canonicalizeSaturatedAdd(ICmpInst *Cmp, Value *TVal, Value *FVal,
// Strictness of the comparison is irrelevant.
X = Cmp0;
Y = Cmp1;
- if (match(FVal, m_c_Add(m_Not(m_Specific(X)), m_Specific(Y)))) {
+ if (match(FVal, m_c_Add(m_NotForbidPoison(m_Specific(X)), m_Specific(Y)))) {
// (X u< Y) ? -1 : (~X + Y) --> uadd.sat(~X, Y)
// (X u< Y) ? -1 : (Y + ~X) --> uadd.sat(Y, ~X)
BinaryOperator *BO = cast<BinaryOperator>(FVal);
diff --git a/llvm/test/Transforms/InstCombine/saturating-add-sub.ll b/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
index a88fd3cc21f1bc..af8a9314a08049 100644
--- a/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
+++ b/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
@@ -1890,6 +1890,21 @@ define <2 x i32> @uadd_sat_not_ugt_commute_add(<2 x i32> %x, <2 x i32> %yp) {
ret <2 x i32> %r
}
+define <2 x i32> @uadd_sat_not_ugt_commute_add_partial_poison(<2 x i32> %x, <2 x i32> %yp) {
+; CHECK-LABEL: @uadd_sat_not_ugt_commute_add_partial_poison(
+; CHECK-NEXT: [[NOTX:%.*]] = xor <2 x i32> [[X:%.*]], <i32 -1, i32 poison>
+; CHECK-NEXT: [[A:%.*]] = add nuw <2 x i32> [[YP:%.*]], [[NOTX]]
+; CHECK-NEXT: [[C:%.*]] = icmp ugt <2 x i32> [[YP]], [[X]]
+; CHECK-NEXT: [[R:%.*]] = select <2 x i1> [[C]], <2 x i32> <i32 -1, i32 -1>, <2 x i32> [[A]]
+; CHECK-NEXT: ret <2 x i32> [[R]]
+;
+ %notx = xor <2 x i32> %x, <i32 -1, i32 poison>
+ %a = add nuw <2 x i32> %yp, %notx
+ %c = icmp ugt <2 x i32> %yp, %x
+ %r = select <2 x i1> %c, <2 x i32> <i32 -1, i32 -1>, <2 x i32> %a
+ ret <2 x i32> %r
+}
+
define i32 @uadd_sat_not_commute_select(i32 %x, i32 %y) {
; CHECK-LABEL: @uadd_sat_not_commute_select(
; CHECK-NEXT: [[NOTX:%.*]] = xor i32 [[X:%.*]], -1
|
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 although I have trouble believing this is the only place we have this type of bug.
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
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/478 Here is the relevant piece of the build log for the reference
|
…~X + Y) --> uadd.sat(~X, Y)` (llvm#114345) Alive2: https://alive2.llvm.org/ce/z/mTGCo- We cannot reuse `~X` if `m_AllOnes` matches a vector constant with some poison elts. An alternative solution is to create a new not instead of reusing `~X`. But it doesn't worth the effort because we need to add a one-use check. Fixes llvm#113869.
…~X + Y) --> uadd.sat(~X, Y)` (llvm#114345) Alive2: https://alive2.llvm.org/ce/z/mTGCo- We cannot reuse `~X` if `m_AllOnes` matches a vector constant with some poison elts. An alternative solution is to create a new not instead of reusing `~X`. But it doesn't worth the effort because we need to add a one-use check. Fixes llvm#113869.
Alive2: https://alive2.llvm.org/ce/z/mTGCo-
We cannot reuse
~X
ifm_AllOnes
matches a vector constant with some poison elts. An alternative solution is to create a new not instead of reusing~X
. But it doesn't worth the effort because we need to add a one-use check.Fixes #113869.