-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[InstCombine] When -A + B both have nsw flag, set nsw flag. #72127
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
@llvm/pr-subscribers-llvm-transforms Author: None (Z572) ChangesFixes #72119 https://alive2.llvm.org/ce/z/5f_QuC Full diff: https://github.com/llvm/llvm-project/pull/72127.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 22fd3edc39acb03..9ff1a57b6be93ba 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1487,7 +1487,11 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
return BinaryOperator::CreateNeg(Builder.CreateAdd(A, B));
// -A + B --> B - A
- return BinaryOperator::CreateSub(RHS, A);
+ auto *Sub = BinaryOperator::CreateSub(RHS, A);
+ if (auto *Op1 = dyn_cast<BinaryOperator>(LHS)) {
+ Sub->setHasNoSignedWrap(I.hasNoSignedWrap() && Op1->hasNoSignedWrap());
+ }
+ return Sub;
}
// A + -B --> A - B
diff --git a/llvm/test/Transforms/InstCombine/add.ll b/llvm/test/Transforms/InstCombine/add.ll
index 7234fb39e566ebf..fff839c2d89a138 100644
--- a/llvm/test/Transforms/InstCombine/add.ll
+++ b/llvm/test/Transforms/InstCombine/add.ll
@@ -120,6 +120,36 @@ define i32 @test5(i32 %A, i32 %B) {
ret i32 %D
}
+define i32 @test5_2(i32 %A, i32 %B) {
+; CHECK-LABEL: @test5_2(
+; CHECK-NEXT: [[D:%.*]] = sub nsw i32 [[B:%.*]], [[A:%.*]]
+; CHECK-NEXT: ret i32 [[D]]
+;
+ %C = sub nsw i32 0, %A
+ %D = add nsw i32 %C, %B
+ ret i32 %D
+}
+
+define i32 @test5_3(i32 %A, i32 %B) {
+; CHECK-LABEL: @test5_3(
+; CHECK-NEXT: [[D:%.*]] = sub i32 [[B:%.*]], [[A:%.*]]
+; CHECK-NEXT: ret i32 [[D]]
+;
+ %C = sub nsw i32 0, %A
+ %D = add i32 %C, %B
+ ret i32 %D
+}
+
+define i32 @test5_4(i32 %A, i32 %B) {
+; CHECK-LABEL: @test5_4(
+; CHECK-NEXT: [[D:%.*]] = sub i32 [[B:%.*]], [[A:%.*]]
+; CHECK-NEXT: ret i32 [[D]]
+;
+ %C = sub i32 0, %A
+ %D = add nsw i32 %C, %B
+ ret i32 %D
+}
+
define <2 x i8> @neg_op0_vec_undef_elt(<2 x i8> %a, <2 x i8> %b) {
; CHECK-LABEL: @neg_op0_vec_undef_elt(
; CHECK-NEXT: [[R:%.*]] = sub <2 x i8> [[B:%.*]], [[A:%.*]]
diff --git a/llvm/test/Transforms/InstCombine/pr14365.ll b/llvm/test/Transforms/InstCombine/pr14365.ll
index 5e8dca13fa1b4d8..3a09b55aba309d9 100644
--- a/llvm/test/Transforms/InstCombine/pr14365.ll
+++ b/llvm/test/Transforms/InstCombine/pr14365.ll
@@ -31,7 +31,7 @@ define i32 @test1(i32 %a0) {
; CHECK-LABEL: @test1(
; CHECK-NEXT: [[TMP1:%.*]] = lshr i32 [[A0:%.*]], 1
; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[TMP1]], 1431655765
-; CHECK-NEXT: [[TMP3:%.*]] = sub i32 [[A0]], [[TMP2]]
+; CHECK-NEXT: [[TMP3:%.*]] = sub nsw i32 [[A0]], [[TMP2]]
; CHECK-NEXT: ret i32 [[TMP3]]
;
%1 = ashr i32 %a0, 1
@@ -46,7 +46,7 @@ define <4 x i32> @test1_vec(<4 x i32> %a0) {
; CHECK-LABEL: @test1_vec(
; CHECK-NEXT: [[TMP1:%.*]] = lshr <4 x i32> [[A0:%.*]], <i32 1, i32 1, i32 1, i32 1>
; CHECK-NEXT: [[TMP2:%.*]] = and <4 x i32> [[TMP1]], <i32 1431655765, i32 1431655765, i32 1431655765, i32 1431655765>
-; CHECK-NEXT: [[TMP3:%.*]] = sub <4 x i32> [[A0]], [[TMP2]]
+; CHECK-NEXT: [[TMP3:%.*]] = sub nsw <4 x i32> [[A0]], [[TMP2]]
; CHECK-NEXT: ret <4 x i32> [[TMP3]]
;
%1 = ashr <4 x i32> %a0, <i32 1, i32 1, i32 1, i32 1>
|
if (auto *Op1 = dyn_cast<BinaryOperator>(LHS)) { | ||
Sub->setHasNoSignedWrap(I.hasNoSignedWrap() && Op1->hasNoSignedWrap()); | ||
} |
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.
if (auto *Op1 = dyn_cast<BinaryOperator>(LHS)) { | |
Sub->setHasNoSignedWrap(I.hasNoSignedWrap() && Op1->hasNoSignedWrap()); | |
} | |
auto *OBO = cast<OverflowingBinaryOperator>(LHS)); | |
Sub->setHasNoSignedWrap(I.hasNoSignedWrap() && OBO->hasNoSignedWrap()); |
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.
This was not fixed in the committed version, it's still using dyn_cast.
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.
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
Fixes #72119
https://alive2.llvm.org/ce/z/5f_QuC