From 76b28b775559649effca03c9ff0c2f4f77e6da15 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Mon, 9 Dec 2024 00:51:29 +0900 Subject: [PATCH] AtomicCell: Use atomic-maybe-uninit --- .github/workflows/ci.yml | 4 + crossbeam-utils/Cargo.toml | 3 +- crossbeam-utils/build.rs | 10 +- crossbeam-utils/src/atomic/atomic_cell.rs | 194 ++++++++++++++++------ crossbeam-utils/tests/atomic_cell.rs | 55 ++++-- 5 files changed, 191 insertions(+), 75 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25a1a2f56..d63274e72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,6 +62,10 @@ jobs: - rust: nightly os: ubuntu-latest target: armv5te-unknown-linux-gnueabi + # Test target without stable inline asm support. + - rust: stable + os: ubuntu-latest + target: sparc64-unknown-linux-gnu runs-on: ${{ matrix.os }} timeout-minutes: 60 steps: diff --git a/crossbeam-utils/Cargo.toml b/crossbeam-utils/Cargo.toml index ead50eeae..eeec4e823 100644 --- a/crossbeam-utils/Cargo.toml +++ b/crossbeam-utils/Cargo.toml @@ -25,9 +25,10 @@ default = ["std"] std = [] # Enable `atomic` module. -atomic = [] +atomic = ["atomic-maybe-uninit"] [dependencies] +atomic-maybe-uninit = { version = "0.3.4", optional = true } # Enable the use of loom for concurrency testing. # diff --git a/crossbeam-utils/build.rs b/crossbeam-utils/build.rs index 346c9b428..dcf39db26 100644 --- a/crossbeam-utils/build.rs +++ b/crossbeam-utils/build.rs @@ -17,7 +17,7 @@ include!("build-common.rs"); fn main() { println!("cargo:rerun-if-changed=no_atomic.rs"); - println!("cargo:rustc-check-cfg=cfg(crossbeam_no_atomic,crossbeam_sanitize_thread)"); + println!("cargo:rustc-check-cfg=cfg(crossbeam_no_atomic,crossbeam_sanitize_thread,crossbeam_atomic_cell_force_fallback)"); let target = match env::var("TARGET") { Ok(target) => convert_custom_linux_target(target), @@ -39,8 +39,10 @@ fn main() { } // `cfg(sanitize = "..")` is not stabilized. - let sanitize = env::var("CARGO_CFG_SANITIZE").unwrap_or_default(); - if sanitize.contains("thread") { - println!("cargo:rustc-cfg=crossbeam_sanitize_thread"); + if let Ok(sanitize) = env::var("CARGO_CFG_SANITIZE") { + if sanitize.contains("thread") { + println!("cargo:rustc-cfg=crossbeam_sanitize_thread"); + } + println!("cargo:rustc-cfg=crossbeam_atomic_cell_force_fallback"); } } diff --git a/crossbeam-utils/src/atomic/atomic_cell.rs b/crossbeam-utils/src/atomic/atomic_cell.rs index 657084a1f..98fb8d495 100644 --- a/crossbeam-utils/src/atomic/atomic_cell.rs +++ b/crossbeam-utils/src/atomic/atomic_cell.rs @@ -99,6 +99,8 @@ impl AtomicCell { /// # Examples /// /// ``` + /// # // Always use fallback for now on environments that do not support inline assembly. + /// # if cfg!(any(miri, crossbeam_loom, crossbeam_atomic_cell_force_fallback)) { return; } /// use crossbeam_utils::atomic::AtomicCell; /// /// // This type is internally represented as `AtomicUsize` so we can just use atomic @@ -307,13 +309,29 @@ macro_rules! atomic { loop { atomic!(@check, $t, AtomicUnit, $a, $atomic_op); - atomic!(@check, $t, atomic::AtomicU8, $a, $atomic_op); - atomic!(@check, $t, atomic::AtomicU16, $a, $atomic_op); - atomic!(@check, $t, atomic::AtomicU32, $a, $atomic_op); - #[cfg(target_has_atomic = "64")] - atomic!(@check, $t, atomic::AtomicU64, $a, $atomic_op); - // TODO: AtomicU128 is unstable - // atomic!(@check, $t, atomic::AtomicU128, $a, $atomic_op); + // Always use fallback for now on environments that do not support inline assembly. + #[cfg(not(any( + miri, + crossbeam_loom, + crossbeam_atomic_cell_force_fallback, + )))] + atomic_maybe_uninit::cfg_has_atomic_cas! { + atomic_maybe_uninit::cfg_has_atomic_8! { + atomic!(@check, $t, atomic_maybe_uninit::AtomicMaybeUninit, $a, $atomic_op); + } + atomic_maybe_uninit::cfg_has_atomic_16! { + atomic!(@check, $t, atomic_maybe_uninit::AtomicMaybeUninit, $a, $atomic_op); + } + atomic_maybe_uninit::cfg_has_atomic_32! { + atomic!(@check, $t, atomic_maybe_uninit::AtomicMaybeUninit, $a, $atomic_op); + } + atomic_maybe_uninit::cfg_has_atomic_64! { + atomic!(@check, $t, atomic_maybe_uninit::AtomicMaybeUninit, $a, $atomic_op); + } + atomic_maybe_uninit::cfg_has_atomic_128! { + atomic!(@check, $t, atomic_maybe_uninit::AtomicMaybeUninit, $a, $atomic_op); + } + } break $fallback_op; } @@ -321,7 +339,7 @@ macro_rules! atomic { } macro_rules! impl_arithmetic { - ($t:ty, fallback, $example:tt) => { + ($t:ty, fetch_update, $example:tt) => { impl AtomicCell<$t> { /// Increments the current value by `val` and returns the previous value. /// @@ -339,11 +357,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_add(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value = value.wrapping_add(val); - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(old.wrapping_add(val))).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value = value.wrapping_add(val); + old + } + } } /// Decrements the current value by `val` and returns the previous value. @@ -362,11 +388,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_sub(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value = value.wrapping_sub(val); - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(old.wrapping_sub(val))).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value = value.wrapping_sub(val); + old + } + } } /// Applies bitwise "and" to the current value and returns the previous value. @@ -383,11 +417,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_and(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value &= val; - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(old & val)).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value &= val; + old + } + } } /// Applies bitwise "nand" to the current value and returns the previous value. @@ -404,11 +446,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_nand(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value = !(old & val); - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(!(old & val))).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value = !(old & val); + old + } + } } /// Applies bitwise "or" to the current value and returns the previous value. @@ -425,11 +475,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_or(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value |= val; - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(old | val)).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value |= val; + old + } + } } /// Applies bitwise "xor" to the current value and returns the previous value. @@ -446,11 +504,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_xor(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value ^= val; - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(old ^ val)).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value ^= val; + old + } + } } /// Compares and sets the maximum of the current value and `val`, @@ -468,11 +534,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_max(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value = cmp::max(old, val); - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(cmp::max(old, val))).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value = cmp::max(old, val); + old + } + } } /// Compares and sets the minimum of the current value and `val`, @@ -490,11 +564,19 @@ macro_rules! impl_arithmetic { /// ``` #[inline] pub fn fetch_min(&self, val: $t) -> $t { - let _guard = lock(self.as_ptr() as usize).write(); - let value = unsafe { &mut *(self.as_ptr()) }; - let old = *value; - *value = cmp::min(old, val); - old + atomic! { + $t, _a, + { + self.fetch_update(|old| Some(cmp::min(old, val))).unwrap() + }, + { + let _guard = lock(self.as_ptr() as usize).write(); + let value = unsafe { &mut *(self.as_ptr()) }; + let old = *value; + *value = cmp::min(old, val); + old + } + } } } }; @@ -754,23 +836,29 @@ impl_arithmetic!(i8, AtomicI8, "let a = AtomicCell::new(7i8);"); impl_arithmetic!(u16, AtomicU16, "let a = AtomicCell::new(7u16);"); impl_arithmetic!(i16, AtomicI16, "let a = AtomicCell::new(7i16);"); +#[cfg(target_has_atomic = "32")] impl_arithmetic!(u32, AtomicU32, "let a = AtomicCell::new(7u32);"); +#[cfg(target_has_atomic = "32")] impl_arithmetic!(i32, AtomicI32, "let a = AtomicCell::new(7i32);"); +#[cfg(not(target_has_atomic = "32"))] +impl_arithmetic!(u32, fetch_update, "let a = AtomicCell::new(7u32);"); +#[cfg(not(target_has_atomic = "32"))] +impl_arithmetic!(i32, fetch_update, "let a = AtomicCell::new(7i32);"); #[cfg(target_has_atomic = "64")] impl_arithmetic!(u64, AtomicU64, "let a = AtomicCell::new(7u64);"); #[cfg(target_has_atomic = "64")] impl_arithmetic!(i64, AtomicI64, "let a = AtomicCell::new(7i64);"); #[cfg(not(target_has_atomic = "64"))] -impl_arithmetic!(u64, fallback, "let a = AtomicCell::new(7u64);"); +impl_arithmetic!(u64, fetch_update, "let a = AtomicCell::new(7u64);"); #[cfg(not(target_has_atomic = "64"))] -impl_arithmetic!(i64, fallback, "let a = AtomicCell::new(7i64);"); +impl_arithmetic!(i64, fetch_update, "let a = AtomicCell::new(7i64);"); -// TODO: AtomicU128 is unstable +// TODO: core::sync::atomic::AtomicU128 is unstable // impl_arithmetic!(u128, AtomicU128, "let a = AtomicCell::new(7u128);"); // impl_arithmetic!(i128, AtomicI128, "let a = AtomicCell::new(7i128);"); -impl_arithmetic!(u128, fallback, "let a = AtomicCell::new(7u128);"); -impl_arithmetic!(i128, fallback, "let a = AtomicCell::new(7i128);"); +impl_arithmetic!(u128, fetch_update, "let a = AtomicCell::new(7u128);"); +impl_arithmetic!(i128, fetch_update, "let a = AtomicCell::new(7i128);"); impl_arithmetic!(usize, AtomicUsize, "let a = AtomicCell::new(7usize);"); impl_arithmetic!(isize, AtomicIsize, "let a = AtomicCell::new(7isize);"); diff --git a/crossbeam-utils/tests/atomic_cell.rs b/crossbeam-utils/tests/atomic_cell.rs index 83b1015d8..4670f7346 100644 --- a/crossbeam-utils/tests/atomic_cell.rs +++ b/crossbeam-utils/tests/atomic_cell.rs @@ -4,48 +4,69 @@ use std::sync::atomic::Ordering::SeqCst; use crossbeam_utils::atomic::AtomicCell; +// Always use fallback for now on environments that do not support inline assembly. +fn always_use_fallback() -> bool { + atomic_maybe_uninit::cfg_has_atomic_cas! { + cfg!(any( + miri, + crossbeam_loom, + crossbeam_atomic_cell_force_fallback, + )) + } + atomic_maybe_uninit::cfg_no_atomic_cas! { false } +} + #[test] fn is_lock_free() { + let always_use_fallback = always_use_fallback(); + struct UsizeWrap(#[allow(dead_code)] usize); struct U8Wrap(#[allow(dead_code)] bool); struct I16Wrap(#[allow(dead_code)] i16); #[repr(align(8))] struct U64Align8(#[allow(dead_code)] u64); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!( + AtomicCell::::is_lock_free(), + !always_use_fallback + ); assert!(AtomicCell::<()>::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); - assert!(AtomicCell::::is_lock_free()); - assert!(AtomicCell::::is_lock_free()); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); + assert_eq!(AtomicCell::::is_lock_free(), !always_use_fallback); // Sizes of both types must be equal, and the alignment of `u64` must be greater or equal than // that of `AtomicU64`. In i686-unknown-linux-gnu, the alignment of `u64` is `4` and alignment // of `AtomicU64` is `8`, so `AtomicCell` is not lock-free. assert_eq!( AtomicCell::::is_lock_free(), - cfg!(target_has_atomic = "64") && std::mem::align_of::() == 8 + cfg!(target_has_atomic = "64") && std::mem::align_of::() == 8 && !always_use_fallback ); assert_eq!(mem::size_of::(), 8); assert_eq!(mem::align_of::(), 8); assert_eq!( AtomicCell::::is_lock_free(), - cfg!(target_has_atomic = "64") + cfg!(target_has_atomic = "64") && !always_use_fallback ); - // AtomicU128 is unstable - assert!(!AtomicCell::::is_lock_free()); + assert_eq!( + AtomicCell::::is_lock_free(), + cfg!(target_has_atomic = "128") + && std::mem::align_of::() == 16 + && !always_use_fallback + ); } #[test] @@ -320,7 +341,7 @@ fn issue_748() { assert_eq!(mem::size_of::(), 8); assert_eq!( AtomicCell::::is_lock_free(), - cfg!(target_has_atomic = "64") + cfg!(target_has_atomic = "64") && !always_use_fallback() ); let x = AtomicCell::new(Test::FieldLess); assert_eq!(x.load(), Test::FieldLess);