Support swap/CAS on MSP430 (experimental)

taiki-e · Oct 20, 2024 · 4fb09a7 · 4fb09a7
1 parent aaac3ad
commit 4fb09a7
Show file tree

Hide file tree

Showing 7 changed files with 188 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -37,8 +37,8 @@ Currently, x86, x86_64, Arm, AArch64, RISC-V, LoongArch64, MIPS32, MIPS64, Power
 | powerpc64 \[4]                   | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64           | ✓          | ✓        |
 | powerpc64 (pwr8+) \[4] \[6]      | i128,u128                                           | ✓          | ✓        |
 | s390x \[4]                       | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64,i128,u128 | ✓          | ✓        |
-| msp430 \[4]                      | isize,usize,i8,u8,i16,u16                           | ✓          |          |
 | arm64ec \[4]                     | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64,i128,u128 | ✓          | ✓        |
+| msp430 \[4] (experimental)       | isize,usize,i8,u8,i16,u16                           | ✓          | ✓        |
 | avr \[4] (experimental)          | isize,usize,i8,u8,i16,u16                           | ✓          | ✓        |
 | hexagon \[4] (experimental)      | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64           | ✓          | ✓        |
 

diff --git a/src/arch/avr.rs b/src/arch/avr.rs
@@ -112,14 +112,15 @@ macro_rules! atomic {
                 val: MaybeUninit<Self>,
                 _order: Ordering,
             ) -> MaybeUninit<Self> {
-                // SAFETY: the caller must uphold the safety contract.
-                unsafe {
-                    let s = disable();
-                    let out = dst.read();
-                    dst.write(val);
-                    restore(s);
-                    out
-                }
+                let s = disable();
+                // SAFETY: the caller must guarantee that pointer is valid and properly aligned.
+                // On single-core systems, disabling interrupts is enough to prevent data race.
+                let out = unsafe { dst.read() };
+                // SAFETY: see dst.read()
+                unsafe { dst.write(val) }
+                // SAFETY: the state was retrieved by the previous `disable`.
+                unsafe { restore(s) }
+                out
             }
         }
         #[cfg(not(atomic_maybe_uninit_no_asm_maybe_uninit))]
@@ -132,22 +133,26 @@ macro_rules! atomic {
                 _success: Ordering,
                 _failure: Ordering,
             ) -> (MaybeUninit<Self>, bool) {
-                // SAFETY: the caller must uphold the safety contract.
-                unsafe {
-                    let s = disable();
-                    let out = dst.read();
-                    // transmute from MaybeUninit<{i,u}{8,16,size}> to MaybeUninit<u{8,16}>
-                    #[allow(clippy::useless_transmute)] // only useless when Self is u{8,16}
-                    let r = $cmp(
+                let s = disable();
+                // SAFETY: the caller must guarantee that pointer is valid and properly aligned.
+                // On single-core systems, disabling interrupts is enough to prevent data race.
+                let out = unsafe { dst.read() };
+                // transmute from MaybeUninit<{i,u}{8,16,size}> to MaybeUninit<u{8,16}>
+                #[allow(clippy::useless_transmute)] // only useless when Self is u{8,16}
+                // SAFETY: Self and $cmp_ty has the same layout
+                let r = unsafe {
+                    $cmp(
                         core::mem::transmute::<MaybeUninit<Self>, MaybeUninit<$cmp_ty>>(old),
                         core::mem::transmute::<MaybeUninit<Self>, MaybeUninit<$cmp_ty>>(out),
-                    );
-                    if r {
-                        dst.write(new);
-                    }
-                    restore(s);
-                    (out, r)
+                    )
+                };
+                if r {
+                    // SAFETY: see dst.read()
+                    unsafe { dst.write(new) }
                 }
+                // SAFETY: the state was retrieved by the previous `disable`.
+                unsafe { restore(s) }
+                (out, r)
             }
         }
     };

diff --git a/src/arch/cfgs/msp430.rs b/src/arch/cfgs/msp430.rs
@@ -42,10 +42,22 @@ macro_rules! cfg_has_atomic_128 {
 macro_rules! cfg_no_atomic_128 {
     ($($tt:tt)*) => { $($tt)* };
 }
+#[cfg(not(atomic_maybe_uninit_no_asm_maybe_uninit))]
+#[macro_export]
+macro_rules! cfg_has_atomic_cas {
+    ($($tt:tt)*) => { $($tt)* };
+}
+#[cfg(not(atomic_maybe_uninit_no_asm_maybe_uninit))]
+#[macro_export]
+macro_rules! cfg_no_atomic_cas {
+    ($($tt:tt)*) => {};
+}
+#[cfg(atomic_maybe_uninit_no_asm_maybe_uninit)]
 #[macro_export]
 macro_rules! cfg_has_atomic_cas {
     ($($tt:tt)*) => {};
 }
+#[cfg(atomic_maybe_uninit_no_asm_maybe_uninit)]
 #[macro_export]
 macro_rules! cfg_no_atomic_cas {
     ($($tt:tt)*) => { $($tt)* };

diff --git a/src/arch/msp430.rs b/src/arch/msp430.rs
@@ -3,15 +3,51 @@
 /*
 MSP430
 
-Refs: https://www.ti.com/lit/ug/slau208q/slau208q.pdf
+Refs:
+- MSP430x5xx and MSP430x6xx Family User's Guide https://www.ti.com/lit/ug/slau208q/slau208q.pdf
+- portable-atomic https://github.com/taiki-e/portable-atomic
 */
 
 #[path = "cfgs/msp430.rs"]
 mod cfgs;
 
 use core::{arch::asm, mem::MaybeUninit, sync::atomic::Ordering};
 
-use crate::raw::{AtomicLoad, AtomicStore};
+use crate::raw::{AtomicCompareExchange, AtomicLoad, AtomicStore, AtomicSwap};
+
+// See portable-atomic's interrupt module for more.
+#[inline(always)]
+fn disable() -> u16 {
+    let sr: u16;
+    // SAFETY: reading the status register and disabling interrupts are safe.
+    unsafe {
+        // Do not use `nomem` and `readonly` because prevent subsequent memory accesses from being reordered before interrupts are disabled.
+        // Do not use `preserves_flags` because DINT modifies the GIE (global interrupt enable) bit of the status register.
+        asm!(
+            "mov r2, {0}",
+            "dint {{ nop",
+            out(reg) sr,
+            options(nostack),
+        );
+    }
+    sr
+}
+#[inline(always)]
+unsafe fn restore(sr: u16) {
+    // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`,
+    unsafe {
+        // This clobbers the entire status register, but we never explicitly modify
+        // flags within a critical session, and the only flags that may be changed
+        // within a critical session are the arithmetic flags that are changed as
+        // a side effect of arithmetic operations, etc., which LLVM recognizes,
+        // so it is safe to clobber them here.
+        // See also the discussion at https://github.com/taiki-e/portable-atomic/pull/40.
+        //
+        // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled.
+        // Do not use `preserves_flags` because MOV modifies the status register.
+        asm!("nop {{ mov {0}, r2 {{ nop", in(reg) sr, options(nostack));
+    }
+}
 
 macro_rules! atomic {
     ($int_type:ident, $asm_suffix:tt) => {
@@ -57,6 +93,58 @@ macro_rules! atomic {
                 }
             }
         }
+        impl AtomicSwap for $int_type {
+            #[inline]
+            unsafe fn atomic_swap(
+                dst: *mut MaybeUninit<Self>,
+                val: MaybeUninit<Self>,
+                _order: Ordering,
+            ) -> MaybeUninit<Self> {
+                let s = disable();
+                // SAFETY: the caller must guarantee that pointer is valid and properly aligned.
+                // On single-core systems, disabling interrupts is enough to prevent data race.
+                let out = unsafe { dst.read() };
+                // SAFETY: see dst.read()
+                unsafe { dst.write(val) }
+                // SAFETY: the state was retrieved by the previous `disable`.
+                unsafe { restore(s) }
+                out
+            }
+        }
+        impl AtomicCompareExchange for $int_type {
+            #[inline]
+            unsafe fn atomic_compare_exchange(
+                dst: *mut MaybeUninit<Self>,
+                old: MaybeUninit<Self>,
+                new: MaybeUninit<Self>,
+                _success: Ordering,
+                _failure: Ordering,
+            ) -> (MaybeUninit<Self>, bool) {
+                let s = disable();
+                // SAFETY: the caller must guarantee that pointer is valid and properly aligned.
+                // On single-core systems, disabling interrupts is enough to prevent data race.
+                let out = unsafe { dst.read() };
+                // SAFETY: calling xor is safe.
+                let r = unsafe {
+                    let r: $int_type;
+                    asm!(
+                        concat!("xor", $asm_suffix, " {b}, {a}"),
+                        a = inout(reg) old => r,
+                        b = in(reg) out,
+                        // Do not use `preserves_flags` because XOR modifies the V, N, Z, and C bits of the status register.
+                        options(pure, nomem, nostack),
+                    );
+                    r == 0
+                };
+                if r {
+                    // SAFETY: see dst.read()
+                    unsafe { dst.write(new) }
+                }
+                // SAFETY: the state was retrieved by the previous `disable`.
+                unsafe { restore(s) }
+                (out, r)
+            }
+        }
     };
 }
 

diff --git a/src/arch_legacy/msp430.rs b/src/arch_legacy/msp430.rs
@@ -3,15 +3,51 @@
 /*
 MSP430
 
-Refs: https://www.ti.com/lit/ug/slau208q/slau208q.pdf
+Refs:
+- MSP430x5xx and MSP430x6xx Family User's Guide https://www.ti.com/lit/ug/slau208q/slau208q.pdf
+- portable-atomic https://github.com/taiki-e/portable-atomic
 */
 
 #[path = "../arch/cfgs/msp430.rs"]
 mod cfgs;
 
 use core::{arch::asm, mem::MaybeUninit, sync::atomic::Ordering};
 
-use crate::raw::{AtomicLoad, AtomicStore};
+use crate::raw::{AtomicLoad, AtomicStore, AtomicSwap};
+
+// See portable-atomic's interrupt module for more.
+#[inline(always)]
+fn disable() -> u16 {
+    let sr: u16;
+    // SAFETY: reading the status register and disabling interrupts are safe.
+    unsafe {
+        // Do not use `nomem` and `readonly` because prevent subsequent memory accesses from being reordered before interrupts are disabled.
+        // Do not use `preserves_flags` because DINT modifies the GIE (global interrupt enable) bit of the status register.
+        asm!(
+            "mov r2, {0}",
+            "dint {{ nop",
+            out(reg) sr,
+            options(nostack),
+        );
+    }
+    sr
+}
+#[inline(always)]
+unsafe fn restore(sr: u16) {
+    // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`,
+    unsafe {
+        // This clobbers the entire status register, but we never explicitly modify
+        // flags within a critical session, and the only flags that may be changed
+        // within a critical session are the arithmetic flags that are changed as
+        // a side effect of arithmetic operations, etc., which LLVM recognizes,
+        // so it is safe to clobber them here.
+        // See also the discussion at https://github.com/taiki-e/portable-atomic/pull/40.
+        //
+        // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled.
+        // Do not use `preserves_flags` because MOV modifies the status register.
+        asm!("nop {{ mov {0}, r2 {{ nop", in(reg) sr, options(nostack));
+    }
+}
 
 macro_rules! atomic {
     ($int_type:ident, $asm_suffix:tt) => {
@@ -68,6 +104,24 @@ macro_rules! atomic {
                 }
             }
         }
+        impl AtomicSwap for $int_type {
+            #[inline]
+            unsafe fn atomic_swap(
+                dst: *mut MaybeUninit<Self>,
+                val: MaybeUninit<Self>,
+                _order: Ordering,
+            ) -> MaybeUninit<Self> {
+                let s = disable();
+                // SAFETY: the caller must guarantee that pointer is valid and properly aligned.
+                // On single-core systems, disabling interrupts is enough to prevent data race.
+                let out = unsafe { dst.read() };
+                // SAFETY: see dst.read()
+                unsafe { dst.write(val) }
+                // SAFETY: the state was retrieved by the previous `disable`.
+                unsafe { restore(s) }
+                out
+            }
+        }
     };
 }
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -31,8 +31,8 @@ Currently, x86, x86_64, Arm, AArch64, RISC-V, LoongArch64, MIPS32, MIPS64, Power
 | powerpc64 \[4]                   | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64           | ✓          | ✓        |
 | powerpc64 (pwr8+) \[4] \[6]      | i128,u128                                           | ✓          | ✓        |
 | s390x \[4]                       | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64,i128,u128 | ✓          | ✓        |
-| msp430 \[4]                      | isize,usize,i8,u8,i16,u16                           | ✓          |          |
 | arm64ec \[4]                     | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64,i128,u128 | ✓          | ✓        |
+| msp430 \[4] (experimental)       | isize,usize,i8,u8,i16,u16                           | ✓          | ✓        |
 | avr \[4] (experimental)          | isize,usize,i8,u8,i16,u16                           | ✓          | ✓        |
 | hexagon \[4] (experimental)      | isize,usize,i8,u8,i16,u16,i32,u32,i64,u64           | ✓          | ✓        |
 

diff --git a/tests/msp430/src/main.rs b/tests/msp430/src/main.rs
@@ -38,7 +38,6 @@ macro_rules! __test_atomic {
                 }
             }
         }
-        cfg_has_atomic_cas! {
         swap();
         fn swap() {
             unsafe {
@@ -53,6 +52,7 @@ macro_rules! __test_atomic {
                 }
             }
         }
+        cfg_has_atomic_cas! {
         compare_exchange();
         fn compare_exchange() {
             unsafe {
@@ -191,10 +191,10 @@ fn load_orderings() -> [Ordering; 3] {
 fn store_orderings() -> [Ordering; 3] {
     [Ordering::Relaxed, Ordering::Release, Ordering::SeqCst]
 }
-cfg_has_atomic_cas! {
 fn swap_orderings() -> [Ordering; 5] {
     [Ordering::Relaxed, Ordering::Release, Ordering::Acquire, Ordering::AcqRel, Ordering::SeqCst]
 }
+cfg_has_atomic_cas! {
 fn compare_exchange_orderings() -> [(Ordering, Ordering); 15] {
     [
         (Ordering::Relaxed, Ordering::Relaxed),