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 (ct{t,l}z Pow2) -> Log2(Pow2) #122620

Merged
merged 8 commits into from
Jan 13, 2025

Conversation

goldsteinn
Copy link
Contributor

@goldsteinn goldsteinn commented Jan 11, 2025

  • [InstCombine] Add tests for folding (ct{t,l}z Pow2); NFC
  • [InstCombine] Fold (ct{t,l}z Pow2) -> Log2(Pow2)

Do so we can find Log2(Pow2) for "free" with takeLog2

https://alive2.llvm.org/ce/z/CL77fo

@llvmbot
Copy link
Member

llvmbot commented Jan 11, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes
  • [InstCombine] Add tests for folding (ct{t,l}z Pow2); NFC
  • [InstCombine] Fold (ct{t,l}z Pow2) -> Log2(Pow2)

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+16)
  • (modified) llvm/test/Transforms/InstCombine/cttz.ll (+69)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 7454382412369f..94240773f46a80 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -588,6 +588,22 @@ static Instruction *foldCttzCtlz(IntrinsicInst &II, InstCombinerImpl &IC) {
     }
   }
 
+  // cttz(Pow2) -> Log2(Pow2)
+  // ctlz(Pow2) -> BitWidth - 1 - Log2(Pow2)
+  if (IsTZ || II.hasOneUse()) {
+    if (auto *R = IC.tryGetLog2(Op0, match(Op1, m_One()))) {
+      if (IsTZ)
+        return IC.replaceInstUsesWith(II, R);
+      BinaryOperator *BO = BinaryOperator::CreateSub(
+          ConstantInt::get(R->getType(),
+                           R->getType()->getScalarSizeInBits() - 1),
+          R);
+      BO->setHasNoSignedWrap();
+      BO->setHasNoUnsignedWrap();
+      return BO;
+    }
+  }
+
   KnownBits Known = IC.computeKnownBits(Op0, 0, &II);
 
   // Create a mask for bits above (ctlz) or below (cttz) the first known one.
diff --git a/llvm/test/Transforms/InstCombine/cttz.ll b/llvm/test/Transforms/InstCombine/cttz.ll
index cb0bc59ae79958..5717e352c81e13 100644
--- a/llvm/test/Transforms/InstCombine/cttz.ll
+++ b/llvm/test/Transforms/InstCombine/cttz.ll
@@ -297,3 +297,72 @@ define i16 @cttz_assume(i16 %x) {
   %cttz = call i16 @llvm.cttz.i16(i16 %x, i1 false)
   ret i16 %cttz
 }
+
+
+declare void @use.i8(i8)
+define i8 @fold_ctz_log2(i8 %x) {
+; CHECK-LABEL: @fold_ctz_log2(
+; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.umin.i8(i8 [[X:%.*]], i8 5)
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %p2 = shl i8 1, %x
+  %v = call i8 @llvm.umin(i8 %p2, i8 32)
+  %r = call i8 @llvm.cttz(i8 %v, i1 false)
+  ret i8 %r
+}
+
+define i8 @fold_ctz_log2_maybe_z(i8 %x, i8 %y, i1 %c) {
+; CHECK-LABEL: @fold_ctz_log2_maybe_z(
+; CHECK-NEXT:    [[V:%.*]] = shl i8 2, [[V_V:%.*]]
+; CHECK-NEXT:    [[P2_2:%.*]] = shl i8 4, [[Y:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = select i1 [[C:%.*]], i8 [[V]], i8 [[P2_2]]
+; CHECK-NEXT:    [[R:%.*]] = call range(i8 1, 9) i8 @llvm.cttz.i8(i8 [[V1]], i1 false)
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %p2 = shl i8 2, %x
+  %p2_2 = shl i8 4, %y
+  %v = select i1 %c, i8 %p2, i8 %p2_2
+  %r = call i8 @llvm.cttz(i8 %v, i1 false)
+  ret i8 %r
+}
+
+define i8 @fold_ctz_log2_maybe_z_okay(i8 %x, i8 %y, i1 %c) {
+; CHECK-LABEL: @fold_ctz_log2_maybe_z_okay(
+; CHECK-NEXT:    [[X:%.*]] = add i8 [[X1:%.*]], 1
+; CHECK-NEXT:    [[Y:%.*]] = add i8 [[Y1:%.*]], 2
+; CHECK-NEXT:    [[V_V:%.*]] = select i1 [[C:%.*]], i8 [[X]], i8 [[Y]]
+; CHECK-NEXT:    ret i8 [[V_V]]
+;
+  %p2 = shl i8 2, %x
+  %p2_2 = shl i8 4, %y
+  %v = select i1 %c, i8 %p2, i8 %p2_2
+  %r = call i8 @llvm.cttz(i8 %v, i1 true)
+  ret i8 %r
+}
+
+define i8 @fold_clz_log2(i8 %x) {
+; CHECK-LABEL: @fold_clz_log2(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umin.i8(i8 [[X:%.*]], i8 5)
+; CHECK-NEXT:    [[R:%.*]] = xor i8 [[TMP1]], 7
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %p2 = shl i8 1, %x
+  %v = call i8 @llvm.umin(i8 %p2, i8 32)
+  %r = call i8 @llvm.ctlz(i8 %v, i1 false)
+  ret i8 %r
+}
+
+define i8 @fold_clz_log2_fail_multi_use(i8 %x) {
+; CHECK-LABEL: @fold_clz_log2_fail_multi_use(
+; CHECK-NEXT:    [[P2:%.*]] = shl nuw i8 1, [[X:%.*]]
+; CHECK-NEXT:    [[V:%.*]] = call i8 @llvm.umin.i8(i8 [[P2]], i8 32)
+; CHECK-NEXT:    [[R:%.*]] = call range(i8 2, 9) i8 @llvm.ctlz.i8(i8 [[V]], i1 true)
+; CHECK-NEXT:    call void @use.i8(i8 [[R]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %p2 = shl i8 1, %x
+  %v = call i8 @llvm.umin(i8 %p2, i8 32)
+  %r = call i8 @llvm.ctlz(i8 %v, i1 false)
+  call void @use.i8(i8 %r)
+  ret i8 %r
+}

@goldsteinn goldsteinn changed the title goldsteinn/cttz ctlz of p2 [InstCombine] Fold (ct{t,l}z Pow2) -> Log2(Pow2) Jan 11, 2025
@goldsteinn goldsteinn requested a review from dtcxzyw January 11, 2025 23:04
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.

@@ -588,6 +588,22 @@ static Instruction *foldCttzCtlz(IntrinsicInst &II, InstCombinerImpl &IC) {
}
}

// cttz(Pow2) -> Log2(Pow2)
// ctlz(Pow2) -> BitWidth - 1 - Log2(Pow2)
if (IsTZ || II.hasOneUse()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does the number of uses of the intrinsic matter?

Copy link
Member

Choose a reason for hiding this comment

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

Oops. I guess @goldsteinn means Op0->hasOneUse().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should probably be no multi-use check. We are replacing ctlz with sub which is just worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your new condition effectively drops ctlz entirely :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:/ fixed....

@goldsteinn goldsteinn force-pushed the goldsteinn/cttz-ctlz-of-p2 branch from aa09334 to 3599487 Compare January 12, 2025 16:09
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

No changes on llvm-opt-benchmark from relaxing the one-use constraint.

@goldsteinn goldsteinn merged commit 3318a72 into llvm:main Jan 13, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 13, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building llvm at step 6 "test".

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

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: Utility/./UtilityTests/1/8 (2055 of 2064)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests/2/3 (2056 of 2064)
PASS: lldb-unit :: Utility/./UtilityTests/4/8 (2057 of 2064)
PASS: lldb-unit :: Utility/./UtilityTests/2/8 (2058 of 2064)
PASS: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests/0/2 (2059 of 2064)
PASS: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests/1/2 (2060 of 2064)
PASS: lldb-unit :: Target/./TargetTests/11/14 (2061 of 2064)
PASS: lldb-unit :: Host/./HostTests/2/13 (2062 of 2064)
PASS: lldb-unit :: Process/gdb-remote/./ProcessGdbRemoteTests/8/9 (2063 of 2064)
UNRESOLVED: lldb-api :: tools/lldb-server/TestLldbGdbServer.py (2064 of 2064)
******************** TEST 'lldb-api :: tools/lldb-server/TestLldbGdbServer.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-server -p TestLldbGdbServer.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 3318a7248ae464af0abd0bea5515fa58c962b890)
  clang revision 3318a7248ae464af0abd0bea5515fa58c962b890
  llvm revision 3318a7248ae464af0abd0bea5515fa58c962b890
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hc_then_Csignal_signals_correct_thread_launch_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hc_then_Csignal_signals_correct_thread_launch_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_fails_on_another_pid_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_fails_on_minus_one_pid_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_fails_on_zero_pid_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_switches_to_3_threads_launch_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_switches_to_3_threads_launch_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_and_p_thread_suffix_work_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_and_p_thread_suffix_work_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_writes_all_gpr_registers_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_writes_all_gpr_registers_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_attach_commandline_continue_app_exits_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
Program aborted due to an unhandled Error:
Operation not permitted
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb-server gdbserver --attach=125281 --reverse-connect [127.0.0.1]:37481
 #0 0x0000aaaab70d7a5c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb-server+0xb27a5c)
 #1 0x0000aaaab70d5a8c llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb-server+0xb25a8c)
 #2 0x0000aaaab70d816c SignalHandler(int) Signals.cpp:0:0
 #3 0x0000ffff8110c7dc (linux-vdso.so.1+0x7dc)
 #4 0x0000ffff8090f200 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 13, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building llvm at step 6 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lld :: MachO/arm64-thunks.s' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 15: rm -rf /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp; mkdir /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp
+ rm -rf /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp
+ mkdir /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp
RUN: at line 16: /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/llvm-mc -filetype=obj -triple=arm64-apple-darwin /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/lld/test/MachO/arm64-thunks.s -o /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/input.o
+ /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/llvm-mc -filetype=obj -triple=arm64-apple-darwin /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/lld/test/MachO/arm64-thunks.s -o /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/input.o
RUN: at line 17: ld64.lld -arch x86_64 -platform_version macos 11.0 11.0 -syslibroot /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/lld/test/MachO/Inputs/MacOSX.sdk -lSystem -fatal_warnings -arch arm64 -dead_strip -lSystem -U _extern_sym -map /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/thunk.map -o /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/thunk /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/input.o
+ ld64.lld -arch x86_64 -platform_version macos 11.0 11.0 -syslibroot /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/lld/test/MachO/Inputs/MacOSX.sdk -lSystem -fatal_warnings -arch arm64 -dead_strip -lSystem -U _extern_sym -map /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/thunk.map -o /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/thunk /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/input.o
ld64.lld: error: failed to write output '/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/lld/test/MachO/Output/arm64-thunks.s.tmp/thunk': No space left on device

--

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


kazutakahirata pushed a commit to kazutakahirata/llvm-project that referenced this pull request Jan 13, 2025
- **[InstCombine] Add tests for folding `(ct{t,l}z Pow2)`; NFC**
- **[InstCombine] Fold `(ct{t,l}z Pow2)` -> `Log2(Pow2)`**

Do so we can find `Log2(Pow2)` for "free" with `takeLog2`

https://alive2.llvm.org/ce/z/CL77fo
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.

5 participants