Optimize checked_next_power_of_two consumers - #161069
Conversation
rustbot
added
S-waiting-on-review
|
r? @khyperia rustbot has assigned @khyperia. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? libs |
|
Hmm, I can see these being useful, but I'm a little sceptical that the compiler is going to properly optimise this, given the complexity of the operations. (>= self is probably fine, though.) Would you be willing to find some cases to test the codegen that are maybe a little less contrived? Like, perhaps trying to convert to an |
This is the correct instinct, but in this very particular case LLVM added support for it relatively-recently: llvm/llvm-project#58996 That said, I do wonder if maybe there's just a better way we could phrase this instead that would make it more obvious to LLVM that of course this is a power of two, without needing the For example, suppose it was something like fn cnpot(x: u32) -> Option<u32> {
let e = if let Some(x) = NonZero::new(x) {
(x.get() - 1).bit_width()
} else {
0
};
1_u32.checked_shl(e)
}Then we wouldn't need the assume, because when it's Aside: I wish this returned |
|
For meta-context: the For the versions that don't need that distinction ( We could always do So the most important thing here is some really good codegen tests that demonstrate that, whatever the implementation, you get the optimizations that you wanted. Then we can change the implementation and the tests will ensure that you still get what you want. In particular, that will let people try removing the |
|
Now you're making me want to try and see if I could automate a script to individually remove Similarly to |
|
Just comparing, I think #161209 is going to be better going forward, so, I'm going to close this. |
rustbot
removed
the
S-waiting-on-review
Rework `next_power_of_two` to always be `1 << …` r? @clarfonthey Who accidentally nerd-sniped me by mentioning rust-lang#161069 This obviates that PR by reworking the `(checked_)next_power_of_two` logic to calculate the necessary exponent e, then return 2ᵉ, so it's structurally obvious from the IR -- the `shl nuw 1, …` instruction -- that it's always a power of two. As always, the reason this is tricky is because shifts don't work for `<< Self::BITS`. The previous code handled that by checking `self <= 1` first, and thus doing a `-1 >> n` where `n < BITS`. This code instead flips the check: it looks up-front for a value that will wrap (an input above `1 << (BITS - 1)`) and thus by excluding those cases the calculated `1 << n` always has `n < BITS`. And the codegen tests show that, without needing any `llvm.assume`s, LLVM can take advantage of it to do things like rewriting `%` to masking. Plus the overflow check optimizes out for constrained inputs like slice lengths.
Rework `next_power_of_two` to always be `1 << …` r? @clarfonthey Who accidentally nerd-sniped me by mentioning rust-lang#161069 This obviates that PR by reworking the `(checked_)next_power_of_two` logic to calculate the necessary exponent e, then return 2ᵉ, so it's structurally obvious from the IR -- the `shl nuw 1, …` instruction -- that it's always a power of two. As always, the reason this is tricky is because shifts don't work for `<< Self::BITS`. The previous code handled that by checking `self <= 1` first, and thus doing a `-1 >> n` where `n < BITS`. This code instead flips the check: it looks up-front for a value that will wrap (an input above `1 << (BITS - 1)`) and thus by excluding those cases the calculated `1 << n` always has `n < BITS`. And the codegen tests show that, without needing any `llvm.assume`s, LLVM can take advantage of it to do things like rewriting `%` to masking. Plus the overflow check optimizes out for constrained inputs like slice lengths.
Rollup merge of #161209 - scottmcm:redo-npot, r=clarfonthey Rework `next_power_of_two` to always be `1 << …` r? @clarfonthey Who accidentally nerd-sniped me by mentioning #161069 This obviates that PR by reworking the `(checked_)next_power_of_two` logic to calculate the necessary exponent e, then return 2ᵉ, so it's structurally obvious from the IR -- the `shl nuw 1, …` instruction -- that it's always a power of two. As always, the reason this is tricky is because shifts don't work for `<< Self::BITS`. The previous code handled that by checking `self <= 1` first, and thus doing a `-1 >> n` where `n < BITS`. This code instead flips the check: it looks up-front for a value that will wrap (an input above `1 << (BITS - 1)`) and thus by excluding those cases the calculated `1 << n` always has `n < BITS`. And the codegen tests show that, without needing any `llvm.assume`s, LLVM can take advantage of it to do things like rewriting `%` to masking. Plus the overflow check optimizes out for constrained inputs like slice lengths.
5 participants
Hint that checked_next_power_of_two results are powers of two or are at least as large as the input