From fa65c6c5fc7e02ed06aeef960d09269faa7609b3 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 5 Mar 2023 17:01:06 +0900 Subject: [PATCH] Relax failure ordering in private CAS loop helpers They are private functions and all closure instances only operate on the value loaded, so there is no need to synchronize the first load/failed CAS. --- src/imp/atomic128/intrinsics.rs | 7 ++++--- src/imp/atomic128/s390x.rs | 7 ++++--- src/imp/atomic128/x86_64.rs | 8 ++++---- src/imp/core_atomic.rs | 9 +++++---- src/imp/float.rs | 9 +++++---- src/lib.rs | 9 +++++---- src/utils.rs | 1 + 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/imp/atomic128/intrinsics.rs b/src/imp/atomic128/intrinsics.rs index c32fadaa..f02700d1 100644 --- a/src/imp/atomic128/intrinsics.rs +++ b/src/imp/atomic128/intrinsics.rs @@ -284,13 +284,14 @@ unsafe fn atomic_update(dst: *mut u128, order: Ordering, mut f: F) -> u128 where F: FnMut(u128) -> u128, { - let failure = crate::utils::strongest_failure_ordering(order); // SAFETY: the caller must uphold the safety contract for `atomic_update`. unsafe { - let mut old = atomic_load(dst, failure); + // This is a private function and all instances of `f` only operate on the value + // loaded, so there is no need to synchronize the first load/failed CAS. + let mut old = atomic_load(dst, Ordering::Relaxed); loop { let next = f(old); - match atomic_compare_exchange_weak(dst, old, next, order, failure) { + match atomic_compare_exchange_weak(dst, old, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(x) => old = x, } diff --git a/src/imp/atomic128/s390x.rs b/src/imp/atomic128/s390x.rs index 58e49f40..bc7b0c30 100644 --- a/src/imp/atomic128/s390x.rs +++ b/src/imp/atomic128/s390x.rs @@ -196,13 +196,14 @@ unsafe fn atomic_update(dst: *mut u128, order: Ordering, mut f: F) -> u128 where F: FnMut(u128) -> u128, { - let failure = crate::utils::strongest_failure_ordering(order); // SAFETY: the caller must uphold the safety contract for `atomic_update`. unsafe { - let mut old = atomic_load(dst, failure); + // This is a private function and all instances of `f` only operate on the value + // loaded, so there is no need to synchronize the first load/failed CAS. + let mut old = atomic_load(dst, Ordering::Relaxed); loop { let next = f(old); - match atomic_compare_exchange_weak(dst, old, next, order, failure) { + match atomic_compare_exchange_weak(dst, old, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(x) => old = x, } diff --git a/src/imp/atomic128/x86_64.rs b/src/imp/atomic128/x86_64.rs index 270b4b86..4cdc19e3 100644 --- a/src/imp/atomic128/x86_64.rs +++ b/src/imp/atomic128/x86_64.rs @@ -367,7 +367,6 @@ unsafe fn atomic_update(dst: *mut u128, order: Ordering, mut f: F) -> u128 where F: FnMut(u128) -> u128, { - let failure = crate::utils::strongest_failure_ordering(order); // SAFETY: the caller must uphold the safety contract for `atomic_update`. unsafe { // This is based on the code generated for the first load in DW RMWs by LLVM, @@ -387,7 +386,9 @@ where let mut old = byte_wise_atomic_load(dst); loop { let next = f(old); - match atomic_compare_exchange_weak(dst, old, next, order, failure) { + // This is a private function and all instances of `f` only operate on the value + // loaded, so there is no need to synchronize the first load/failed CAS. + match atomic_compare_exchange_weak(dst, old, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(x) => old = x, } @@ -561,12 +562,11 @@ mod tests_no_cmpxchg16b { where F: FnMut(u128) -> u128, { - let failure = crate::utils::strongest_failure_ordering(order); unsafe { let mut old = byte_wise_atomic_load(dst); loop { let next = f(old); - match atomic_compare_exchange_weak(dst, old, next, order, failure) { + match atomic_compare_exchange_weak(dst, old, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(x) => old = x, } diff --git a/src/imp/core_atomic.rs b/src/imp/core_atomic.rs index 4190d203..d7d8ab48 100644 --- a/src/imp/core_atomic.rs +++ b/src/imp/core_atomic.rs @@ -291,15 +291,16 @@ macro_rules! atomic_int { } #[allow(dead_code)] #[inline] - fn fetch_update_(&self, set_order: Ordering, mut f: F) -> $int_type + fn fetch_update_(&self, order: Ordering, mut f: F) -> $int_type where F: FnMut($int_type) -> $int_type, { - let fetch_order = crate::utils::strongest_failure_ordering(set_order); - let mut prev = self.load(fetch_order); + // This is a private function and all instances of `f` only operate on the value + // loaded, so there is no need to synchronize the first load/failed CAS. + let mut prev = self.load(Ordering::Relaxed); loop { let next = f(prev); - match self.compare_exchange_weak(prev, next, set_order, fetch_order) { + match self.compare_exchange_weak(prev, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(next_prev) => prev = next_prev, } diff --git a/src/imp/float.rs b/src/imp/float.rs index 94eb0be5..9ac011ff 100644 --- a/src/imp/float.rs +++ b/src/imp/float.rs @@ -153,15 +153,16 @@ macro_rules! atomic_float { } #[inline] - fn fetch_update_(&self, set_order: Ordering, mut f: F) -> $float_type + fn fetch_update_(&self, order: Ordering, mut f: F) -> $float_type where F: FnMut($float_type) -> $float_type, { - let fetch_order = crate::utils::strongest_failure_ordering(set_order); - let mut prev = self.load(fetch_order); + // This is a private function and all instances of `f` only operate on the value + // loaded, so there is no need to synchronize the first load/failed CAS. + let mut prev = self.load(Ordering::Relaxed); loop { let next = f(prev); - match self.compare_exchange_weak(prev, next, set_order, fetch_order) { + match self.compare_exchange_weak(prev, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(next_prev) => prev = next_prev, } diff --git a/src/lib.rs b/src/lib.rs index 3607e7d3..5d713b18 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1917,15 +1917,16 @@ impl AtomicPtr { #[cfg(miri)] #[inline] - fn fetch_update_(&self, set_order: Ordering, mut f: F) -> *mut T + fn fetch_update_(&self, order: Ordering, mut f: F) -> *mut T where F: FnMut(*mut T) -> *mut T, { - let fetch_order = crate::utils::strongest_failure_ordering(set_order); - let mut prev = self.load(fetch_order); + // This is a private function and all instances of `f` only operate on the value + // loaded, so there is no need to synchronize the first load/failed CAS. + let mut prev = self.load(Ordering::Relaxed); loop { let next = f(prev); - match self.compare_exchange_weak(prev, next, set_order, fetch_order) { + match self.compare_exchange_weak(prev, next, order, Ordering::Relaxed) { Ok(x) => return x, Err(next_prev) => prev = next_prev, } diff --git a/src/utils.rs b/src/utils.rs index 366ade54..36bd37fe 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -269,6 +269,7 @@ pub(crate) struct NoRefUnwindSafe(UnsafeCell<()>); unsafe impl Sync for NoRefUnwindSafe {} // https://github.com/rust-lang/rust/blob/1.67.0/library/core/src/sync/atomic.rs#L2956 +#[allow(dead_code)] #[inline] pub(crate) fn strongest_failure_ordering(order: Ordering) -> Ordering { match order {