irq: Optimize interrupt save/restore

Avoid unnecessary mask and branch instructions

src/interrupt.rs

@@ -30,42 +30,115 @@
 
 pub use bare_metal::{CriticalSection, Mutex, Nr};
 
+use core::marker::PhantomData;
 #[cfg(target_arch = "avr")]
 use core::arch::asm;
 
-#[inline]
-/// Disables all interrupts
+/// Opaque structure for storing the global interrupt flag status.
 ///
-/// Returns a bool, reflecting whether interrupts were enabled prior to calling this method.
-pub fn disable() -> bool {
+/// This structure does not implement `Copy` and `Clone`,
+/// because the user shall not duplicate and pass it twice to [crate::interrupt::restore].
+pub struct IrqFlag {
+    // The saved SREG.
+    sreg: u8,
+    // Use PhantomData with raw pointer to disable Send and Sync auto-trait impls.
+    // Negative impl of Send/Sync is not stabilized in rustc, yet.
+    // This is not strictly necessary for memory safety.
+    // It's just to keep the user from passing this struct between interrupt contexts,
+    // which would not be meaningful.
+    _phantom: PhantomData<*mut u8>,
+}
+
+impl IrqFlag {
+    #[inline(always)]
+    fn new(sreg: u8) -> IrqFlag {
+        IrqFlag {
+            sreg,
+            _phantom: PhantomData,
+        }
+    }
+
+    /// Check the status of the saved global interrupt flag.
+    ///
+    /// Returns true, if the saved global interrupt flag is set (IRQs enabled).
+    /// Otherwise returns false.
+    ///
+    /// This method can be used to check whether interrupts were enabled
+    /// before the [crate::interrupt::disable_save] call.
+    /// You probably shouldn't make your program behavior dependent on this state.
+    /// Consider using a different design.
+    #[inline(always)]
+    pub fn enabled(&self) -> bool {
+        self.sreg & 0x80 != 0
+    }
+}
+
+/// Disable the global interrupt flag.
+///
+/// *Hint*: Most of the time you probably don't want to use this function directly.
+///         Consider creating a critical section with [crate::interrupt::free] instead.
+///
+/// This function is an optimization fence.
+/// That means memory accesses will not be re-ordered by the compiler across this function call.
+#[inline(always)]
+pub fn disable() {
     cfg_if::cfg_if! {
         if #[cfg(target_arch = "avr")] {
-            // Store current state
-            let sreg: u8;
+            // Disable interrupts
+            unsafe { asm!("cli") };
+        } else {
+            unimplemented!()
+        }
+    }
+}
 
+/// Disable the global interrupt flag and return an opaque representation of the previous flag status.
+///
+/// *Hint*: Most of the time you probably don't want to use this function directly.
+///         Consider creating a critical section with [crate::interrupt::free] instead.
+///
+/// This function is an optimization fence.
+/// That means memory accesses will not be re-ordered by the compiler across this function call.
+///
+/// Returns an object that contains the status of the global interrupt flag from *before* the `disable_save()` call.
+/// This object shall later be passed to the [crate::interrupt::restore] function.
+#[inline(always)]
+pub fn disable_save() -> IrqFlag {
+    let sreg;
+    cfg_if::cfg_if! {
+        if #[cfg(target_arch = "avr")] {
+            // Store current state
             unsafe {
                 asm!(
                     "in {sreg}, 0x3F",
                     sreg = out(reg) sreg,
                 )
             };
-
-            // Disable interrupts
-            unsafe { asm!("cli") };
-
-            sreg & 0x80 == 0x80
         } else {
             unimplemented!()
         }
     }
+    // Disable interrupts
+    disable();
+
+    IrqFlag::new(sreg)
 }
 
-#[inline]
-/// Enables all the interrupts
+/// Enable the global interrupt flag.
+///
+/// *Warning*: This function enables interrupts, no matter what the enable-state was before [crate::interrupt::disable].
+///            Especially in library code, where the previous interrupt state may be unknown,
+///            this function call shall be avoided.
+///            Most of the time you probably don't want to use this function directly.
+///            Consider creating a critical section with [crate::interrupt::free] instead.
+///
+/// This function is an optimization fence.
+/// That means memory accesses will not be re-ordered by the compiler across this function call.
 ///
 /// # Safety
 ///
 /// - Do not call this function inside an [crate::interrupt::free] critical section
+#[inline(always)]
 pub unsafe fn enable() {
     cfg_if::cfg_if! {
         if #[cfg(target_arch = "avr")] {
@@ -76,24 +149,79 @@ pub unsafe fn enable() {
     }
 }
 
+/// Restore the global interrupt flag to its previous state before [crate::interrupt::disable_save].
+///
+/// *Hint*: Most of the time you probably don't want to use this function directly.
+///         Consider creating a critical section with [crate::interrupt::free] instead.
+///
+/// This function is an optimization fence.
+/// That means memory accesses will not be re-ordered by the compiler across this function call.
+///
+/// # Safety
+///
+/// - Do not call this function inside an [crate::interrupt::free] critical section
+#[inline(always)]
+pub unsafe fn restore(irq_flag: IrqFlag) {
+    cfg_if::cfg_if! {
+        if #[cfg(target_arch = "avr")] {
+            // Restore global interrupt flag in SREG.
+            // This also clobbers all other bits in SREG.
+            asm!(
+                "out 0x3F, {sreg}",
+                sreg = in(reg) irq_flag.sreg,
+            );
+        } else {
+            unimplemented!()
+        }
+    }
+}
+
+/// Check whether the global interrupt flag is currently enabled (in SREG).
+///
+/// *Warning*: You shouldn't use this to hand craft your own memory/interrupt safety mechanisms.
+///            This function may be used for things such as deciding whether to do
+///            expensive calculations in library code, or similar things.
+///
+/// This function is **not** an optimization fence.
+/// That means memory accesses *can* be re-ordered by the compiler across this function call.
+#[inline(always)]
+pub fn is_enabled() -> bool {
+    let sreg;
+    cfg_if::cfg_if! {
+        if #[cfg(target_arch = "avr")] {
+            // Store current state
+            unsafe {
+                asm!(
+                    "in {sreg}, 0x3F",
+                    sreg = out(reg) sreg,
+                    options(readonly, preserves_flags, nostack),
+                )
+            };
+        } else {
+            unimplemented!()
+        }
+    }
+
+    IrqFlag::new(sreg).enabled()
+}
+
 /// Execute closure `f` in an interrupt-free context.
 ///
 /// This as also known as a "critical section".
+#[inline(always)]
 pub fn free<F, R>(f: F) -> R
 where
     F: FnOnce(&CriticalSection) -> R,
 {
     cfg_if::cfg_if! {
         if #[cfg(target_arch = "avr")] {
-            // Disable interrupts
-            let interrupts_enabled = disable();
+            // Disable interrupts. This is an optimization fence.
+            let irq_flag = disable_save();
 
             let r = f(unsafe { &CriticalSection::new() });
 
-            // Restore interrupt state
-            if interrupts_enabled {
-                unsafe { enable(); }
-            }
+            // Restore interrupt state. This is an optimization fence.
+            unsafe { restore(irq_flag); }
 
             r
         } else {