Skip to content
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

[InstCombine] Fold umax(X, C) + -C into usub.sat(X, C) #118195

Merged
merged 2 commits into from
Dec 1, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 1, 2024

@llvmbot
Copy link
Member

llvmbot commented Dec 1, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Alive2: https://alive2.llvm.org/ce/z/oSWe5S
Closes #118155


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+6)
  • (modified) llvm/test/Transforms/InstCombine/saturating-add-sub.ll (+36)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 6fe96935818531..63725e4ca8113e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -994,6 +994,12 @@ Instruction *InstCombinerImpl::foldAddWithConstant(BinaryOperator &Add) {
     }
   }
 
+  // umax(X, C) + -C --> usub.sat(X, C)
+  if (match(Op0, m_OneUse(m_UMax(m_Value(X), m_SpecificInt(-*C)))))
+    return replaceInstUsesWith(
+        Add, Builder.CreateBinaryIntrinsic(
+                 Intrinsic::usub_sat, X, ConstantInt::get(Add.getType(), -*C)));
+
   // Fold (add (zext (add X, -1)), 1) -> (zext X) if X is non-zero.
   // TODO: There's a general form for any constant on the outer add.
   if (C->isOne()) {
diff --git a/llvm/test/Transforms/InstCombine/saturating-add-sub.ll b/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
index 9236d96f59a55b..d043f5b7ad116d 100644
--- a/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
+++ b/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
@@ -2316,3 +2316,39 @@ define i32 @uadd_sat_via_add_swapped_cmp_select_nonstrict(i32 %x, i32 %y) {
   %r = select i1 %c, i32 %a, i32 -1
   ret i32 %r
 }
+
+define i8 @fold_add_umax_to_usub(i8 %a) {
+; CHECK-LABEL: @fold_add_umax_to_usub(
+; CHECK-NEXT:    [[SEL:%.*]] = call i8 @llvm.usub.sat.i8(i8 [[A:%.*]], i8 10)
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %umax = call i8 @llvm.umax.i8(i8 %a, i8 10)
+  %sel = add i8 %umax, -10
+  ret i8 %sel
+}
+
+define i8 @fold_add_umax_to_usub_incorrect_rhs(i8 %a) {
+; CHECK-LABEL: @fold_add_umax_to_usub_incorrect_rhs(
+; CHECK-NEXT:    [[UMAX:%.*]] = call i8 @llvm.umax.i8(i8 [[A:%.*]], i8 10)
+; CHECK-NEXT:    [[SEL:%.*]] = add i8 [[UMAX]], -11
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %umax = call i8 @llvm.umax.i8(i8 %a, i8 10)
+  %sel = add i8 %umax, -11
+  ret i8 %sel
+}
+
+define i8 @fold_add_umax_to_usub_multiuse(i8 %a) {
+; CHECK-LABEL: @fold_add_umax_to_usub_multiuse(
+; CHECK-NEXT:    [[UMAX:%.*]] = call i8 @llvm.umax.i8(i8 [[A:%.*]], i8 10)
+; CHECK-NEXT:    call void @usei8(i8 [[UMAX]])
+; CHECK-NEXT:    [[SEL:%.*]] = add i8 [[UMAX]], -10
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %umax = call i8 @llvm.umax.i8(i8 %a, i8 10)
+  call void @usei8(i8 %umax)
+  %sel = add i8 %umax, -10
+  ret i8 %sel
+}
+
+declare void @usei8(i8)

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@dtcxzyw dtcxzyw merged commit 1a3eace into llvm:main Dec 1, 2024
11 checks passed
@dtcxzyw dtcxzyw deleted the perf/add_umax_to_usub branch December 1, 2024 15:29
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 1, 2024

LLVM Buildbot has detected a new failure on builder libc-x86_64-debian-fullbuild-dbg-asan running on libc-x86_64-debian-fullbuild while building llvm at step 4 "annotate".

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

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[937/1098] Running unit test libc.test.src.sys.mman.linux.remap_file_pages_test
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcRemapFilePagesTest.NoError
[       OK ] LlvmLibcRemapFilePagesTest.NoError (165 us)
[ RUN      ] LlvmLibcRemapFilePagesTest.ErrorInvalidFlags
[       OK ] LlvmLibcRemapFilePagesTest.ErrorInvalidFlags (61 us)
[ RUN      ] LlvmLibcRemapFilePagesTest.ErrorInvalidAddress
[       OK ] LlvmLibcRemapFilePagesTest.ErrorInvalidAddress (5 us)
Ran 3 tests.  PASS: 3  FAIL: 0
[938/1098] Running unit test libc.test.src.sys.mman.linux.process_mrelease_test
FAILED: projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test 
cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/mman/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/mman/linux/libc.test.src.sys.mman.linux.process_mrelease_test.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcProcessMReleaseTest.NoError
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/llvm-project/libc/test/src/sys/mman/linux/process_mrelease_test.cpp:44: FAILURE
Failed to match LIBC_NAMESPACE::process_mrelease(pidfd, 0) against Succeeds().
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "No such process".
[  FAILED  ] LlvmLibcProcessMReleaseTest.NoError
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNotKilled
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNotKilled (756 us)
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd (9 us)
Ran 3 tests.  PASS: 2  FAIL: 1
[939/1098] Running unit test libc.test.src.math.smoke.lrintf_test.__unit__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcRoundToIntegerTest.InfinityAndNaN
[       OK ] LlvmLibcRoundToIntegerTest.InfinityAndNaN (8 us)
[ RUN      ] LlvmLibcRoundToIntegerTest.RoundNumbers
[       OK ] LlvmLibcRoundToIntegerTest.RoundNumbers (9 us)
[ RUN      ] LlvmLibcRoundToIntegerTest.SubnormalRange
[       OK ] LlvmLibcRoundToIntegerTest.SubnormalRange (933 ms)
Ran 3 tests.  PASS: 3  FAIL: 0
[940/1098] Running unit test libc.test.src.sys.mman.linux.mincore_test
[==========] Running 6 tests from 1 test suite.
[ RUN      ] LlvmLibcMincoreTest.UnMappedMemory
[       OK ] LlvmLibcMincoreTest.UnMappedMemory (18 us)
[ RUN      ] LlvmLibcMincoreTest.UnalignedAddr
[       OK ] LlvmLibcMincoreTest.UnalignedAddr (48 us)
[ RUN      ] LlvmLibcMincoreTest.InvalidVec
[       OK ] LlvmLibcMincoreTest.InvalidVec (13 us)
[ RUN      ] LlvmLibcMincoreTest.NoError
[       OK ] LlvmLibcMincoreTest.NoError (14 us)
[ RUN      ] LlvmLibcMincoreTest.NegativeLength
[       OK ] LlvmLibcMincoreTest.NegativeLength (13 us)
[ RUN      ] LlvmLibcMincoreTest.PageOut
[       OK ] LlvmLibcMincoreTest.PageOut (45 us)
Ran 6 tests.  PASS: 6  FAIL: 0
[941/1098] Running unit test libc.test.src.sys.mman.linux.shm_test
Step 8 (libc-unit-tests) failure: libc-unit-tests (failure)
...
[937/1098] Running unit test libc.test.src.sys.mman.linux.remap_file_pages_test
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcRemapFilePagesTest.NoError
[       OK ] LlvmLibcRemapFilePagesTest.NoError (165 us)
[ RUN      ] LlvmLibcRemapFilePagesTest.ErrorInvalidFlags
[       OK ] LlvmLibcRemapFilePagesTest.ErrorInvalidFlags (61 us)
[ RUN      ] LlvmLibcRemapFilePagesTest.ErrorInvalidAddress
[       OK ] LlvmLibcRemapFilePagesTest.ErrorInvalidAddress (5 us)
Ran 3 tests.  PASS: 3  FAIL: 0
[938/1098] Running unit test libc.test.src.sys.mman.linux.process_mrelease_test
FAILED: projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test 
cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/mman/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/mman/linux/libc.test.src.sys.mman.linux.process_mrelease_test.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcProcessMReleaseTest.NoError
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/llvm-project/libc/test/src/sys/mman/linux/process_mrelease_test.cpp:44: FAILURE
Failed to match LIBC_NAMESPACE::process_mrelease(pidfd, 0) against Succeeds().
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "No such process".
[  FAILED  ] LlvmLibcProcessMReleaseTest.NoError
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNotKilled
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNotKilled (756 us)
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd (9 us)
Ran 3 tests.  PASS: 2  FAIL: 1
[939/1098] Running unit test libc.test.src.math.smoke.lrintf_test.__unit__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcRoundToIntegerTest.InfinityAndNaN
[       OK ] LlvmLibcRoundToIntegerTest.InfinityAndNaN (8 us)
[ RUN      ] LlvmLibcRoundToIntegerTest.RoundNumbers
[       OK ] LlvmLibcRoundToIntegerTest.RoundNumbers (9 us)
[ RUN      ] LlvmLibcRoundToIntegerTest.SubnormalRange
[       OK ] LlvmLibcRoundToIntegerTest.SubnormalRange (933 ms)
Ran 3 tests.  PASS: 3  FAIL: 0
[940/1098] Running unit test libc.test.src.sys.mman.linux.mincore_test
[==========] Running 6 tests from 1 test suite.
[ RUN      ] LlvmLibcMincoreTest.UnMappedMemory
[       OK ] LlvmLibcMincoreTest.UnMappedMemory (18 us)
[ RUN      ] LlvmLibcMincoreTest.UnalignedAddr
[       OK ] LlvmLibcMincoreTest.UnalignedAddr (48 us)
[ RUN      ] LlvmLibcMincoreTest.InvalidVec
[       OK ] LlvmLibcMincoreTest.InvalidVec (13 us)
[ RUN      ] LlvmLibcMincoreTest.NoError
[       OK ] LlvmLibcMincoreTest.NoError (14 us)
[ RUN      ] LlvmLibcMincoreTest.NegativeLength
[       OK ] LlvmLibcMincoreTest.NegativeLength (13 us)
[ RUN      ] LlvmLibcMincoreTest.PageOut
[       OK ] LlvmLibcMincoreTest.PageOut (45 us)
Ran 6 tests.  PASS: 6  FAIL: 0
[941/1098] Running unit test libc.test.src.sys.mman.linux.shm_test

@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 1, 2024

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/37/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-16376-37-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=37 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


TIFitis pushed a commit to TIFitis/llvm-project that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[InstCombine] umax(X, C) + -C -> usub.sat(X, C)
4 participants