Optimize `checked_ilog` and `pow` when `base` is a power of two #147250

Kmeakin · 2025-10-02T00:33:17Z

Optimize checked_ilog and pow when the base is a power of two

rustbot · 2025-10-02T00:33:22Z

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

workingjubilee · 2025-10-02T02:09:34Z

does this affect the codegen in practical circumstances? I would expect even a fairly weak optimizer to perform this optimization, making us hand-coding it irrelevant and potentially even harmful because we introduce more conditional logic to chew through.

Kmeakin · 2025-10-02T02:21:24Z

does this affect the codegen in practical circumstances?

It replaces a loop by some bit manipulations. Whether anyone actually calls either function with a compile-time known, power of 2 base is another question. But it can't hurt, since it is guarded by is_statically_known.

I would expect even a fairly weak optimizer to perform this optimization, making us hand-coding it irrelevant

No, in either function, LLVM is not able to see that the loop is performing a log/pow and apply the identities: https://godbolt.org/z/6vMsxc9Kh

and potentially even harmful because we introduce more conditional logic to chew through.

They're guarded by is_statically_known so will be ignored if the base is not known at compile-time

workingjubilee · 2025-10-02T02:40:04Z

No, in either function, LLVM is not able to see that the loop is performing a log/pow and apply the identities: https://godbolt.org/z/6vMsxc9Kh

...then I'm kinda surprised! Nice catch.

library/core/src/num/uint_macros.rs

scottmcm · 2025-10-02T06:01:57Z

does this affect the codegen in practical circumstances?

Would be good to have codegen tests to demonstrate what this is doing -- especially since that way there's a way for people to try removing the special cases later if they think that LLVM no longer needs them.

library/core/src/num/uint_macros.rs

workingjubilee · 2025-10-05T18:10:15Z

thanks for the codegen tests!

Kmeakin · 2025-10-05T21:25:43Z

@rustbot ready

Kmeakin · 2025-10-10T22:51:45Z

@scottmcm ping?

scottmcm · 2025-10-14T02:41:27Z

tests/codegen-llvm/ilog_known_base.rs

+#[no_mangle]
+pub fn checked_ilog16(val: u32) -> Option<u32> {
+    // CHECK: %[[ICMP:.+]] = icmp ne i32 %val, 0
+    // CHECK: %[[CTZ:.+]] = tail call range(i32 0, 33) i32 @llvm.ctlz.i32(i32 %val, i1 true)


This isn't really this PR's problem, but consider filing an LLVM bug (assuming it's also true in trunk) that this range is wider than necessary -- we're passing true for is_zero_poison https://llvm.org/docs/LangRef.html#llvm-ctlz-intrinsic so the range here should be range(i32 0, 32).

The range attribute is too large, but it seems LLVM is still able to propagate the knowledge:

https://godbolt.org/z/dzxfGnhMf

scottmcm

The code here is looking good, but can you make sure there's normal runtime behaviour tests for it too? Notably, after the base conversation (that's fixed in the code) it made me think that that's not currently covered by any tests, so we should have something -- maybe just add to the # Examples that it returns None for base zero or one? (They seem like perfectly reasonable and helpful examples, in addition to giving coverage for this stuff.)

And maybe add some should_panic tests to ensure that the overflow checking is still correct for pow when overflow checks are enabled?

View changes since this review

if base == 2 ** k, then log(base, n) == log(2, n) / k

rustbot · 2025-10-15T00:51:53Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

if base == 2 ** k, then (2 ** k) ** n == 2 ** (k * n) == 1 << (k * n)

rustbot assigned scottmcm Oct 2, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 2, 2025

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from 8f19ce6 to e5ca8cd Compare October 2, 2025 01:31

This comment has been minimized.

Sign in to view

Kmeakin changed the title ~~Optimize checked_ilog when base is a power of two~~ Optimize checked_ilog and pow when base is a power of two Oct 2, 2025

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch 2 times, most recently from 2faa397 to abb7f32 Compare October 2, 2025 02:38

scottmcm reviewed Oct 2, 2025

View reviewed changes

library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved

scottmcm reviewed Oct 2, 2025

View reviewed changes

library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved

scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from abb7f32 to 9ab0f63 Compare October 2, 2025 23:13

nikic reviewed Oct 3, 2025

View reviewed changes

library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from 9ab0f63 to 1d0ac82 Compare October 3, 2025 21:16

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from 1d0ac82 to c84b99e Compare October 5, 2025 18:29

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 5, 2025

scottmcm reviewed Oct 14, 2025

View reviewed changes

scottmcm requested changes Oct 14, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 14, 2025

Optimize checked_ilog when base is a power of two

ee9dd66

if base == 2 ** k, then log(base, n) == log(2, n) / k

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from c84b99e to 946ce96 Compare October 15, 2025 00:51

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from 946ce96 to e481662 Compare October 15, 2025 01:31

This comment has been minimized.

Sign in to view

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from e481662 to 264cefa Compare October 16, 2025 21:59

Kmeakin requested a review from scottmcm October 16, 2025 21:59

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 16, 2025

This comment has been minimized.

Sign in to view

Optimize pow when self is a power of two

53449c9

if base == 2 ** k, then (2 ** k) ** n == 2 ** (k * n) == 1 << (k * n)

Kmeakin force-pushed the km/optimize-ilog-base-power-of-two branch from 264cefa to 53449c9 Compare October 17, 2025 11:46

Optimize checked_ilog and pow when base is a power of two #147250

Are you sure you want to change the base?

Optimize checked_ilog and pow when base is a power of two #147250

Conversation

Kmeakin commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 2, 2025

Uh oh!

This comment has been minimized.

workingjubilee commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kmeakin commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Oct 2, 2025

Uh oh!

Uh oh!

Uh oh!

scottmcm commented Oct 2, 2025

Uh oh!

Uh oh!

workingjubilee commented Oct 5, 2025

Uh oh!

Kmeakin commented Oct 5, 2025

Uh oh!

Kmeakin commented Oct 10, 2025

Uh oh!

scottmcm Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Kmeakin Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

scottmcm left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Oct 15, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Optimize `checked_ilog` and `pow` when `base` is a power of two #147250

Optimize `checked_ilog` and `pow` when `base` is a power of two #147250

Kmeakin commented Oct 2, 2025 •

edited

Loading

workingjubilee commented Oct 2, 2025 •

edited

Loading

Kmeakin commented Oct 2, 2025 •

edited

Loading

scottmcm left a comment •

edited by rustbot

Loading