Skip to content

Mark const ptr methods and free functions as inline(always) to match *mut - #160816

Open
zakrad wants to merge 1 commit into
rust-lang:mainfrom
zakrad:fix-const-ptr-read-inline-160809
Open

Mark const ptr methods and free functions as inline(always) to match *mut#160816
zakrad wants to merge 1 commit into
rust-lang:mainfrom
zakrad:fix-const-ptr-read-inline-160809

Conversation

@zakrad

@zakrad zakrad commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Closes #160809. The *const read/read_volatile/read_unaligned methods were #[inline] while the *mut equivalents are #[inline(always)] and verified the *const forms now inline in debug instead of emitting an out of line call.

@rustbot 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 Aug 9, 2026
@rustbot

rustbot commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

r? @nia-e

rustbot has assigned @nia-e.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, nia-e

@nia-e

nia-e commented Aug 9, 2026

Copy link
Copy Markdown
Member

this should probably also cover std::ptr::read & co, as per the linked issue, but otherwise looks good ^^

@rustbot author

@rustbot 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 Aug 9, 2026
@rustbot

rustbot commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@zakrad
zakrad force-pushed the fix-const-ptr-read-inline-160809 branch from 5b7cb48 to 3206bcf Compare August 9, 2026 18:52
@zakrad

zakrad commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

this should probably also cover std::ptr::read & co, as per the linked issue, but otherwise looks good ^^

@rustbot author

Thanks! Added #[inline(always)] to the free functions too

@nia-e

nia-e commented Aug 9, 2026

Copy link
Copy Markdown
Member

I did a thorough check and there's a fair few other places (though these should also be quite trivial):

  • copy_to, copy_to_nonoverlapping, offset_from, and (on slices) len are #[inline(always)] only on *mut T but not on *const T;
  • write, write_unaligned, write_volatile, replace, and swap are #[inline(always)] on *mut T but not on the functions in ptr/mod.rs;
  • same with as_ptr vs as_mut_ptr and get_unchecked vs get_unchecked_mut on *const T & *mut T;
  • the impls of PartialEq and PartialOrd also have different inline attrs on *const T and *mut T.

once these are all inline(always), that should actually be eveything :D ty!

Many <*const T> methods (read family, copy_to{,_nonoverlapping}, offset_from,
len, as_ptr, get_unchecked), the ptr::{read,write,replace,swap,...} free
functions, and the PartialEq/PartialOrd impls were left as inline while their
<*mut T> equivalents are inline(always). This left them with out-of-line calls
in debug codegen; make them consistent.
@zakrad
zakrad force-pushed the fix-const-ptr-read-inline-160809 branch from 3206bcf to 1b7548a Compare August 9, 2026 19:36
@zakrad zakrad changed the title Mark <*const T>::read family as inline(always) to match <*mut T> Mark const ptr methods and free functions as inline(always) to match *mut Aug 9, 2026
@nia-e

nia-e commented Aug 9, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 1b7548a has been approved by nia-e

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 9, 2026
@zakrad

zakrad commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the help, I actually spotted a few of these and was planning for a follow up, but since you listed them all here it is.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2026
@nia-e nia-e removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 9, 2026
…60809, r=nia-e

Mark const ptr methods and free functions as inline(always) to match *mut

Closes rust-lang#160809. The `*const`  read/read_volatile/read_unaligned methods were `#[inline]` while the `*mut` equivalents are `#[inline(always)]` and verified the `*const` forms now inline in debug instead of emitting an out of line call.
rust-bors Bot pushed a commit that referenced this pull request Aug 10, 2026
Rollup of 7 pull requests

Successful merges:

 - #160533 (dirfd dir operations (3/4))
 - #160675 (bootstrap: Remove `PATH_REMAP` from command-line selector handling )
 - #160807 (Update rustc crate rkyv to 0.8.18)
 - #160560 (Add nightly-only support for Cargo unremap trim-paths files in `rust-gdb`)
 - #160804 (Change .expect message on net/parser to follow precondition style)
 - #160805 (`extern "custom"`: add tests)
 - #160816 (Mark const ptr methods and free functions as inline(always) to match *mut)
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 10, 2026
…60809, r=nia-e

Mark const ptr methods and free functions as inline(always) to match *mut

Closes rust-lang#160809. The `*const`  read/read_volatile/read_unaligned methods were `#[inline]` while the `*mut` equivalents are `#[inline(always)]` and verified the `*const` forms now inline in debug instead of emitting an out of line call.
@saethlin

Copy link
Copy Markdown
Member

This PR is likely perf sensitive because of the attribute changes and is not a good candidate for adding to a roll-up. I would have marked it iffy and perfed it before merge. But given the recent queue slowness I won't r- it, someone can just perf it after merge.

The always variant of the inline attribute is much less of a hint, and it tends to produce the desired inlining even in unoptimized builds and it changes unoptimized monomorphization to do this. I think it should be used very delicately in libraries.

@nia-e

nia-e commented Aug 10, 2026

Copy link
Copy Markdown
Member

I'm happy to do a perf run, but the benchmarks in question were mostly run already in #85218 so I didn't see the case for marking this as perf-sensitive independent of that - I'd be surprised if it's not just more of the same.

if it's useful:
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 10, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 10, 2026
Mark const ptr methods and free functions as inline(always) to match *mut
rust-bors Bot pushed a commit that referenced this pull request Aug 10, 2026
Rollup of 11 pull requests

Successful merges:

 - #160675 (bootstrap: Remove `PATH_REMAP` from command-line selector handling )
 - #160807 (Update rustc crate rkyv to 0.8.18)
 - #159690 (Clarify `--remap-path-scope` impact on `rustc` metadata)
 - #160560 (Add nightly-only support for Cargo unremap trim-paths files in `rust-gdb`)
 - #160608 (normalization rework: clean up projection_ty_core)
 - #160785 (Get rid of LLM disclosure checkboxes)
 - #160804 (Change .expect message on net/parser to follow precondition style)
 - #160805 (`extern "custom"`: add tests)
 - #160816 (Mark const ptr methods and free functions as inline(always) to match *mut)
 - #160820 (Stabilize fs_set_times)
 - #160826 (Rename parse_delimited_token_tree in cfg_select)
@rust-bors

rust-bors Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 0f1a713 (0f1a713f5caf1917f14cfe76880d8de8a3551187)
Base parent: 969b803 (969b803cbe1d4499f841ae0a49c637d8c70a0458)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (0f1a713): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.2%, 0.5%] 7
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.7% [-0.7%, -0.7%] 1

Max RSS (memory usage)

Results (primary -0.6%, secondary 0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.8% [2.1%, 3.5%] 2
Regressions ❌
(secondary)
3.6% [0.4%, 12.5%] 5
Improvements ✅
(primary)
-4.0% [-5.4%, -2.6%] 2
Improvements ✅
(secondary)
-1.2% [-4.2%, -0.4%] 6
All ❌✅ (primary) -0.6% [-5.4%, 3.5%] 4

Cycles

Results (primary -2.4%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.2% [0.4%, 4.3%] 10
Improvements ✅
(primary)
-2.4% [-2.4%, -2.4%] 1
Improvements ✅
(secondary)
-1.2% [-3.2%, -0.4%] 9
All ❌✅ (primary) -2.4% [-2.4%, -2.4%] 1

Binary size

Results (primary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.2% [0.0%, 0.2%] 5
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [-0.4%, 0.2%] 7

Bootstrap: 458.441s -> 464.25s (1.27%)
Artifact size: 398.59 MiB -> 398.66 MiB (0.02%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 10, 2026
@nia-e

nia-e commented Aug 10, 2026

Copy link
Copy Markdown
Member

yeah, this reads as noise to me. I'm not sure if there's much to read into here ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

*mut's read() is #[inline(always)], but *const's read() is #[inline]

5 participants