From 8069abb0350c9adbe8629c26e3b412a0f671f1b8 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 2 Apr 2025 15:04:03 -0400 Subject: [PATCH 1/7] Move read_volatile to volatile module and implement thumbv6 in asm. The goal is to ensure LLVM doesn't do anything silly (:3) with the loads. --- library/core/src/ptr/mod.rs | 83 +------------------- library/core/src/ptr/volatile.rs | 127 +++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 80 deletions(-) create mode 100644 library/core/src/ptr/volatile.rs diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index ea53da78d3b..a5a66b33d6f 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -425,9 +425,12 @@ pub use non_null::NonNull; mod unique; #[unstable(feature = "ptr_internals", issue = "none")] pub use unique::Unique; +#[stable(feature = "volatile", since = "1.9.0")] +pub use volatile::read_volatile; mod const_ptr; mod mut_ptr; +mod volatile; /// Executes the destructor (if any) of the pointed-to value. /// @@ -1669,86 +1672,6 @@ pub const unsafe fn write_unaligned(dst: *mut T, src: T) { } } -/// Performs a volatile read of the value from `src` without moving it. This -/// leaves the memory in `src` unchanged. -/// -/// Volatile operations are intended to act on I/O memory, and are guaranteed -/// to not be elided or reordered by the compiler across other volatile -/// operations. -/// -/// # Notes -/// -/// Rust does not currently have a rigorously and formally defined memory model, -/// so the precise semantics of what "volatile" means here is subject to change -/// over time. That being said, the semantics will almost always end up pretty -/// similar to [C11's definition of volatile][c11]. -/// -/// The compiler shouldn't change the relative order or number of volatile -/// memory operations. However, volatile memory operations on zero-sized types -/// (e.g., if a zero-sized type is passed to `read_volatile`) are noops -/// and may be ignored. -/// -/// [c11]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf -/// -/// # Safety -/// -/// Behavior is undefined if any of the following conditions are violated: -/// -/// * `src` must be [valid] for reads. -/// -/// * `src` must be properly aligned. -/// -/// * `src` must point to a properly initialized value of type `T`. -/// -/// Like [`read`], `read_volatile` creates a bitwise copy of `T`, regardless of -/// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the returned -/// value and the value at `*src` can [violate memory safety][read-ownership]. -/// However, storing non-[`Copy`] types in volatile memory is almost certainly -/// incorrect. -/// -/// Note that even if `T` has size `0`, the pointer must be properly aligned. -/// -/// [valid]: self#safety -/// [read-ownership]: read#ownership-of-the-returned-value -/// -/// Just like in C, whether an operation is volatile has no bearing whatsoever -/// on questions involving concurrent access from multiple threads. Volatile -/// accesses behave exactly like non-atomic accesses in that regard. In particular, -/// a race between a `read_volatile` and any write operation to the same location -/// is undefined behavior. -/// -/// # Examples -/// -/// Basic usage: -/// -/// ``` -/// let x = 12; -/// let y = &x as *const i32; -/// -/// unsafe { -/// assert_eq!(std::ptr::read_volatile(y), 12); -/// } -/// ``` -#[inline] -#[stable(feature = "volatile", since = "1.9.0")] -#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces -#[rustc_diagnostic_item = "ptr_read_volatile"] -pub unsafe fn read_volatile(src: *const T) -> T { - // SAFETY: the caller must uphold the safety contract for `volatile_load`. - unsafe { - ub_checks::assert_unsafe_precondition!( - check_language_ub, - "ptr::read_volatile requires that the pointer argument is aligned and non-null", - ( - addr: *const () = src as *const (), - align: usize = align_of::(), - is_zst: bool = T::IS_ZST, - ) => ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) - ); - intrinsics::volatile_load(src) - } -} - /// Performs a volatile write of a memory location with the given value without /// reading or dropping the old value. /// diff --git a/library/core/src/ptr/volatile.rs b/library/core/src/ptr/volatile.rs new file mode 100644 index 00000000000..2f6dbd16311 --- /dev/null +++ b/library/core/src/ptr/volatile.rs @@ -0,0 +1,127 @@ +use crate::{mem::SizedTypeProperties, cfg_match, intrinsics}; + +/// Performs a volatile read of the value from `src` without moving it. This +/// leaves the memory in `src` unchanged. +/// +/// Volatile operations are intended to act on I/O memory, and are guaranteed +/// to not be elided or reordered by the compiler across other volatile +/// operations. +/// +/// # Notes +/// +/// Rust does not currently have a rigorously and formally defined memory model, +/// so the precise semantics of what "volatile" means here is subject to change +/// over time. That being said, the semantics will almost always end up pretty +/// similar to [C11's definition of volatile][c11]. +/// +/// The compiler shouldn't change the relative order or number of volatile +/// memory operations. However, volatile memory operations on zero-sized types +/// (e.g., if a zero-sized type is passed to `read_volatile`) are noops +/// and may be ignored. +/// +/// [c11]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf +/// +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `src` must be [valid] for reads. +/// +/// * `src` must be properly aligned. +/// +/// * `src` must point to a properly initialized value of type `T`. +/// +/// Like [`read`], `read_volatile` creates a bitwise copy of `T`, regardless of +/// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the returned +/// value and the value at `*src` can [violate memory safety][read-ownership]. +/// However, storing non-[`Copy`] types in volatile memory is almost certainly +/// incorrect. +/// +/// Note that even if `T` has size `0`, the pointer must be properly aligned. +/// +/// [valid]: self#safety +/// [read-ownership]: read#ownership-of-the-returned-value +/// +/// Just like in C, whether an operation is volatile has no bearing whatsoever +/// on questions involving concurrent access from multiple threads. Volatile +/// accesses behave exactly like non-atomic accesses in that regard. In particular, +/// a race between a `read_volatile` and any write operation to the same location +/// is undefined behavior. +/// +/// # Examples +/// +/// Basic usage: +/// +/// ``` +/// let x = 12; +/// let y = &x as *const i32; +/// +/// unsafe { +/// assert_eq!(std::ptr::read_volatile(y), 12); +/// } +/// ``` +#[inline] +#[stable(feature = "volatile", since = "1.9.0")] +#[cfg_attr(miri, track_caller)] // Even without panics, this helps for Miri backtraces +#[rustc_diagnostic_item = "ptr_read_volatile"] +pub unsafe fn read_volatile(src: *const T) -> T { + // SAFETY: the caller must uphold the safety contract for `volatile_load`. + unsafe { + crate::ub_checks::assert_unsafe_precondition!( + check_language_ub, + "ptr::read_volatile requires that the pointer argument is aligned and non-null", + ( + addr: *const () = src as *const (), + align: usize = align_of::(), + is_zst: bool = T::IS_ZST, + ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) + ); + cfg_match! { + all(target_arch = "arm", target_feature = "thumb-mode", target_pointer_width = "32") => { + { + use crate::arch::asm; + use crate::mem::MaybeUninit; + + match size_of::() { + // For the relevant sizes, ensure that just a single load is emitted + // for the read with nothing merged or split. + 1 => { + let byte: MaybeUninit::; + asm!( + "ldrb {out}, [{in}]", + in = in(reg) src, + out = out(reg) byte + ); + + intrinsics::transmute_unchecked(byte) + } + 2 => { + let halfword: MaybeUninit::; + asm!( + "ldrh {out}, [{in}]", + in = in(reg) src, + out = out(reg) halfword + ); + + intrinsics::transmute_unchecked(halfword) + }, + 4 => { + let word: MaybeUninit::; + asm!( + "ldr {out}, [{in}]", + in = in(reg) src, + out = out(reg) word + ); + + intrinsics::transmute_unchecked(word) + }, + // Anything else is mostly meaningless. + _ => intrinsics::volatile_load(src), + } + }} + _ => { + intrinsics::volatile_load(src) + } + } + } +} From e55c95b18bafd705f0f4c98dc4923bf7f0d7bb6c Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 2 Apr 2025 22:18:30 -0400 Subject: [PATCH 2/7] Move write_volatile to the volatile module --- library/core/src/ptr/mod.rs | 83 +------------------------------ library/core/src/ptr/volatile.rs | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 81 deletions(-) diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index a5a66b33d6f..9ef308e39a0 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -425,8 +425,9 @@ pub use non_null::NonNull; mod unique; #[unstable(feature = "ptr_internals", issue = "none")] pub use unique::Unique; + #[stable(feature = "volatile", since = "1.9.0")] -pub use volatile::read_volatile; +pub use volatile::{read_volatile, write_volatile}; mod const_ptr; mod mut_ptr; @@ -1672,86 +1673,6 @@ pub const unsafe fn write_unaligned(dst: *mut T, src: T) { } } -/// Performs a volatile write of a memory location with the given value without -/// reading or dropping the old value. -/// -/// Volatile operations are intended to act on I/O memory, and are guaranteed -/// to not be elided or reordered by the compiler across other volatile -/// operations. -/// -/// `write_volatile` does not drop the contents of `dst`. This is safe, but it -/// could leak allocations or resources, so care should be taken not to overwrite -/// an object that should be dropped. -/// -/// Additionally, it does not drop `src`. Semantically, `src` is moved into the -/// location pointed to by `dst`. -/// -/// # Notes -/// -/// Rust does not currently have a rigorously and formally defined memory model, -/// so the precise semantics of what "volatile" means here is subject to change -/// over time. That being said, the semantics will almost always end up pretty -/// similar to [C11's definition of volatile][c11]. -/// -/// The compiler shouldn't change the relative order or number of volatile -/// memory operations. However, volatile memory operations on zero-sized types -/// (e.g., if a zero-sized type is passed to `write_volatile`) are noops -/// and may be ignored. -/// -/// [c11]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf -/// -/// # Safety -/// -/// Behavior is undefined if any of the following conditions are violated: -/// -/// * `dst` must be [valid] for writes. -/// -/// * `dst` must be properly aligned. -/// -/// Note that even if `T` has size `0`, the pointer must be properly aligned. -/// -/// [valid]: self#safety -/// -/// Just like in C, whether an operation is volatile has no bearing whatsoever -/// on questions involving concurrent access from multiple threads. Volatile -/// accesses behave exactly like non-atomic accesses in that regard. In particular, -/// a race between a `write_volatile` and any other operation (reading or writing) -/// on the same location is undefined behavior. -/// -/// # Examples -/// -/// Basic usage: -/// -/// ``` -/// let mut x = 0; -/// let y = &mut x as *mut i32; -/// let z = 12; -/// -/// unsafe { -/// std::ptr::write_volatile(y, z); -/// assert_eq!(std::ptr::read_volatile(y), 12); -/// } -/// ``` -#[inline] -#[stable(feature = "volatile", since = "1.9.0")] -#[rustc_diagnostic_item = "ptr_write_volatile"] -#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces -pub unsafe fn write_volatile(dst: *mut T, src: T) { - // SAFETY: the caller must uphold the safety contract for `volatile_store`. - unsafe { - ub_checks::assert_unsafe_precondition!( - check_language_ub, - "ptr::write_volatile requires that the pointer argument is aligned and non-null", - ( - addr: *mut () = dst as *mut (), - align: usize = align_of::(), - is_zst: bool = T::IS_ZST, - ) => ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) - ); - intrinsics::volatile_store(dst, src); - } -} - /// Align pointer `p`. /// /// Calculate offset (in terms of elements of `size_of::()` stride) that has to be applied diff --git a/library/core/src/ptr/volatile.rs b/library/core/src/ptr/volatile.rs index 2f6dbd16311..7bdf9bf2c0d 100644 --- a/library/core/src/ptr/volatile.rs +++ b/library/core/src/ptr/volatile.rs @@ -125,3 +125,87 @@ pub unsafe fn read_volatile(src: *const T) -> T { } } } + +/// Performs a volatile write of a memory location with the given value without +/// reading or dropping the old value. +/// +/// Volatile operations are intended to act on I/O memory, and are guaranteed +/// to not be elided or reordered by the compiler across other volatile +/// operations. +/// +/// `write_volatile` does not drop the contents of `dst`. This is safe, but it +/// could leak allocations or resources, so care should be taken not to overwrite +/// an object that should be dropped. +/// +/// Additionally, it does not drop `src`. Semantically, `src` is moved into the +/// location pointed to by `dst`. +/// +/// # Notes +/// +/// Rust does not currently have a rigorously and formally defined memory model, +/// so the precise semantics of what "volatile" means here is subject to change +/// over time. That being said, the semantics will almost always end up pretty +/// similar to [C11's definition of volatile][c11]. +/// +/// The compiler shouldn't change the relative order or number of volatile +/// memory operations. However, volatile memory operations on zero-sized types +/// (e.g., if a zero-sized type is passed to `write_volatile`) are noops +/// and may be ignored. +/// +/// [c11]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf +/// +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `dst` must be [valid] for writes. +/// +/// * `dst` must be properly aligned. +/// +/// Note that even if `T` has size `0`, the pointer must be properly aligned. +/// +/// [valid]: self#safety +/// +/// Just like in C, whether an operation is volatile has no bearing whatsoever +/// on questions involving concurrent access from multiple threads. Volatile +/// accesses behave exactly like non-atomic accesses in that regard. In particular, +/// a race between a `write_volatile` and any other operation (reading or writing) +/// on the same location is undefined behavior. +/// +/// # Examples +/// +/// Basic usage: +/// +/// ``` +/// let mut x = 0; +/// let y = &mut x as *mut i32; +/// let z = 12; +/// +/// unsafe { +/// std::ptr::write_volatile(y, z); +/// assert_eq!(std::ptr::read_volatile(y), 12); +/// } +/// ``` +#[inline] +#[stable(feature = "volatile", since = "1.9.0")] +#[cfg_attr(miri, track_caller)] // Even without panics, this helps for Miri backtraces +#[rustc_diagnostic_item = "ptr_write_volatile"] +pub unsafe fn write_volatile(dst: *mut T, src: T) { + // SAFETY: the caller must uphold the safety contract for `volatile_write`. + unsafe { + crate::ub_checks::assert_unsafe_precondition!( + check_language_ub, + "ptr::write_volatile requires that the pointer argument is aligned and non-null", + ( + addr: *mut () = dst as *mut (), + align: usize = crate::mem::align_of::(), + is_zst: bool = T::IS_ZST, + ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) + ); + cfg_match! { + _ => { + intrinsics::volatile_store(dst, src); + } + } + } +} From 6910d0fe1a7cb1386f94d103d4b5d3782b646f4a Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 2 Apr 2025 22:40:00 -0400 Subject: [PATCH 3/7] Add thumb asm volatile writes --- library/core/src/ptr/mod.rs | 1 - library/core/src/ptr/volatile.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 9ef308e39a0..eddaa4103ab 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -425,7 +425,6 @@ pub use non_null::NonNull; mod unique; #[unstable(feature = "ptr_internals", issue = "none")] pub use unique::Unique; - #[stable(feature = "volatile", since = "1.9.0")] pub use volatile::{read_volatile, write_volatile}; diff --git a/library/core/src/ptr/volatile.rs b/library/core/src/ptr/volatile.rs index 7bdf9bf2c0d..b48bba6405b 100644 --- a/library/core/src/ptr/volatile.rs +++ b/library/core/src/ptr/volatile.rs @@ -1,4 +1,5 @@ -use crate::{mem::SizedTypeProperties, cfg_match, intrinsics}; +use crate::mem::SizedTypeProperties; +use crate::{cfg_match, intrinsics}; /// Performs a volatile read of the value from `src` without moving it. This /// leaves the memory in `src` unchanged. @@ -203,6 +204,33 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) ); cfg_match! { + all(target_arch = "arm", target_feature = "thumb-mode", target_pointer_width = "32") => { + { + use crate::arch::asm; + use crate::mem::MaybeUninit; + + match size_of::() { + 1 => + asm!( + "strb {val}, [{dest}]", + val = in(reg) intrinsics::transmute_unchecked::>(src), + dest = in(reg) dst + ), + + 2 => asm!( + "strh {val}, [{dest}]", + val = in(reg) intrinsics::transmute_unchecked::>(src), + dest = in(reg) dst, + ), + 4 => asm!( + "str {val}, [{dest}]", + val = in(reg) intrinsics::transmute_unchecked::>(src), + dest = in(reg) dst + ), + _ => intrinsics::volatile_store(dst, src) + } + } + } _ => { intrinsics::volatile_store(dst, src); } From ec83f3051f1495b4d8181705d4ddc89b384b3b72 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Thu, 3 Apr 2025 12:25:19 -0400 Subject: [PATCH 4/7] Fix links --- library/core/src/ptr/volatile.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/library/core/src/ptr/volatile.rs b/library/core/src/ptr/volatile.rs index b48bba6405b..1dc36290066 100644 --- a/library/core/src/ptr/volatile.rs +++ b/library/core/src/ptr/volatile.rs @@ -32,7 +32,7 @@ use crate::{cfg_match, intrinsics}; /// /// * `src` must point to a properly initialized value of type `T`. /// -/// Like [`read`], `read_volatile` creates a bitwise copy of `T`, regardless of +/// Like [read], `read_volatile` creates a bitwise copy of `T`, regardless of /// whether `T` is [`Copy`]. If `T` is not [`Copy`], using both the returned /// value and the value at `*src` can [violate memory safety][read-ownership]. /// However, storing non-[`Copy`] types in volatile memory is almost certainly @@ -40,8 +40,9 @@ use crate::{cfg_match, intrinsics}; /// /// Note that even if `T` has size `0`, the pointer must be properly aligned. /// -/// [valid]: self#safety -/// [read-ownership]: read#ownership-of-the-returned-value +/// [valid]: crate::ptr#safety +/// [read-ownership]: crate::ptr::read#ownership-of-the-returned-value +/// [read]: crate::ptr::read /// /// Just like in C, whether an operation is volatile has no bearing whatsoever /// on questions involving concurrent access from multiple threads. Volatile @@ -165,7 +166,7 @@ pub unsafe fn read_volatile(src: *const T) -> T { /// /// Note that even if `T` has size `0`, the pointer must be properly aligned. /// -/// [valid]: self#safety +/// [valid]: crate::ptr#safety /// /// Just like in C, whether an operation is volatile has no bearing whatsoever /// on questions involving concurrent access from multiple threads. Volatile From c7f50ec379e47f68615c8dd56260e70fa91f7ec3 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:38:13 -0400 Subject: [PATCH 5/7] asm -> atomic_relaxed TODO: atomic_volatile_relaxed --- library/core/src/ptr/volatile.rs | 123 ++++++++++++------------------- 1 file changed, 46 insertions(+), 77 deletions(-) diff --git a/library/core/src/ptr/volatile.rs b/library/core/src/ptr/volatile.rs index 1dc36290066..9f9ef024cb1 100644 --- a/library/core/src/ptr/volatile.rs +++ b/library/core/src/ptr/volatile.rs @@ -1,5 +1,7 @@ use crate::mem::SizedTypeProperties; -use crate::{cfg_match, intrinsics}; +use crate::intrinsics; +use crate::macros::cfg; +use crate::sync::atomic::{AtomicU8, AtomicU16, AtomicU32, Atomicu64}; /// Performs a volatile read of the value from `src` without moving it. This /// leaves the memory in `src` unchanged. @@ -78,52 +80,28 @@ pub unsafe fn read_volatile(src: *const T) -> T { is_zst: bool = T::IS_ZST, ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) ); - cfg_match! { - all(target_arch = "arm", target_feature = "thumb-mode", target_pointer_width = "32") => { - { - use crate::arch::asm; - use crate::mem::MaybeUninit; - - match size_of::() { - // For the relevant sizes, ensure that just a single load is emitted - // for the read with nothing merged or split. - 1 => { - let byte: MaybeUninit::; - asm!( - "ldrb {out}, [{in}]", - in = in(reg) src, - out = out(reg) byte - ); - - intrinsics::transmute_unchecked(byte) - } - 2 => { - let halfword: MaybeUninit::; - asm!( - "ldrh {out}, [{in}]", - in = in(reg) src, - out = out(reg) halfword - ); - - intrinsics::transmute_unchecked(halfword) - }, - 4 => { - let word: MaybeUninit::; - asm!( - "ldr {out}, [{in}]", - in = in(reg) src, - out = out(reg) word - ); - - intrinsics::transmute_unchecked(word) - }, - // Anything else is mostly meaningless. - _ => intrinsics::volatile_load(src), - } - }} - _ => { - intrinsics::volatile_load(src) - } + match size_of() { + 1 => if cfg!(target_has_atomic_load_store = "8") && align_of::() == align_of::() { + intrinsics::atomic_load_relaxed(src) + } else { + intrinsics::volatile_load(dst, val) + } + 2 => if cfg!(target_has_atomic_load_store = "16") && align_of::() == align_of::() { + intrinsics::atomic_load_relaxed(src) + } else { + intrinsics::volatile_load(dst, val) + } + 4 => if cfg!(target_has_atomic_load_store = "32") && align_of::() == align_of::() { + intrinsics::atomic_load_relaxed(src) + } else { + intrinsics::volatile_load(dst, val) + } + 8 => if cfg!(target_has_atomic_load_store = "64") && align_of::() == align_of::() { + intrinsics::atomic_load_relaxed(src) + } else { + intrinsics::volatile_load(dst, val) + } + _ => intrinsics::volatile_load(dst, val) } } } @@ -204,37 +182,28 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { is_zst: bool = T::IS_ZST, ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) ); - cfg_match! { - all(target_arch = "arm", target_feature = "thumb-mode", target_pointer_width = "32") => { - { - use crate::arch::asm; - use crate::mem::MaybeUninit; - - match size_of::() { - 1 => - asm!( - "strb {val}, [{dest}]", - val = in(reg) intrinsics::transmute_unchecked::>(src), - dest = in(reg) dst - ), - - 2 => asm!( - "strh {val}, [{dest}]", - val = in(reg) intrinsics::transmute_unchecked::>(src), - dest = in(reg) dst, - ), - 4 => asm!( - "str {val}, [{dest}]", - val = in(reg) intrinsics::transmute_unchecked::>(src), - dest = in(reg) dst - ), - _ => intrinsics::volatile_store(dst, src) - } + match size_of() { + 1 => if cfg!(target_has_atomic_load_store = "8") && align_of::() == align_of::() { + intrinsics::atomic_store_relaxed(dst, val) + } else { + intrinsics::volatile_store(dst, val) } - } - _ => { - intrinsics::volatile_store(dst, src); - } + 2 => if cfg!(target_has_atomic_load_store = "16") && align_of::() == align_of::() { + intrinsics::atomic_store_relaxed(dst, val) + } else { + intrinsics::volatile_store(dst, val) + } + 4 => if cfg!(target_has_atomic_load_store = "32") && align_of::() == align_of::() { + intrinsics::atomic_store_relaxed(dst, val) + } else { + intrinsics::volatile_store(dst, val) + } + 8 => if cfg!(target_has_atomic_load_store = "64") && align_of::() == align_of::() { + intrinsics::atomic_store_relaxed(dst, val) + } else { + intrinsics::volatile_store(dst, val) + } + _ => intrinsics::volatile_store(dst, val) } } } From fd1d1b59fe7470cded4c3ebdfdcb5f1a61adcf00 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:43:06 -0400 Subject: [PATCH 6/7] Add new volatile relaxed atomic intrinsic Copied the code from here: https://github.com/llvm/llvm-project/blob/24dfcc0c024f9ab8ba61c0994513f57e882961fc/llvm/lib/Frontend/Atomic/Atomic.cpp#L31-L32 Did not implement it for GCC or craneline. --- compiler/rustc_codegen_llvm/src/builder.rs | 4 ++ compiler/rustc_codegen_llvm/src/intrinsic.rs | 1 - compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 + .../rustc_codegen_ssa/src/mir/intrinsic.rs | 44 ++++++++++++++++++- .../rustc_codegen_ssa/src/traits/builder.rs | 2 + .../rustc_hir_analysis/src/check/intrinsic.rs | 8 +++- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 13 +++++- compiler/rustc_span/src/symbol.rs | 2 + 8 files changed, 70 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 297f104d124..e851c63a6cc 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -613,6 +613,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { ptr: &'ll Value, order: rustc_codegen_ssa::common::AtomicOrdering, size: Size, + is_volatile: bool, ) -> &'ll Value { unsafe { let load = llvm::LLVMRustBuildAtomicLoad( @@ -621,6 +622,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { ptr, UNNAMED, AtomicOrdering::from_generic(order), + is_volatile, ); // LLVM requires the alignment of atomic loads to be at least the size of the type. llvm::LLVMSetAlignment(load, size.bytes() as c_uint); @@ -851,6 +853,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { ptr: &'ll Value, order: rustc_codegen_ssa::common::AtomicOrdering, size: Size, + is_volatile: bool, ) { debug!("Store {:?} -> {:?}", val, ptr); assert_eq!(self.cx.type_kind(self.cx.val_ty(ptr)), TypeKind::Pointer); @@ -860,6 +863,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { val, ptr, AtomicOrdering::from_generic(order), + is_volatile, ); // LLVM requires the alignment of atomic stores to be at least the size of the type. llvm::LLVMSetAlignment(store, size.bytes() as c_uint); diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 67135fcc308..916d19207fd 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -285,7 +285,6 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { _ => bug!("the va_arg intrinsic does not work with non-scalar types"), } } - sym::volatile_load | sym::unaligned_volatile_load => { let tp_ty = fn_args.type_at(0); let ptr = args[0].immediate(); diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 3ce3761944b..776c4e41701 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1970,6 +1970,7 @@ unsafe extern "C" { PointerVal: &'a Value, Name: *const c_char, Order: AtomicOrdering, + isVolatile: bool, ) -> &'a Value; pub(crate) fn LLVMRustBuildAtomicStore<'a>( @@ -1977,6 +1978,7 @@ unsafe extern "C" { Val: &'a Value, Ptr: &'a Value, Order: AtomicOrdering, + isVolatile: bool, ) -> &'a Value; pub(crate) fn LLVMRustTimeTraceProfilerInitialize(); diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 63025a4574f..6049dc0d8d7 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -396,6 +396,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { source, parse_ordering(bx, ordering), size, + false, ) } else { invalid_monomorphization(ty); @@ -409,7 +410,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let size = bx.layout_of(ty).size; let val = args[1].immediate(); let ptr = args[0].immediate(); - bx.atomic_store(val, ptr, parse_ordering(bx, ordering), size); + bx.atomic_store(val, ptr, parse_ordering(bx, ordering), size, false); } else { invalid_monomorphization(ty); } @@ -491,7 +492,48 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } } } + sym::volatile_load_atomic_relaxed => { + use crate::common::AtomicOrdering; + let ty = fn_args.type_at(0); + if int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr() { + let layout = bx.layout_of(ty); + let size = layout.size; + let source = args[0].immediate(); + bx.atomic_load( + bx.backend_type(layout), + source, + AtomicOrdering::Relaxed, + size, + true, + ); + } else { + bx.tcx().dcx().emit_err(InvalidMonomorphization::BasicIntegerType { + span, + name, + ty, + }); + } + return Ok(()); + } + sym::volatile_store_atomic_relaxed => { + use crate::common::AtomicOrdering; + + let ty = fn_args.type_at(0); + if int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr() { + let size = bx.layout_of(ty).size; + let val = args[1].immediate(); + let ptr = args[0].immediate(); + bx.atomic_store(val, ptr, AtomicOrdering::Relaxed, size, true); + } else { + bx.tcx().dcx().emit_err(InvalidMonomorphization::BasicIntegerType { + span, + name, + ty, + }); + } + return Ok(()); + } sym::nontemporal_store => { let dst = args[0].deref(bx.cx()); args[1].val.nontemporal_store(bx, dst); diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index f66309cf340..d277b74ebb9 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -236,6 +236,7 @@ pub trait BuilderMethods<'a, 'tcx>: ptr: Self::Value, order: AtomicOrdering, size: Size, + is_volatile: bool, ) -> Self::Value; fn load_from_place(&mut self, ty: Self::Type, place: PlaceValue) -> Self::Value { assert_eq!(place.llextra, None); @@ -316,6 +317,7 @@ pub trait BuilderMethods<'a, 'tcx>: ptr: Self::Value, order: AtomicOrdering, size: Size, + is_volatile: bool, ); fn gep(&mut self, ty: Self::Type, ptr: Self::Value, indices: &[Self::Value]) -> Self::Value; diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 42d785c8dd0..0097255a91d 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -430,10 +430,14 @@ pub fn check_intrinsic_type( sym::roundf64 => (0, 0, vec![tcx.types.f64], tcx.types.f64), sym::roundf128 => (0, 0, vec![tcx.types.f128], tcx.types.f128), - sym::volatile_load | sym::unaligned_volatile_load => { + sym::volatile_load + | sym::unaligned_volatile_load + | sym::volatile_load_atomic_relaxed => { (1, 0, vec![Ty::new_imm_ptr(tcx, param(0))], param(0)) } - sym::volatile_store | sym::unaligned_volatile_store => { + sym::volatile_store + | sym::unaligned_volatile_store + | sym::volatile_store_atomic_relaxed => { (1, 0, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], tcx.types.unit) } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 53df59930f4..5d13a4b889e 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -604,19 +604,28 @@ extern "C" void LLVMRustSetAllowReassoc(LLVMValueRef V) { extern "C" LLVMValueRef LLVMRustBuildAtomicLoad(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Source, - const char *Name, LLVMAtomicOrdering Order) { + const char *Name, LLVMAtomicOrdering Order, LLVMBool isVolatile) { Value *Ptr = unwrap(Source); LoadInst *LI = unwrap(B)->CreateLoad(unwrap(Ty), Ptr, Name); LI->setAtomic(fromRust(Order)); + + // atomic volatile + if (isVolatile) + LI->setVolatile(true); return wrap(LI); } extern "C" LLVMValueRef LLVMRustBuildAtomicStore(LLVMBuilderRef B, LLVMValueRef V, LLVMValueRef Target, - LLVMAtomicOrdering Order) { + LLVMAtomicOrdering Order, + LLVMBool isVolatile) { StoreInst *SI = unwrap(B)->CreateStore(unwrap(V), unwrap(Target)); SI->setAtomic(fromRust(Order)); + + // atomic volatile + if (isVolatile) + SI->setVolatile(true); return wrap(SI); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index bc853fe9079..4371420f18e 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -2264,8 +2264,10 @@ symbols! { volatile_copy_memory, volatile_copy_nonoverlapping_memory, volatile_load, + volatile_load_atomic_relaxed, volatile_set_memory, volatile_store, + volatile_store_atomic_relaxed, vreg, vreg_low16, vsx, From 2628b10be76b51f6f52e0b50a7306955b87dc537 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:43:06 -0400 Subject: [PATCH 7/7] Use atomic volatile relaxed intrinsic in the implementation of volatile. --- library/core/src/intrinsics/mod.rs | 20 ++++++ library/core/src/ptr/volatile.rs | 104 ++++++++++++++++------------- 2 files changed, 77 insertions(+), 47 deletions(-) diff --git a/library/core/src/intrinsics/mod.rs b/library/core/src/intrinsics/mod.rs index 81e59a1f349..1ec84c1dc45 100644 --- a/library/core/src/intrinsics/mod.rs +++ b/library/core/src/intrinsics/mod.rs @@ -1841,6 +1841,26 @@ pub unsafe fn unaligned_volatile_load(src: *const T) -> T; #[rustc_diagnostic_item = "intrinsics_unaligned_volatile_store"] pub unsafe fn unaligned_volatile_store(dst: *mut T, val: T); +/// Performs a volatile load from the `dst` pointer. +/// This pointer is required to be aligned and supported for +/// lock-free atomic operations. +/// +/// It also creates a relaxed atomic ordering at this place. +#[rustc_intrinsic] +#[rustc_nounwind] +#[cfg(not(bootstrap))] +pub unsafe fn volatile_load_atomic_relaxed(src: *const T) -> T; + +/// Performs a volatile store to the `dst` pointer. +/// This pointer is required to be aligned, and the value supported +/// for lock-free atomic operations. +/// +/// This also creates a relaxed atomic ordering on at this place. +#[rustc_intrinsic] +#[rustc_nounwind] +#[cfg(not(bootstrap))] +pub unsafe fn volatile_store_atomic_relaxed(dst: *mut T, val: T); + /// Returns the square root of an `f16` /// /// The stabilized version of this intrinsic is diff --git a/library/core/src/ptr/volatile.rs b/library/core/src/ptr/volatile.rs index 9f9ef024cb1..44fe2c67e2d 100644 --- a/library/core/src/ptr/volatile.rs +++ b/library/core/src/ptr/volatile.rs @@ -1,7 +1,7 @@ use crate::mem::SizedTypeProperties; -use crate::intrinsics; -use crate::macros::cfg; -use crate::sync::atomic::{AtomicU8, AtomicU16, AtomicU32, Atomicu64}; +#[cfg(not(bootstrap))] +use crate::sync::atomic::{AtomicU8, AtomicU16, AtomicU32, AtomicU64}; +use crate::{cfg, intrinsics}; /// Performs a volatile read of the value from `src` without moving it. This /// leaves the memory in `src` unchanged. @@ -80,29 +80,34 @@ pub unsafe fn read_volatile(src: *const T) -> T { is_zst: bool = T::IS_ZST, ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) ); - match size_of() { - 1 => if cfg!(target_has_atomic_load_store = "8") && align_of::() == align_of::() { - intrinsics::atomic_load_relaxed(src) - } else { - intrinsics::volatile_load(dst, val) - } - 2 => if cfg!(target_has_atomic_load_store = "16") && align_of::() == align_of::() { - intrinsics::atomic_load_relaxed(src) - } else { - intrinsics::volatile_load(dst, val) - } - 4 => if cfg!(target_has_atomic_load_store = "32") && align_of::() == align_of::() { - intrinsics::atomic_load_relaxed(src) - } else { - intrinsics::volatile_load(dst, val) - } - 8 => if cfg!(target_has_atomic_load_store = "64") && align_of::() == align_of::() { - intrinsics::atomic_load_relaxed(src) - } else { - intrinsics::volatile_load(dst, val) - } - _ => intrinsics::volatile_load(dst, val) + + // TODO: Guard patterns + #[cfg(not(bootstrap))] + match size_of::() { + 1 if cfg!(target_has_atomic_load_store = "8") + && align_of::() == align_of::() => + { + intrinsics::volatile_load_atomic_relaxed(src) + } + 2 if cfg!(target_has_atomic_load_store = "16") + && align_of::() == align_of::() => + { + intrinsics::volatile_load_atomic_relaxed(src) + } + 4 if cfg!(target_has_atomic_load_store = "32") + && align_of::() == align_of::() => + { + intrinsics::volatile_load_atomic_relaxed(src) + } + 8 if cfg!(target_has_atomic_load_store = "64") + && align_of::() == align_of::() => + { + intrinsics::volatile_load_atomic_relaxed(src) + } + _ => intrinsics::volatile_load(src), } + #[cfg(bootstrap)] + intrinsics::volatile_load(src) } } @@ -182,28 +187,33 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { is_zst: bool = T::IS_ZST, ) => crate::ub_checks::maybe_is_aligned_and_not_null(addr, align, is_zst) ); - match size_of() { - 1 => if cfg!(target_has_atomic_load_store = "8") && align_of::() == align_of::() { - intrinsics::atomic_store_relaxed(dst, val) - } else { - intrinsics::volatile_store(dst, val) - } - 2 => if cfg!(target_has_atomic_load_store = "16") && align_of::() == align_of::() { - intrinsics::atomic_store_relaxed(dst, val) - } else { - intrinsics::volatile_store(dst, val) - } - 4 => if cfg!(target_has_atomic_load_store = "32") && align_of::() == align_of::() { - intrinsics::atomic_store_relaxed(dst, val) - } else { - intrinsics::volatile_store(dst, val) - } - 8 => if cfg!(target_has_atomic_load_store = "64") && align_of::() == align_of::() { - intrinsics::atomic_store_relaxed(dst, val) - } else { - intrinsics::volatile_store(dst, val) - } - _ => intrinsics::volatile_store(dst, val) + + // TODO: Guard patterns + #[cfg(not(bootstrap))] + match size_of::() { + 1 if cfg!(target_has_atomic_load_store = "8") + && align_of::() == align_of::() => + { + intrinsics::volatile_store_atomic_relaxed(dst, src) + } + 2 if cfg!(target_has_atomic_load_store = "16") + && align_of::() == align_of::() => + { + intrinsics::volatile_store_atomic_relaxed(dst, src) + } + 4 if cfg!(target_has_atomic_load_store = "32") + && align_of::() == align_of::() => + { + intrinsics::volatile_store_atomic_relaxed(dst, src) + } + 8 if cfg!(target_has_atomic_load_store = "64") + && align_of::() == align_of::() => + { + intrinsics::volatile_store_atomic_relaxed(dst, src) + } + _ => intrinsics::volatile_store(dst, src), } + #[cfg(bootstrap)] + intrinsics::volatile_store(dst, src) } }