-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Reland "[InstCombine] Fold (sub nuw X, (Y << nuw Z)) >>u exact Z --> (X >>u exact Z) sub nuw Y" #93571
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: AtariDreams (AtariDreams) ChangesThis is the same fold as ((X << nuw Z) sub nuw Y) >>u exact Z --> X sub nuw (Y >>u exact Z), but with the sub operands swapped. Proof: https://alive2.llvm.org/ce/z/2cRcdx Relanding #91386 (comment), approved by @dtcxzyw Full diff: https://github.com/llvm/llvm-project/pull/93571.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index ba297111d945f..8d0209ccd13ef 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -1271,6 +1271,17 @@ Instruction *InstCombinerImpl::visitLShr(BinaryOperator &I) {
return NewSub;
}
+ // (sub nuw X, (Y << nuw Z)) >>u exact Z --> (X >>u exact Z) sub nuw Y
+ if (I.isExact() &&
+ match(Op0, m_OneUse(m_NUWSub(m_Value(X),
+ m_NUWShl(m_Value(Y), m_Specific(Op1)))))) {
+ Value *NewLshr = Builder.CreateLShr(X, Op1, "", /*isExact=*/true);
+ auto *NewSub = BinaryOperator::CreateNUWSub(NewLshr, Y);
+ NewSub->setHasNoSignedWrap(
+ cast<OverflowingBinaryOperator>(Op0)->hasNoSignedWrap());
+ return NewSub;
+ }
+
auto isSuitableBinOpcode = [](Instruction::BinaryOps BinOpcode) {
switch (BinOpcode) {
default:
diff --git a/llvm/test/Transforms/InstCombine/lshr.ll b/llvm/test/Transforms/InstCombine/lshr.ll
index fa92c1c4b3be4..1e370d960c45d 100644
--- a/llvm/test/Transforms/InstCombine/lshr.ll
+++ b/llvm/test/Transforms/InstCombine/lshr.ll
@@ -466,6 +466,33 @@ define i32 @shl_sub_lshr(i32 %x, i32 %c, i32 %y) {
ret i32 %lshr
}
+define i32 @shl_sub_lshr_reverse(i32 %x, i32 %c, i32 %y) {
+; CHECK-LABEL: @shl_sub_lshr_reverse(
+; CHECK-NEXT: [[TMP1:%.*]] = lshr exact i32 [[Y:%.*]], [[C:%.*]]
+; CHECK-NEXT: [[LSHR:%.*]] = sub nuw nsw i32 [[TMP1]], [[X:%.*]]
+; CHECK-NEXT: ret i32 [[LSHR]]
+;
+ %shl = shl nuw i32 %x, %c
+ %sub = sub nuw nsw i32 %y, %shl
+ %lshr = lshr exact i32 %sub, %c
+ ret i32 %lshr
+}
+
+; Negative test
+
+define i32 @shl_sub_lshr_reverse_no_exact(i32 %x, i32 %c, i32 %y) {
+; CHECK-LABEL: @shl_sub_lshr_reverse_no_exact(
+; CHECK-NEXT: [[SHL:%.*]] = shl nuw i32 [[X:%.*]], [[C:%.*]]
+; CHECK-NEXT: [[SUB:%.*]] = sub nuw nsw i32 [[Y:%.*]], [[SHL]]
+; CHECK-NEXT: [[LSHR:%.*]] = lshr i32 [[SUB]], [[C]]
+; CHECK-NEXT: ret i32 [[LSHR]]
+;
+ %shl = shl nuw i32 %x, %c
+ %sub = sub nuw nsw i32 %y, %shl
+ %lshr = lshr i32 %sub, %c
+ ret i32 %lshr
+}
+
define i32 @shl_or_lshr(i32 %x, i32 %c, i32 %y) {
; CHECK-LABEL: @shl_or_lshr(
; CHECK-NEXT: [[TMP1:%.*]] = lshr i32 [[Y:%.*]], [[C:%.*]]
|
@dtcxzyw Is this good to be relanded now? |
BTW, don't @ someone in the PR description/commit message, it is annoying. |
9f099ce
to
b44b4ec
Compare
d7a902f
to
721baca
Compare
…xact Z) sub nuw Y This is the same fold as ((X << nuw Z) sub nuw Y) >>u exact Z --> X sub nuw (Y >>u exact Z), but with the sub operands swapped. Alive2 Proof: https://alive2.llvm.org/ce/z/2cRcdx
@dtcxzyw I did all the possible negative tests. I think this can be merged now! |
@nikic Ping? |
This LGTM, please wait for another approval before pushing. |
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.
I'll merge this on Monday if nobody beats me to it.
This is the same fold as ((X << nuw Z) sub nuw Y) >>u exact Z --> X sub nuw (Y >>u exact Z), but with the sub operands swapped.
Alive2 Proof:
https://alive2.llvm.org/ce/z/pT-RxG