Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

document when atomic loads are guaranteed read-only #115577

Merged
merged 9 commits into from
Oct 17, 2023
Merged

document when atomic loads are guaranteed read-only #115577

Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b5e67a0
document when atomic loads are guaranteed read-only
RalfJung Sep 5, 2023
7453235
add ARM and RISC-V values
RalfJung Sep 5, 2023
07b8c10
add general powerpc64le bound
RalfJung Sep 8, 2023
69b62ec
define 'read-only memory'
RalfJung Sep 8, 2023
275d5c8
wording
RalfJung Sep 9, 2023
9b8686d
only guarantee for Relaxed; add ptr-size fallback
RalfJung Oct 14, 2023
9d8506d
acquire loads can be done as relaxed load; acquire fence
RalfJung Oct 15, 2023
6605116
use target-arch based table
RalfJung Oct 16, 2023
e494df4
remove 128bit atomics, they are anyway not exposed on those targets
RalfJung Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions library/core/src/sync/atomic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,41 @@
//!
//! [lock-free]: https://en.wikipedia.org/wiki/Non-blocking_algorithm
//!
//! # Atomic accesses to read-only memory
//!
//! In general, *all* atomic accesses on read-only memory are Undefined Behavior. For instance, attempting
//! to do a `compare_exchange` that will definitely fail (making it conceptually a read-only
//! operation) can still cause a page fault if the underlying memory page is mapped read-only. Since
//! atomic `load`s might be implemented using compare-exchange operations, even a `load` can fault
//! on read-only memory.
//!
//! For the purpose of this section, "read-only memory" is defined as memory that is read-only in
//! the underlying target, i.e., the pages are mapped with a read-only flag and any attempt to write
//! will cause a page fault. In particular, an `&u128` reference that points to memory that is
//! read-write mapped is *not* considered to point to "read-only memory". In Rust, almost all memory
//! is read-write; the only exceptions are memory created by `const` items or `static` items without
//! interior mutability, and memory that was specifically marked as read-only by the operating
//! system via platform-specific APIs.
//!
//! As an exception from the general rule stated above, "sufficiently small" atomic loads with
//! `Ordering::Relaxed` are implemented in a way that works on read-only memory, and are hence not
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
//! Undefined Behavior. The exact size limit for what makes a load "sufficiently small" varies
//! depending on the target:
//!
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
//! | `target_arch` | Size limit |
//! |---------------|---------|
//! | `x86`, `arm`, `mips`, `mips32r6, `powerpc`, `riscv32`, `sparc`, `hexagon` | 4 bytes |
//! | `x86_64`, `aarch64`, `loongarch64`, `mips64`, `mips64r6`, `powerpc64`, `riscv64`, `sparc64` | 8 bytes |
//! | `s390x`, `powerpc64` with `target_feature = "quadword-atomics"` | 16 bytes |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quadword-atomics is not a valid rust target feature. Also note that even if valid, the powerpc target feature is not stable.

rust/compiler/rustc_codegen_ssa/src/target_features.rs

Lines 225 to 235 in 49691b1

const POWERPC_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
// tidy-alphabetical-start
("altivec", Some(sym::powerpc_target_feature)),
("power10-vector", Some(sym::powerpc_target_feature)),
("power8-altivec", Some(sym::powerpc_target_feature)),
("power8-vector", Some(sym::powerpc_target_feature)),
("power9-altivec", Some(sym::powerpc_target_feature)),
("power9-vector", Some(sym::powerpc_target_feature)),
("vsx", Some(sym::powerpc_target_feature)),
// tidy-alphabetical-end
];

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we have some targets that enable it. How do you suggest we reflect that in the list?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is reasonable to document it once the 128-bit atomics in that target and the corresponding target feature have stabilized.

//!
//! Atomics loads that are larger than this limit as well as atomic loads with ordering other
//! than `Relaxed`, as well as *all* atomic loads on targets not listed in the table, might still be
//! read-only under certain conditions, but that is not a stable guarantee and should not be relied
//! upon.
//!
//! If you need to do an acquire load on read-only memory, you can do a relaxed load followed by an
//! acquire fence instead.
//!
//! # Examples
//!
//! A simple spinlock:
Expand Down
Loading