Use unchecked_sub in str indexing by saethlin · Pull Request #123561 · rust-lang...
source link: https://github.com/rust-lang/rust/pull/123561
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Member
#108763 applied this logic to indexing for slices, but of course str has its own separate impl.
Found this by skimming over the codegen for https://github.com/oxidecomputer/hubris/; their dist builds enable overflow checks so the lack of unchecked_sub was producing an impossible-to-hit overflow check and also inhibiting some inlining.
r? scottmcm
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
let len = self.end - self.start; |
||
ptr::slice_from_raw_parts(ptr, len) as *const str |
||
unsafe { |
||
let new_len = unchecked_sub(self.end, self.start); |
Does this get different codegen if you just use the public stdlib unchecked_sub?
It gets a boatload more MIR, or at least it will once #121571 lands.
(We really need a "look, nobody cares about the parameter debug info from this method" for things like that and i32:PartialOrd and …)
Ah okay! Cool then.
Member
Author
No, but it doubles up on the precondition check.
Member
|
Ah, good catch! |
added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
labels
Contributor
|
☀️ Test successful - checks-actions |
Collaborator
|
Finished benchmarking commit (4e431fa): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results CyclesResults Binary sizeResults Bootstrap: 667.899s -> 667.449s (-0.07%) |
Member
@rustbot label: +perf-regression-triaged |
added the perf-regression-triaged The performance regression has been triaged. label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK