Optimize pre-v6 ARM load/store on single-core systems

CHANGELOG.md

@@ -10,6 +10,8 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Optimize pre-v6 ARM load/store when `portable_atomic_unsafe_assume_single_core` cfg is used.
+
 ## [0.3.13] - 2022-08-15
 
 - Use track_caller when debug assertions are enabled on Rust 1.46+.

src/imp/interrupt/armv4t.rs

@@ -38,3 +38,151 @@ pub(super) unsafe fn restore(State(prev): State) {
         asm!("msr cpsr_c, {0}", in(reg) prev, options(nostack));
     }
 }
+
+// On pre-v6 ARM, we cannot use core::sync::atomic here because they call the
+// `__sync_*` builtins for non-relaxed load/store (because pre-v6 ARM doesn't
+// have Data Memory Barrier).
+pub(crate) mod atomic {
+    #[cfg(not(portable_atomic_no_asm))]
+    use core::arch::asm;
+    use core::{cell::UnsafeCell, sync::atomic::Ordering};
+
+    #[repr(transparent)]
+    pub(crate) struct AtomicBool {
+        v: UnsafeCell<u8>,
+    }
+
+    impl AtomicBool {
+        #[inline]
+        pub(crate) fn load(&self, order: Ordering) -> bool {
+            // 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 { u8::atomic_load(self.v.get(), order) != 0 }
+        }
+
+        #[inline]
+        pub(crate) fn store(&self, val: bool, order: Ordering) {
+            // 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 {
+                u8::atomic_store(self.v.get(), val as u8, order);
+            }
+        }
+    }
+
+    #[repr(transparent)]
+    pub(crate) struct AtomicPtr<T> {
+        p: UnsafeCell<*mut T>,
+    }
+
+    impl<T> AtomicPtr<T> {
+        #[inline]
+        pub(crate) fn load(&self, order: Ordering) -> *mut T {
+            // SAFETY: any data races are prevented by atomic intrinsics and the raw
+            // pointer passed in is valid because we got it from a reference.
+            // TODO: remove int to ptr cast
+            unsafe { usize::atomic_load(self.p.get() as *mut usize, order) as *mut T }
+        }
+
+        #[inline]
+        pub(crate) fn store(&self, ptr: *mut T, order: Ordering) {
+            // SAFETY: any data races are prevented by atomic intrinsics and the raw
+            // pointer passed in is valid because we got it from a reference.
+            // TODO: remove int to ptr cast
+            unsafe {
+                usize::atomic_store(self.p.get() as *mut usize, ptr as usize, order);
+            }
+        }
+    }
+
+    macro_rules! atomic_int {
+        ($int_type:ident, $atomic_type:ident, $asm_suffix:expr) => {
+            #[repr(transparent)]
+            pub(crate) struct $atomic_type {
+                v: UnsafeCell<$int_type>,
+            }
+
+            impl $atomic_type {
+                #[inline]
+                pub(crate) fn load(&self, order: Ordering) -> $int_type {
+                    // 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 { $int_type::atomic_load(self.v.get(), order) }
+                }
+
+                #[inline]
+                pub(crate) fn store(&self, val: $int_type, order: Ordering) {
+                    // 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 {
+                        $int_type::atomic_store(self.v.get(), val, order);
+                    }
+                }
+            }
+
+            impl AtomicOperations for $int_type {
+                #[inline]
+                unsafe fn atomic_load(src: *const Self, order: Ordering) -> Self {
+                    // SAFETY: the caller must uphold the safety contract for `atomic_load`.
+                    unsafe {
+                        let out;
+                        match order {
+                            Ordering::Relaxed => {
+                                asm!(
+                                    concat!("ldr", $asm_suffix, " {out}, [{src}]"),
+                                    src = in(reg) src,
+                                    out = lateout(reg) out,
+                                    options(nostack, preserves_flags, readonly),
+                                );
+                            }
+                            Ordering::Acquire | Ordering::SeqCst => {
+                                // inline asm without nomem/readonly implies compiler fence.
+                                // And compiler fence is fine because the user explicitly declares that
+                                // the system is single-core by using an unsafe cfg.
+                                asm!(
+                                    concat!("ldr", $asm_suffix, " {out}, [{src}]"),
+                                    src = in(reg) src,
+                                    out = lateout(reg) out,
+                                    options(nostack, preserves_flags),
+                                );
+                            }
+                            _ => unreachable!("{:?}", order),
+                        }
+                        out
+                    }
+                }
+
+                #[inline]
+                unsafe fn atomic_store(dst: *mut Self, val: Self, _order: Ordering) {
+                    // SAFETY: the caller must uphold the safety contract for `atomic_store`.
+                    unsafe {
+                        // inline asm without nomem/readonly implies compiler fence.
+                        // And compiler fence is fine because the user explicitly declares that
+                        // the system is single-core by using an unsafe cfg.
+                        asm!(
+                            concat!("str", $asm_suffix, " {val}, [{dst}]"),
+                            dst = in(reg) dst,
+                            val = in(reg) val,
+                            options(nostack, preserves_flags),
+                        );
+                    }
+                }
+            }
+
+        };
+    }
+
+    atomic_int!(i8, AtomicI8, "b");
+    atomic_int!(u8, AtomicU8, "b");
+    atomic_int!(i16, AtomicI16, "h");
+    atomic_int!(u16, AtomicU16, "h");
+    atomic_int!(i32, AtomicI32, "");
+    atomic_int!(u32, AtomicU32, "");
+    atomic_int!(isize, AtomicIsize, "");
+    atomic_int!(usize, AtomicUsize, "");
+
+    trait AtomicOperations: Sized {
+        unsafe fn atomic_load(src: *const Self, order: Ordering) -> Self;
+        unsafe fn atomic_store(dst: *mut Self, val: Self, order: Ordering);
+    }
+}

src/imp/interrupt/armv6m.rs

@@ -3,6 +3,8 @@
 #[cfg(not(portable_atomic_no_asm))]
 use core::arch::asm;
 
+pub(super) use core::sync::atomic;
+
 #[derive(Clone, Copy)]
 pub(super) struct WasEnabled(bool);
 

src/imp/interrupt/mod.rs

@@ -27,14 +27,8 @@
 // CAS together with atomic load/store. The load/store will not be
 // called while interrupts are disabled, and since the load/store is
 // atomic, it is not affected by interrupts even if interrupts are enabled.
-#[cfg(target_arch = "msp430")]
-use super::msp430 as atomic;
-#[cfg(any(target_arch = "riscv32", target_arch = "riscv64"))]
-use super::riscv as atomic;
-// On pre-v6 ARM, we cannot use core::sync::atomic here because they call the
-// `__sync_*` builtins for non-relaxed loads and stores.
-#[cfg(portable_atomic_armv6m)]
-use core::sync::atomic;
+#[cfg(not(target_arch = "avr"))]
+use arch::atomic;
 
 #[cfg_attr(portable_atomic_armv6m, path = "armv6m.rs")]
 #[cfg_attr(
@@ -116,24 +110,12 @@ impl AtomicBool {
         crate::utils::assert_load_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            )))]
+            #[cfg(not(target_arch = "avr"))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe { (*(self as *const Self as *const atomic::AtomicBool)).load(order) },
-            #[cfg(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            ))]
+            #[cfg(target_arch = "avr")]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -147,26 +129,14 @@ impl AtomicBool {
         crate::utils::assert_store_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            )))]
+            #[cfg(not(target_arch = "avr"))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe {
                 (*(self as *const Self as *const atomic::AtomicBool)).store(val, order);
             },
-            #[cfg(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            ))]
+            #[cfg(target_arch = "avr")]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -318,24 +288,12 @@ impl<T> AtomicPtr<T> {
         crate::utils::assert_load_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            )))]
+            #[cfg(not(target_arch = "avr"))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe { (*(self as *const Self as *const atomic::AtomicPtr<T>)).load(order) },
-            #[cfg(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            ))]
+            #[cfg(target_arch = "avr")]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -349,26 +307,14 @@ impl<T> AtomicPtr<T> {
         crate::utils::assert_store_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            )))]
+            #[cfg(not(target_arch = "avr"))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe {
                 (*(self as *const Self as *const atomic::AtomicPtr<T>)).store(ptr, order);
             },
-            #[cfg(any(
-                target_arch = "avr",
-                all(
-                    target_arch = "arm",
-                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
-                )
-            ))]
+            #[cfg(target_arch = "avr")]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -472,32 +418,14 @@ macro_rules! atomic_int {
                 crate::utils::assert_load_ordering(order);
                 #[deny(unreachable_patterns)]
                 match () {
-                    #[cfg(not(any(
-                        target_arch = "avr",
-                        all(
-                            target_arch = "arm",
-                            not(any(
-                                target_feature = "v6",
-                                portable_atomic_target_feature = "v6"
-                            ))
-                        )
-                    )))]
+                    #[cfg(not(target_arch = "avr"))]
                     // SAFETY: any data races are prevented by atomic intrinsics (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.
                     () => unsafe {
                         (*(self as *const Self as *const atomic::$atomic_type)).load(order)
                     },
-                    #[cfg(any(
-                        target_arch = "avr",
-                        all(
-                            target_arch = "arm",
-                            not(any(
-                                target_feature = "v6",
-                                portable_atomic_target_feature = "v6"
-                            ))
-                        )
-                    ))]
+                    #[cfg(target_arch = "avr")]
                     // SAFETY: any data races are prevented by disabling interrupts (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.
@@ -511,32 +439,14 @@ macro_rules! atomic_int {
                 crate::utils::assert_store_ordering(order);
                 #[deny(unreachable_patterns)]
                 match () {
-                    #[cfg(not(any(
-                        target_arch = "avr",
-                        all(
-                            target_arch = "arm",
-                            not(any(
-                                target_feature = "v6",
-                                portable_atomic_target_feature = "v6"
-                            ))
-                        )
-                    )))]
+                    #[cfg(not(target_arch = "avr"))]
                     // SAFETY: any data races are prevented by atomic intrinsics (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.
                     () => unsafe {
                         (*(self as *const Self as *const atomic::$atomic_type)).store(val, order);
                     },
-                    #[cfg(any(
-                        target_arch = "avr",
-                        all(
-                            target_arch = "arm",
-                            not(any(
-                                target_feature = "v6",
-                                portable_atomic_target_feature = "v6"
-                            ))
-                        )
-                    ))]
+                    #[cfg(target_arch = "avr")]
                     // SAFETY: any data races are prevented by disabling interrupts (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.

src/imp/interrupt/msp430.rs

@@ -3,6 +3,8 @@
 #[cfg(not(portable_atomic_no_asm))]
 use core::arch::asm;
 
+pub(super) use super::super::msp430 as atomic;
+
 #[derive(Clone, Copy)]
 pub(super) struct WasEnabled(bool);
 

src/imp/interrupt/riscv.rs

@@ -3,6 +3,8 @@
 #[cfg(not(portable_atomic_no_asm))]
 use core::arch::asm;
 
+pub(super) use super::super::riscv as atomic;
+
 #[derive(Clone, Copy)]
 pub(super) struct WasEnabled(bool);
 

src/imp/msp430.rs

@@ -288,7 +288,7 @@ atomic_int!(u16, AtomicU16, ".w");
 atomic_int!(isize, AtomicIsize, ".w");
 atomic_int!(usize, AtomicUsize, ".w");
 
-trait AtomicOperations {
+trait AtomicOperations: Sized {
     unsafe fn atomic_load(src: *const Self) -> Self;
     unsafe fn atomic_store(dst: *mut Self, val: Self);
 }