From ad2bf4e77c42b547405584e2751c51fa1b8cbd25 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Mon, 9 Jan 2023 21:02:53 +0100 Subject: [PATCH] More work on new spawn/executor --- Cargo.toml | 2 + macros/src/codegen/async_dispatchers.rs | 50 ++++--------------------- macros/src/codegen/module.rs | 29 +++++++------- macros/src/codegen/software_tasks.rs | 14 +------ macros/src/codegen/util.rs | 10 ----- src/export.rs | 25 +------------ src/export/executor.rs | 11 ++++-- xtask/src/command.rs | 10 ++++- 8 files changed, 43 insertions(+), 108 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cad929196c..6eb691df6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,8 @@ rtic-monotonic = "1.0.0" rtic-core = "1.0.0" heapless = "0.7.7" bare-metal = "1.0.0" +#portable-atomic = { version = "0.3.19" } +atomic-polyfill = "1" [build-dependencies] version_check = "0.9" diff --git a/macros/src/codegen/async_dispatchers.rs b/macros/src/codegen/async_dispatchers.rs index 341f76ff6f..012bd61a36 100644 --- a/macros/src/codegen/async_dispatchers.rs +++ b/macros/src/codegen/async_dispatchers.rs @@ -16,11 +16,10 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 { items.push(quote!( #[allow(non_camel_case_types)] - type #type_name = impl core::future::Future + 'static; + type #type_name = impl core::future::Future; #[allow(non_upper_case_globals)] - static #exec_name: - rtic::RacyCell> = - rtic::RacyCell::new(rtic::export::executor::AsyncTaskExecutor::new()); + static #exec_name: rtic::export::executor::AsyncTaskExecutor<#type_name> = + rtic::export::executor::AsyncTaskExecutor::new(); )); } @@ -47,38 +46,13 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 { let exec_name = util::internal_task_ident(name, "EXEC"); // let task = &app.software_tasks[name]; // let cfgs = &task.cfgs; - let executor_run_ident = util::executor_run_ident(name); - - let rq = util::rq_async_ident(name); - - items.push(quote!( - #[doc(hidden)] - #[allow(non_camel_case_types)] - #[allow(non_upper_case_globals)] - static #rq: core::sync::atomic::AtomicBool = core::sync::atomic::AtomicBool::new(false); - )); stmts.push(quote!( - if !(&*#exec_name.get()).is_running() { - // TODO Fix this to be compare and swap - if #rq.load(core::sync::atomic::Ordering::Relaxed) { - #rq.store(false, core::sync::atomic::Ordering::Relaxed); - - (&mut *#exec_name.get_mut()).spawn(#name(#name::Context::new())); - #executor_run_ident.store(true, core::sync::atomic::Ordering::Relaxed); - } - } - - if #executor_run_ident.load(core::sync::atomic::Ordering::Relaxed) { - #executor_run_ident.store(false, core::sync::atomic::Ordering::Relaxed); - if (&mut *#exec_name.get_mut()).poll(|| { - #executor_run_ident.store(true, core::sync::atomic::Ordering::Release); + if #exec_name.check_and_clear_pending() { + #exec_name.poll(|| { + #exec_name.set_pending(); #pend_interrupt - }) && #rq.load(core::sync::atomic::Ordering::Relaxed) { - // If the ready queue is not empty and the executor finished, restart this - // dispatch to check if the executor should be restarted. - #pend_interrupt - } + }); } )); } @@ -96,12 +70,7 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 { const PRIORITY: u8 = #level; rtic::export::run(PRIORITY, || { - // Have the acquire/release semantics outside the checks to no overdo it - core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire); - #(#stmts)* - - core::sync::atomic::fence(core::sync::atomic::Ordering::Release); }); } )); @@ -110,12 +79,7 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 { #[allow(non_snake_case)] unsafe fn #dispatcher_name() -> ! { loop { - // Have the acquire/release semantics outside the checks to no overdo it - core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire); - #(#stmts)* - - core::sync::atomic::fence(core::sync::atomic::Ordering::Release); } } )); diff --git a/macros/src/codegen/module.rs b/macros/src/codegen/module.rs index 70fbb5e651..19cf2417bb 100644 --- a/macros/src/codegen/module.rs +++ b/macros/src/codegen/module.rs @@ -98,6 +98,7 @@ pub fn codegen(ctxt: Context, app: &App, analysis: &Analysis) -> TokenStream2 { }; let internal_context_name = util::internal_task_ident(name, "Context"); + let exec_name = util::internal_task_ident(name, "EXEC"); items.push(quote!( #(#cfgs)* @@ -147,25 +148,25 @@ pub fn codegen(ctxt: Context, app: &App, analysis: &Analysis) -> TokenStream2 { let internal_spawn_ident = util::internal_task_ident(name, "spawn"); // Spawn caller - let rq = util::rq_async_ident(name); items.push(quote!( - - #(#cfgs)* - /// Spawns the task directly - #[allow(non_snake_case)] - #[doc(hidden)] - pub fn #internal_spawn_ident() -> Result<(), ()> { - unsafe { - // TODO: Fix this to be compare and swap - if #rq.load(core::sync::atomic::Ordering::Acquire) { - Err(()) - } else { - #rq.store(true, core::sync::atomic::Ordering::Release); + #(#cfgs)* + /// Spawns the task directly + #[allow(non_snake_case)] + #[doc(hidden)] + pub fn #internal_spawn_ident() -> Result<(), ()> { + if #exec_name.try_reserve() { + unsafe { + // TODO: Add args here + #exec_name.spawn_unchecked(#name(#name::Context::new())); + } #pend_interrupt + Ok(()) + } else { + Err(()) } } - })); + )); module_items.push(quote!( #(#cfgs)* diff --git a/macros/src/codegen/software_tasks.rs b/macros/src/codegen/software_tasks.rs index 4cb1fa95f6..b923283269 100644 --- a/macros/src/codegen/software_tasks.rs +++ b/macros/src/codegen/software_tasks.rs @@ -1,7 +1,7 @@ use crate::syntax::{ast::App, Context}; use crate::{ analyze::Analysis, - codegen::{local_resources_struct, module, shared_resources_struct, util}, + codegen::{local_resources_struct, module, shared_resources_struct}, }; use proc_macro2::TokenStream as TokenStream2; use quote::quote; @@ -13,18 +13,6 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 { // Any task for (name, task) in app.software_tasks.iter() { - let executor_ident = util::executor_run_ident(name); - mod_app.push(quote!( - #[allow(non_camel_case_types)] - #[allow(non_upper_case_globals)] - #[doc(hidden)] - static #executor_ident: core::sync::atomic::AtomicBool = - core::sync::atomic::AtomicBool::new(false); - )); - - // `${task}Resources` - - // `${task}Locals` if !task.args.local_resources.is_empty() { let (item, constructor) = local_resources_struct::codegen(Context::SoftwareTask(name), app); diff --git a/macros/src/codegen/util.rs b/macros/src/codegen/util.rs index a0caf0aeef..0558d9d102 100644 --- a/macros/src/codegen/util.rs +++ b/macros/src/codegen/util.rs @@ -49,11 +49,6 @@ pub fn impl_mutex( ) } -/// Generates an identifier for the `EXECUTOR_RUN` atomics (`async` API) -pub fn executor_run_ident(task: &Ident) -> Ident { - mark_internal_name(&format!("{task}_EXECUTOR_RUN")) -} - pub fn interrupt_ident() -> Ident { let span = Span::call_site(); Ident::new("interrupt", span) @@ -151,11 +146,6 @@ pub fn local_resources_ident(ctxt: Context, app: &App) -> Ident { mark_internal_name(&s) } -/// Generates an identifier for a ready queue, async task version -pub fn rq_async_ident(async_task_name: &Ident) -> Ident { - mark_internal_name(&format!("ASYNC_TASK_{async_task_name}_RQ")) -} - /// Suffixed identifier pub fn suffixed(name: &str) -> Ident { let span = Span::call_site(); diff --git a/src/export.rs b/src/export.rs index 6017dcf78f..cdca972785 100644 --- a/src/export.rs +++ b/src/export.rs @@ -1,5 +1,4 @@ pub use bare_metal::CriticalSection; -use core::sync::atomic::{AtomicBool, Ordering}; pub use cortex_m::{ asm::nop, asm::wfi, @@ -7,6 +6,8 @@ pub use cortex_m::{ peripheral::{scb::SystemHandler, DWT, NVIC, SCB, SYST}, Peripherals, }; +//pub use portable_atomic as atomic; +pub use atomic_polyfill as atomic; pub mod executor; @@ -72,28 +73,6 @@ where f(); } -pub struct Barrier { - inner: AtomicBool, -} - -impl Barrier { - pub const fn new() -> Self { - Barrier { - inner: AtomicBool::new(false), - } - } - - pub fn release(&self) { - self.inner.store(true, Ordering::Release); - } - - pub fn wait(&self) { - while !self.inner.load(Ordering::Acquire) { - core::hint::spin_loop() - } - } -} - /// Const helper to check architecture pub const fn have_basepri() -> bool { #[cfg(have_basepri)] diff --git a/src/export/executor.rs b/src/export/executor.rs index 874ee192be..2f88eff968 100644 --- a/src/export/executor.rs +++ b/src/export/executor.rs @@ -1,9 +1,9 @@ +use super::atomic::{AtomicBool, Ordering}; use core::{ cell::UnsafeCell, future::Future, mem::{self, MaybeUninit}, pin::Pin, - sync::atomic::{AtomicBool, Ordering}, task::{Context, Poll, RawWaker, RawWakerVTable, Waker}, }; @@ -53,9 +53,11 @@ impl AsyncTaskExecutor { self.running.load(Ordering::Relaxed) } - /// Checks if a waker has pended the executor. - pub fn is_pending(&self) -> bool { - self.pending.load(Ordering::Relaxed) + /// Checks if a waker has pended the executor and simultaneously clears the flag. + pub fn check_and_clear_pending(&self) -> bool { + self.pending + .compare_exchange(true, false, Ordering::Relaxed, Ordering::Relaxed) + .is_ok() } // Used by wakers to indicate that the executor needs to run. @@ -80,6 +82,7 @@ impl AsyncTaskExecutor { debug_assert!(self.running.load(Ordering::Relaxed)); self.task.get().write(MaybeUninit::new(future)); + self.set_pending(); } /// Poll the future in the executor. diff --git a/xtask/src/command.rs b/xtask/src/command.rs index 418f440cb0..4e903691a2 100644 --- a/xtask/src/command.rs +++ b/xtask/src/command.rs @@ -70,7 +70,15 @@ impl<'a> CargoCommand<'a> { features, mode, } => { - let mut args = vec!["+nightly", self.name(), "--examples", "--target", target]; + let mut args = vec![ + "+nightly", + self.name(), + "--examples", + "--target", + target, + "--features", + "test-critical-section", + ]; if let Some(feature_name) = features { args.extend_from_slice(&["--features", feature_name]);