Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 1, 2024

@dtcxzyw dtcxzyw requested a review from goldsteinn December 1, 2024 06:23
@dtcxzyw dtcxzyw requested a review from nikic as a code owner December 1, 2024 06:23
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels 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




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


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.

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

4 participants