Add splitting caveats to {read,write}_volatile#156931
Open
scottmcm wants to merge 1 commit into
Open
Conversation
Collaborator
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
RalfJung
reviewed
May 26, 2026
| /// | ||
| /// # Load splitting | ||
| /// | ||
| /// Exactly which hardware loads are performed by this function are, in general, highly target-dependent. |
Member
There was a problem hiding this comment.
Suggested change
| /// Exactly which hardware loads are performed by this function are, in general, highly target-dependent. | |
| /// Exactly which hardware loads are performed by this function is, in general, highly target-dependent. |
| /// Exactly which hardware loads are performed by this function are, in general, highly target-dependent. | ||
| /// | ||
| /// For a simple scalar, such as when `T` is a thin pointer, this will typically be one load assuming | ||
| /// your target supports a load of exactly that size. |
Member
There was a problem hiding this comment.
Suggested change
| /// your target supports a load of exactly that size. | |
| /// your target supports a load of exactly that size and alignment. |
| /// | ||
| /// For anything else, it will be split into multiple loads in some unspecified way. This can happen | ||
| /// even for scalars: notably, on many targets loading a `u128` will still need to be split, despite | ||
| /// being "one" scalar. A load over, say, 64 bytes will almost certainly need to be split. Any load |
Member
There was a problem hiding this comment.
Suggested change
| /// being "one" scalar. A load over, say, 64 bytes will almost certainly need to be split. Any load | |
| /// being "one" scalar. A load larger than, say, 64 bytes will almost certainly need to be split. Any load |
| /// | ||
| /// For anything else, it will be split into multiple loads in some unspecified way. This can happen | ||
| /// even for scalars: notably, on many targets loading a `u128` will still need to be split, despite | ||
| /// being "one" scalar. A load over, say, 64 bytes will almost certainly need to be split. Any load |
Contributor
There was a problem hiding this comment.
is this really intended to say 64 bytes (not bits)? as far as i am aware, on most supported platforms, splits tend to happen for anything larger than usize, and as the previous sentence says, even u128 is split on x86_64.
Maybe this should say "larger than size_of::<usize>()" or similar?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It sounds like we don't actually have guarantees right now (#t-opsem > Defining volatile splitting @ 💬) so let's document that. I think this doesn't need an FCP since it's not actually guaranteeing anything.