interrupt: Optimize restore on AVR and MSP430

src/imp/interrupt/armv4t.rs

@@ -51,6 +51,8 @@ pub(super) fn disable() -> State {
 pub(super) unsafe fn restore(State(cpsr): State) {
     // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`,
     unsafe {
+        // This clobbers the entire CPSR. See msp430.rs to safety on this.
+        //
         // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled.
         asm!("msr cpsr_c, {0}", in(reg) cpsr, options(nostack));
     }

src/imp/interrupt/avr.rs

@@ -4,11 +4,11 @@
 use core::arch::asm;
 
 #[derive(Clone, Copy)]
-pub(super) struct WasEnabled(bool);
+pub(super) struct State(u8);
 
 /// Disables interrupts and returns the previous interrupt state.
 #[inline]
-pub(super) fn disable() -> WasEnabled {
+pub(super) fn disable() -> State {
     let sreg: u8;
     // SAFETY: reading the status register (SREG) and disabling interrupts are safe.
     // (see module-level comments of interrupt/mod.rs on the safety of using privileged instructions)
@@ -25,28 +25,25 @@ pub(super) fn disable() -> WasEnabled {
         );
         #[cfg(portable_atomic_no_asm)]
         {
-            llvm_asm!("in $0,0x3F" :"=r"(sreg) ::: "volatile");
+            llvm_asm!("in $0, 0x3F" : "=r"(sreg) ::: "volatile");
             llvm_asm!("cli" ::: "memory" : "volatile");
         }
     }
-    // I (Global Interrupt Enable) bit (1 << 7)
-    WasEnabled(sreg & 0x80 != 0)
+    State(sreg)
 }
 
 /// Restores the previous interrupt state.
 #[inline]
-pub(super) unsafe fn restore(WasEnabled(was_enabled): WasEnabled) {
-    if was_enabled {
-        // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`,
-        // and we've checked that interrupts were enabled before disabling interrupts.
-        unsafe {
-            // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled.
-            // Do not use `preserves_flags` because SEI modifies the I bit of the status register (SREG).
-            // Refs: https://ww1.microchip.com/downloads/en/DeviceDoc/AVR-InstructionSet-Manual-DS40002198.pdf#page=127
-            #[cfg(not(portable_atomic_no_asm))]
-            asm!("sei", options(nostack));
-            #[cfg(portable_atomic_no_asm)]
-            llvm_asm!("sei" ::: "memory" : "volatile");
-        }
+pub(super) unsafe fn restore(State(sreg): State) {
+    // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`,
+    unsafe {
+        // This clobbers the entire status register. See msp430.rs to safety on this.
+        //
+        // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled.
+        // Do not use `preserves_flags` because OUT modifies the status register (SREG).
+        #[cfg(not(portable_atomic_no_asm))]
+        asm!("out 0x3F, {0}", in(reg) sreg, options(nostack));
+        #[cfg(portable_atomic_no_asm)]
+        llvm_asm!("out 0x3F, $0" :: "r"(sreg) : "memory" : "volatile");
     }
 }

src/imp/interrupt/msp430.rs

@@ -6,11 +6,11 @@ use core::arch::asm;
 pub(super) use super::super::msp430 as atomic;
 
 #[derive(Clone, Copy)]
-pub(super) struct WasEnabled(bool);
+pub(super) struct State(u16);
 
 /// Disables interrupts and returns the previous interrupt state.
 #[inline]
-pub(super) fn disable() -> WasEnabled {
+pub(super) fn disable() -> State {
     let r: u16;
     // SAFETY: reading the status register and disabling interrupts are safe.
     // (see module-level comments of interrupt/mod.rs on the safety of using privileged instructions)
@@ -31,24 +31,26 @@ pub(super) fn disable() -> WasEnabled {
             llvm_asm!("dint { nop" ::: "memory" : "volatile");
         }
     }
-    // GIE (global interrupt enable) bit (1 << 3)
-    WasEnabled(r & 0x8 != 0)
+    State(r)
 }
 
 /// Restores the previous interrupt state.
 #[inline]
-pub(super) unsafe fn restore(WasEnabled(was_enabled): WasEnabled) {
-    if was_enabled {
-        // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`,
-        // and we've checked that interrupts were enabled before disabling interrupts.
-        unsafe {
-            // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled.
-            // Do not use `preserves_flags` because EINT modifies the GIE (global interrupt enable) bit of the status register.
-            // Refs: http://mspgcc.sourceforge.net/manual/x951.html
-            #[cfg(not(portable_atomic_no_asm))]
-            asm!("nop {{ eint {{ nop", options(nostack));
-            #[cfg(portable_atomic_no_asm)]
-            llvm_asm!("nop { eint { nop" ::: "memory" : "volatile");
-        }
+pub(super) unsafe fn restore(State(r): State) {
+    // 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.
+        #[cfg(not(portable_atomic_no_asm))]
+        asm!("nop {{ mov {0}, R2 {{ nop", in(reg) r, options(nostack));
+        #[cfg(portable_atomic_no_asm)]
+        llvm_asm!("nop { mov $0, R2 { nop" :: "r"(r) : "memory" : "volatile");
     }
 }