rustc_mir: calc hex number length without string allocation #86304
Conversation
|
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| /// ```ignore | ||
| /// assert_eq!(1, hex_number_length(0)); | ||
| /// assert_eq!(1, hex_number_length(1)); | ||
| /// assert_eq!(2, hex_number_length(16)); | ||
| /// ``` | ||
| fn hex_number_length(x: u64) -> usize { |
There was a problem hiding this comment.
Idk how to write doctest for private function :-(
This comment has been minimized.
This comment has been minimized.
|
Bump |
| /// assert_eq!(2, hex_number_length(16)); | ||
| /// ``` | ||
| fn hex_number_length(x: u64) -> usize { | ||
| let mut length = if x == 0 { 1 } else { 0 }; |
There was a problem hiding this comment.
if x ==0, why not just early return 1?
There was a problem hiding this comment.
When i wrote this i remember that this will give the same asm, but i'll recheck.
There was a problem hiding this comment.
Returning early is more clear to me
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit 723293c4f6ced9d10f3c29083de866573b3caa31 with merge 812cf9a5f9dc29d03e0210b7a827600d2c30bdbb... |
|
@jackh726 Yeah, i don't expect perf gains, but if we have opportunity not to use allocations, we can do it (plus, this code not so hard to read, i think). |
|
☀️ Try build successful - checks-actions |
|
Queued 812cf9a5f9dc29d03e0210b7a827600d2c30bdbb with parent 456a032, future comparison URL. |
|
Finished benchmarking try commit (812cf9a5f9dc29d03e0210b7a827600d2c30bdbb): comparison url. Summary: ERROR categorizing benchmark run! Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
This run looks broken - no result info. |
|
@bors try @rust-timer queue take 2 |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 723293c4f6ced9d10f3c29083de866573b3caa31 with merge 4c85d7041c65d68b8ee108c4e58a805ea36dac39... |
|
☀️ Try build successful - checks-actions |
|
Queued 4c85d7041c65d68b8ee108c4e58a805ea36dac39 with parent f2571a2, future comparison URL. |
|
Finished benchmarking try commit (4c85d7041c65d68b8ee108c4e58a805ea36dac39): comparison url. Summary: This benchmark run did not return any significant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
Error: Label perf-regression can only be set by Rust team members Please let |
JohnTitor
removed
the
S-waiting-on-perf
|
r=me after modifying |
jackh726
added
S-waiting-on-author
|
@bors r+ |
|
📌 Commit 2b57fc4 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
No description provided.