Merge #54

54: Add fetch_neg/neg/fetch_not/not r=taiki-e a=taiki-e Part of #48 - Add `AtomicI*::{fetch_neg,neg}` and `AtomicF*::fetch_neg` methods. `AtomicI*::neg` are equivalent to the corresponding `fetch_*` methods, but do not return the previous value. They are intended for optimization on platforms that have atomic instructions for the corresponding operation, such as x86's `lock neg`. Currently, optimizations by these methods (`neg`) are only guaranteed for x86. - Add `Atomic{I,U}*::{fetch_not,not}` methods. `Atomic{I,U}*::not` are equivalent to the corresponding `fetch_*` methods, but do not return the previous value. They are intended for optimization on platforms that have atomic instructions for the corresponding operation, such as x86's `lock not`, MSP430's `inv`. Currently, optimizations by these methods (`not`) are only guaranteed for x86 and MSP430. (Note: `AtomicBool` already has `fetch_not` and `not` methods.) Co-authored-by: Taiki Endo <[email protected]>
taiki-e · Dec 18, 2022 · 16166af · 16166af
2 parents d69976d + 0c4a2d9
commit 16166af
Show file tree

Hide file tree

Showing 18 changed files with 937 additions and 65 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -261,6 +261,7 @@ jobs:
         target:
           - '' # x86_64-unknown-linux-gnu
           - aarch64-unknown-linux-gnu
+          - i686-unknown-linux-gnu
           - powerpc64le-unknown-linux-gnu
           - s390x-unknown-linux-gnu
     runs-on: ubuntu-latest

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,20 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Add `AtomicI*::{fetch_neg,neg}` and `AtomicF*::fetch_neg` methods.
+
+  `AtomicI*::neg` are equivalent to the corresponding `fetch_*` methods, but do not return the previous value. They are intended for optimization on platforms that have atomic instructions for the corresponding operation, such as x86's `lock neg`.
+
+  Currently, optimizations by these methods (`neg`) are only guaranteed for x86.
+
+- Add `Atomic{I,U}*::{fetch_not,not}` methods.
+
+  `Atomic{I,U}*::not` are equivalent to the corresponding `fetch_*` methods, but do not return the previous value. They are intended for optimization on platforms that have atomic instructions for the corresponding operation, such as x86's `lock not`, MSP430's `inv`.
+
+  Currently, optimizations by these methods (`not`) are only guaranteed for x86 and MSP430.
+
+  (Note: `AtomicBool` already has `fetch_not` and `not` methods.)
+
 ## [0.3.18] - 2022-12-15
 
 - Fix build error when not using `portable_atomic_unsafe_assume_single_core` cfg on AVR and MSP430 custom targets. ([#50](https://github.com/taiki-e/portable-atomic/pull/50))
@@ -28,6 +42,8 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
   They are equivalent to the corresponding `fetch_*` methods, but do not return the previous value. They are intended for optimization on platforms that implement atomics using inline assembly, such as the MSP430.
 
+  Currently, optimizations by these methods (`add`,`sub`,`and`,`or`,`xor`) are only guaranteed for MSP430; on x86, LLVM can optimize in most cases, so cases, where this would improve things, should be rare.
+
 - Various improvements to `portable_atomic_unsafe_assume_single_core` cfg. ([#44](https://github.com/taiki-e/portable-atomic/pull/44), [#40](https://github.com/taiki-e/portable-atomic/pull/40))
 
   - Support disabling FIQs on pre-v6 ARM under `portable_atomic_disable_fiq` cfg.

diff --git a/src/imp/atomic128/aarch64.rs b/src/imp/atomic128/aarch64.rs
@@ -28,6 +28,9 @@
 // - https://lists.llvm.org/pipermail/llvm-dev/2016-May/099490.html
 // - https://lists.llvm.org/pipermail/llvm-dev/2018-June/123993.html
 //
+// Note: On Miri and ThreadSanitizer which do not support inline assembly, we don't use
+// this module and use intrinsics.rs instead.
+//
 // Refs:
 // - ARM Compiler armasm User Guide
 //   https://developer.arm.com/documentation/dui0801/latest
@@ -957,8 +960,8 @@ unsafe fn atomic_umin(dst: *mut u128, val: u128, order: Ordering) -> u128 {
     }
 }
 
-atomic128!(AtomicI128, i128, atomic_max, atomic_min);
-atomic128!(AtomicU128, u128, atomic_umax, atomic_umin);
+atomic128!(int, AtomicI128, i128, atomic_max, atomic_min);
+atomic128!(uint, AtomicU128, u128, atomic_umax, atomic_umin);
 
 #[cfg(test)]
 mod tests {
@@ -994,8 +997,8 @@ mod tests_no_outline_atomics {
     // so we always use strong CAS.
     use self::atomic_compare_exchange as atomic_compare_exchange_weak;
 
-    atomic128!(AtomicI128, i128, atomic_max, atomic_min);
-    atomic128!(AtomicU128, u128, atomic_umax, atomic_umin);
+    atomic128!(int, AtomicI128, i128, atomic_max, atomic_min);
+    atomic128!(uint, AtomicU128, u128, atomic_umax, atomic_umin);
 
     mod tests {
         use super::*;

diff --git a/src/imp/atomic128/intrinsics.rs b/src/imp/atomic128/intrinsics.rs
@@ -405,7 +405,7 @@ unsafe fn atomic_umin(dst: *mut u128, val: u128, order: Ordering) -> u128 {
 }
 
 macro_rules! atomic128 {
-    ($atomic_type:ident, $int_type:ident, $atomic_max:ident, $atomic_min:ident) => {
+    (uint, $atomic_type:ident, $int_type:ident, $atomic_max:ident, $atomic_min:ident) => {
         #[repr(C, align(16))]
         pub(crate) struct $atomic_type {
             v: UnsafeCell<$int_type>,
@@ -582,12 +582,50 @@ macro_rules! atomic128 {
                 // pointer passed in is valid because we got it from a reference.
                 unsafe { $atomic_min(self.v.get(), val, order) }
             }
+
+            #[inline]
+            pub(crate) fn fetch_not(&self, order: Ordering) -> $int_type {
+                self.fetch_update_(order, |x| !x)
+            }
+            #[inline]
+            pub(crate) fn not(&self, order: Ordering) {
+                self.fetch_not(order);
+            }
+
+            #[inline]
+            fn fetch_update_<F>(&self, set_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);
+                loop {
+                    let next = f(prev);
+                    match self.compare_exchange_weak(prev, next, set_order, fetch_order) {
+                        Ok(x) => return x,
+                        Err(next_prev) => prev = next_prev,
+                    }
+                }
+            }
+        }
+    };
+    (int, $atomic_type:ident, $int_type:ident, $atomic_max:ident, $atomic_min:ident) => {
+        atomic128!(uint, $atomic_type, $int_type, $atomic_max, $atomic_min);
+        impl $atomic_type {
+            #[inline]
+            pub(crate) fn fetch_neg(&self, order: Ordering) -> $int_type {
+                self.fetch_update_(order, |x| x.wrapping_neg())
+            }
+            #[inline]
+            pub(crate) fn neg(&self, order: Ordering) {
+                self.fetch_neg(order);
+            }
         }
     };
 }
 
-atomic128!(AtomicI128, i128, atomic_max, atomic_min);
-atomic128!(AtomicU128, u128, atomic_umax, atomic_umin);
+atomic128!(int, AtomicI128, i128, atomic_max, atomic_min);
+atomic128!(uint, AtomicU128, u128, atomic_umax, atomic_umin);
 
 #[cfg(test)]
 mod tests {

diff --git a/src/imp/atomic128/macros.rs b/src/imp/atomic128/macros.rs
@@ -1,6 +1,6 @@
 #[cfg(any(target_arch = "s390x", target_arch = "x86_64"))]
 macro_rules! atomic128 {
-    ($atomic_type:ident, $int_type:ident) => {
+    (uint, $atomic_type:ident, $int_type:ident) => {
         #[repr(C, align(16))]
         pub(crate) struct $atomic_type {
             v: core::cell::UnsafeCell<$int_type>,
@@ -201,13 +201,48 @@ macro_rules! atomic128 {
                     }) as $int_type
                 }
             }
+
+            #[inline]
+            pub(crate) fn fetch_not(&self, order: Ordering) -> $int_type {
+                crate::utils::assert_swap_ordering(order);
+                // SAFETY: any data races are prevented by atomic intrinsics and the raw
+                // pointer passed in is valid because we got it from a reference.
+                unsafe {
+                    atomic_update(self.v.get().cast(), order, |x| !(x as $int_type) as u128)
+                        as $int_type
+                }
+            }
+            #[inline]
+            pub(crate) fn not(&self, order: Ordering) {
+                self.fetch_not(order);
+            }
+        }
+    };
+    (int, $atomic_type:ident, $int_type:ident) => {
+        atomic128!(uint, $atomic_type, $int_type);
+        impl $atomic_type {
+            #[inline]
+            pub(crate) fn fetch_neg(&self, order: Ordering) -> $int_type {
+                crate::utils::assert_swap_ordering(order);
+                // SAFETY: any data races are prevented by atomic intrinsics and the raw
+                // pointer passed in is valid because we got it from a reference.
+                unsafe {
+                    atomic_update(self.v.get().cast(), order, |x| {
+                        (x as $int_type).wrapping_neg() as u128
+                    }) as $int_type
+                }
+            }
+            #[inline]
+            pub(crate) fn neg(&self, order: Ordering) {
+                self.fetch_neg(order);
+            }
         }
     };
 }
 
 #[cfg(any(target_arch = "aarch64", target_arch = "powerpc64"))]
 macro_rules! atomic128 {
-    ($atomic_type:ident, $int_type:ident, $atomic_max:ident, $atomic_min:ident) => {
+    (uint, $atomic_type:ident, $int_type:ident, $atomic_max:ident, $atomic_min:ident) => {
         #[repr(C, align(16))]
         pub(crate) struct $atomic_type {
             v: core::cell::UnsafeCell<$int_type>,
@@ -386,6 +421,46 @@ macro_rules! atomic128 {
                 // pointer passed in is valid because we got it from a reference.
                 unsafe { $atomic_min(self.v.get(), val, order) }
             }
+
+            #[inline]
+            pub(crate) fn fetch_not(&self, order: Ordering) -> $int_type {
+                // TODO: define atomic_not function and use it
+                self.fetch_update_(order, |x| !x)
+            }
+            #[inline]
+            pub(crate) fn not(&self, order: Ordering) {
+                self.fetch_not(order);
+            }
+
+            #[inline]
+            fn fetch_update_<F>(&self, set_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);
+                loop {
+                    let next = f(prev);
+                    match self.compare_exchange_weak(prev, next, set_order, fetch_order) {
+                        Ok(x) => return x,
+                        Err(next_prev) => prev = next_prev,
+                    }
+                }
+            }
+        }
+    };
+    (int, $atomic_type:ident, $int_type:ident, $atomic_max:ident, $atomic_min:ident) => {
+        atomic128!(uint, $atomic_type, $int_type, $atomic_max, $atomic_min);
+        impl $atomic_type {
+            #[inline]
+            pub(crate) fn fetch_neg(&self, order: Ordering) -> $int_type {
+                // TODO: define atomic_neg function and use it
+                self.fetch_update_(order, |x| x.wrapping_neg())
+            }
+            #[inline]
+            pub(crate) fn neg(&self, order: Ordering) {
+                self.fetch_neg(order);
+            }
         }
     };
 }
diff --git a/src/imp/atomic128/powerpc64.rs b/src/imp/atomic128/powerpc64.rs
@@ -13,6 +13,9 @@
 // for the compiler to insert operations that might clear the reservation between
 // LL and SC. See aarch64.rs for details.
 //
+// Note: On Miri and ThreadSanitizer which do not support inline assembly, we don't use
+// this module and use intrinsics.rs instead.
+//
 // Refs:
 // - Power ISA https://openpowerfoundation.org/specifications/isa
 // - AIX Assembler language reference https://www.ibm.com/docs/en/aix/7.3?topic=aix-assembler-language-reference
@@ -686,8 +689,8 @@ unsafe fn atomic_umin(dst: *mut u128, val: u128, order: Ordering) -> u128 {
     }
 }
 
-atomic128!(AtomicI128, i128, atomic_max, atomic_min);
-atomic128!(AtomicU128, u128, atomic_umax, atomic_umin);
+atomic128!(int, AtomicI128, i128, atomic_max, atomic_min);
+atomic128!(uint, AtomicU128, u128, atomic_umax, atomic_umin);
 
 #[cfg(test)]
 mod tests {

diff --git a/src/imp/atomic128/s390x.rs b/src/imp/atomic128/s390x.rs
@@ -223,8 +223,8 @@ const fn is_always_lock_free() -> bool {
 }
 use is_always_lock_free as is_lock_free;
 
-atomic128!(AtomicI128, i128);
-atomic128!(AtomicU128, u128);
+atomic128!(int, AtomicI128, i128);
+atomic128!(uint, AtomicU128, u128);
 
 #[cfg(test)]
 mod tests {

diff --git a/src/imp/atomic128/x86_64.rs b/src/imp/atomic128/x86_64.rs
@@ -433,8 +433,8 @@ const fn is_always_lock_free() -> bool {
     cfg!(any(target_feature = "cmpxchg16b", portable_atomic_target_feature = "cmpxchg16b",))
 }
 
-atomic128!(AtomicI128, i128);
-atomic128!(AtomicU128, u128);
+atomic128!(int, AtomicI128, i128);
+atomic128!(uint, AtomicU128, u128);
 
 #[allow(clippy::undocumented_unsafe_blocks, clippy::wildcard_imports)]
 #[cfg(test)]
@@ -598,8 +598,8 @@ mod tests_no_cmpxchg16b {
     }
     use is_always_lock_free as is_lock_free;
 
-    atomic128!(AtomicI128, i128);
-    atomic128!(AtomicU128, u128);
+    atomic128!(int, AtomicI128, i128);
+    atomic128!(uint, AtomicU128, u128);
 
     test_atomic_int!(i128);
     test_atomic_int!(u128);