Skip to content

Conversation

@AZero13
Copy link
Contributor

@AZero13 AZero13 commented Oct 15, 2024

@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Rose (AreaZR)

Changes

Alive2 Proofs:
https://alive2.llvm.org/ce/z/aJnxyp
https://alive2.llvm.org/ce/z/dyeGEv


Full diff: https://github.com/llvm/llvm-project/pull/112407.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (+29)
  • (modified) llvm/test/Transforms/InstCombine/ashr-lshr.ll (+168)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index f4f3644acfe5ea..3378da1130ae68 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -261,6 +261,35 @@ Instruction *InstCombinerImpl::visitMul(BinaryOperator &I) {
     }
   }
 
+  // mul (lshr exact X, N), (2^N + 1) -> add (X, lshr exact (X, N))
+  {
+    Value *NewOp;
+    const APInt *ShiftC;
+    const APInt *MulAP;
+    if (match(&I, m_Mul(m_Exact(m_Shr(m_Value(NewOp), m_APInt(ShiftC))),
+                        m_APInt(MulAP)))) {
+      if (BitWidth > 2 && (*MulAP - 1).isPowerOf2() &&
+          *ShiftC == MulAP->logBase2()) {
+        Value *BinOp = Op0;
+        BinaryOperator *OpBO = cast<BinaryOperator>(Op0);
+        if (!isGuaranteedNotToBeUndef(NewOp, &AC, &I, &DT))
+          NewOp = Builder.CreateFreeze(NewOp, NewOp->getName() + ".fr");
+        if (HasNUW && OpBO->getOpcode() == Instruction::AShr &&
+            OpBO->hasOneUse())
+          BinOp = Builder.CreateLShr(NewOp, ConstantInt::get(Ty, *ShiftC), "",
+                                     /*isExact=*/true);
+
+        auto *NewAdd = BinaryOperator::CreateAdd(NewOp, BinOp);
+        if (HasNSW && (HasNUW || OpBO->getOpcode() == Instruction::LShr ||
+                       ShiftC->getZExtValue() < BitWidth - 1))
+          NewAdd->setHasNoSignedWrap(true);
+
+        NewAdd->setHasNoUnsignedWrap(HasNUW);
+        return NewAdd;
+      }
+    }
+  }
+
   if (Op0->hasOneUse() && match(Op1, m_NegatedPower2())) {
     // Interpret  X * (-1<<C)  as  (-X) * (1<<C)  and try to sink the negation.
     // The "* (1<<C)" thus becomes a potential shifting opportunity.
diff --git a/llvm/test/Transforms/InstCombine/ashr-lshr.ll b/llvm/test/Transforms/InstCombine/ashr-lshr.ll
index 1abf1be2cbedd9..9c4611d0be3947 100644
--- a/llvm/test/Transforms/InstCombine/ashr-lshr.ll
+++ b/llvm/test/Transforms/InstCombine/ashr-lshr.ll
@@ -1077,4 +1077,172 @@ entry:
   ret i32 %shr
 }
 
+define i32 @ashr_shift_mul(i32 noundef %x) {
+; CHECK-LABEL: @ashr_shift_mul(
+; CHECK-NEXT:    [[A:%.*]] = ashr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr exact i32 %x, 3
+  %res = mul i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @ashr_shift_mul_nuw(i32 noundef %x) {
+; CHECK-LABEL: @ashr_shift_mul_nuw(
+; CHECK-NEXT:    [[TMP1:%.*]] = lshr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add nuw i32 [[X]], [[TMP1]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr exact i32 %x, 3
+  %res = mul nuw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @ashr_shift_mul_nsw(i32 noundef %x) {
+; CHECK-LABEL: @ashr_shift_mul_nsw(
+; CHECK-NEXT:    [[A:%.*]] = ashr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add nsw i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr exact i32 %x, 3
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @lshr_shift_mul_nuw(i32 noundef %x) {
+; CHECK-LABEL: @lshr_shift_mul_nuw(
+; CHECK-NEXT:    [[A:%.*]] = lshr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add nuw i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr exact i32 %x, 3
+  %res = mul nuw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @lshr_shift_mul(i32 noundef %x) {
+; CHECK-LABEL: @lshr_shift_mul(
+; CHECK-NEXT:    [[A:%.*]] = lshr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr exact i32 %x, 3
+  %res = mul i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @lshr_shift_mul_nsw(i32 noundef %x) {
+; CHECK-LABEL: @lshr_shift_mul_nsw(
+; CHECK-NEXT:    [[A:%.*]] = lshr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add nsw i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr exact i32 %x, 3
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+; Negative test
+
+define i32 @lshr_no_exact(i32 %x) {
+; CHECK-LABEL: @lshr_no_exact(
+; CHECK-NEXT:    [[A:%.*]] = lshr i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = mul nuw nsw i32 [[A]], 9
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr i32 %x, 3
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+; Negative test
+
+define i32 @ashr_no_exact(i32 %x) {
+; CHECK-LABEL: @ashr_no_exact(
+; CHECK-NEXT:    [[A:%.*]] = ashr i32 [[X:%.*]], 3
+; CHECK-NEXT:    [[RES:%.*]] = mul nsw i32 [[A]], 9
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr i32 %x, 3
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @lshr_no_undef(i32 %x) {
+; CHECK-LABEL: @lshr_no_undef(
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze i32 [[X:%.*]]
+; CHECK-NEXT:    [[A:%.*]] = lshr exact i32 [[X_FR]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add nsw i32 [[X_FR]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr exact i32 %x, 3
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @ashr_no_undef(i32 %x) {
+; CHECK-LABEL: @ashr_no_undef(
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze i32 [[X:%.*]]
+; CHECK-NEXT:    [[A:%.*]] = ashr exact i32 [[X_FR]], 3
+; CHECK-NEXT:    [[RES:%.*]] = add nsw i32 [[X_FR]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr exact i32 %x, 3
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @lshr_multiuse(i32 noundef %x) {
+; CHECK-LABEL: @lshr_multiuse(
+; CHECK-NEXT:    [[A:%.*]] = lshr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    call void @use(i32 [[A]])
+; CHECK-NEXT:    [[RES:%.*]] = add nsw i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr exact i32 %x, 3
+  call void @use(i32 %a)
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @lshr_multiuse_no_flags(i32 noundef %x) {
+; CHECK-LABEL: @lshr_multiuse_no_flags(
+; CHECK-NEXT:    [[A:%.*]] = lshr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    call void @use(i32 [[A]])
+; CHECK-NEXT:    [[RES:%.*]] = add i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = lshr exact i32 %x, 3
+  call void @use(i32 %a)
+  %res = mul i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @ashr_multiuse_no_flags(i32 noundef %x) {
+; CHECK-LABEL: @ashr_multiuse_no_flags(
+; CHECK-NEXT:    [[A:%.*]] = ashr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    call void @use(i32 [[A]])
+; CHECK-NEXT:    [[RES:%.*]] = add i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr exact i32 %x, 3
+  call void @use(i32 %a)
+  %res = mul i32 %a, 9
+  ret i32 %res
+}
+
+define i32 @ashr_multiuse(i32 noundef %x) {
+; CHECK-LABEL: @ashr_multiuse(
+; CHECK-NEXT:    [[A:%.*]] = ashr exact i32 [[X:%.*]], 3
+; CHECK-NEXT:    call void @use(i32 [[A]])
+; CHECK-NEXT:    [[RES:%.*]] = add nsw i32 [[X]], [[A]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %a = ashr exact i32 %x, 3
+  call void @use(i32 %a)
+  %res = mul nsw i32 %a, 9
+  ret i32 %res
+}
+
 declare void @use(i32)

@AZero13 AZero13 changed the title [InstCombine] Fold mul (lshr exact (X, N)), 2^N + 1 -> add (X , lshr exact (X, N)) [InstCombine] Fold mul (shr exact (X, N)), 2^N + 1 -> add (X , shr exact (X, N)) Oct 15, 2024
@AZero13 AZero13 force-pushed the inverse-div-shift branch 2 times, most recently from c6a0f7e to 35698c1 Compare October 15, 2024 17:46
@llvmbot llvmbot added the llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes label Nov 14, 2024
@AZero13
Copy link
Contributor Author

AZero13 commented Nov 17, 2024

Going to ping @nikic

@AZero13
Copy link
Contributor Author

AZero13 commented Feb 6, 2025

@dtcxzyw I was wondering if you would like to merge this or not?

@AZero13 AZero13 force-pushed the inverse-div-shift branch 2 times, most recently from a952c89 to 19ecf32 Compare February 7, 2025 20:40
@github-actions
Copy link

github-actions bot commented Feb 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@AZero13 AZero13 force-pushed the inverse-div-shift branch 6 times, most recently from 708467d to 8699887 Compare February 8, 2025 21:07
@AZero13 AZero13 requested a review from dtcxzyw February 8, 2025 21:07
@AZero13
Copy link
Contributor Author

AZero13 commented Feb 10, 2025

@dtcxzyw I am ready!

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some nits.

@AZero13
Copy link
Contributor Author

AZero13 commented Feb 12, 2025

LGTM with some nits.

Done! @dtcxzyw Let's go!

@dtcxzyw
Copy link
Member

dtcxzyw commented Feb 13, 2025

Please avoid force-pushing if possible

@dtcxzyw dtcxzyw merged commit ffd2633 into llvm:main Feb 13, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 13, 2025

LLVM Buildbot has detected a new failure on builder clang-cmake-x86_64-avx512-win running on avx512-intel64-win while building llvm at step 6 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/4685

Here is the relevant piece of the build log for the reference
Step 6 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Clang :: Index/annotate-attribute.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 1
d:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\stage1\bin\c-index-test.exe -test-load-source all D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\clang\test\Index\annotate-attribute.cpp | d:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\stage1\bin\filecheck.exe D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\clang\test\Index\annotate-attribute.cpp
# executed command: 'd:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\stage1\bin\c-index-test.exe' -test-load-source all 'D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\clang\test\Index\annotate-attribute.cpp'
# executed command: 'd:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\stage1\bin\filecheck.exe' 'D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\clang\test\Index\annotate-attribute.cpp'
# .---command stderr------------
# | D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\clang\test\Index\annotate-attribute.cpp:27:16: error: CHECK-NEXT: is not on the line after the previous match
# | // CHECK-NEXT: CXXMethod=aMethod:5:51 Extent=[5:3 - 5:60]
# |                ^
# | <stdin>:446:8: note: 'next' match was here
# | :5:51: CXXMethod=aMethod:5:51 Extent=[5:3 - 5:60] [access=public]
# |        ^
# | <stdin>:445:106: note: previous match ended here
# | // CHECK: �������������������������������~�0:4:1: CXXAccessSpecifier=:4:1 (Definition) Extent=[4:1 - 4:8] [access=public]
# | <stdin>:446:1: note: non-matching line after previous match is here
# | // CHECK: ����������������������������&�t��
# | 
# | Input file: <stdin>
# | Check file: D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\clang\test\Index\annotate-attribute.cpp
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |          .
# |          .
# |          .
# |        441: // CHECK: <invalid loc>:444:9: macro definition=__STDC_EMBED_NOT_FOUND__ 
# |        442: // CHECK: <invalid loc>:445:9: macro definition=__STDC_EMBED_FOUND__ 
# |        443: // CHECK: <invalid loc>:446:9: macro definition=__STDC_EMBED_EMPTY__ 
# |        444: // CHECK: ����������������������������|��~�
# | :3:7: ClassDecl=Test:3:7 (Definition) Extent=[3:1 - 17:2] 
# |        445: // CHECK: �������������������������������~�0:4:1: CXXAccessSpecifier=:4:1 (Definition) Extent=[4:1 - 4:8] [access=public] 
# |        446: // CHECK: ����������������������������&�t��
# | :5:51: CXXMethod=aMethod:5:51 Extent=[5:3 - 5:60] [access=public] 
# | next:27            !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                              error: match on wrong line
# |        447: // CHECK: �������������������������������~�0:5:18: attribute(annotate)=spiffy_method Extent=[5:18 - 5:43] 
# |        448: // CHECK: ������������������������������\���:7:1: CXXAccessSpecifier=:7:1 (Definition) Extent=[7:1 - 7:43] [access=public] 
# |        449: // CHECK: ������������������������������0���:7:23: attribute(annotate)=works Extent=[7:23 - 7:40] 
# |        450: // CHECK: ����������������������������`�~� :8:8: CXXMethod=anotherMethod:8:8 Extent=[8:3 - 8:23] [access=public] 
# |        451: // CHECK: ������������������������������\���:7:23: attribute(annotate)=works Extent=[7:23 - 7:40] 
# |          .
# |          .
# |          .
# | >>>>>>
# `-----------------------------
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants