From e8eca4be37a2fe1af25b203ace5e99b31fcc3972 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Thu, 22 Oct 2020 21:36:32 +0200 Subject: [PATCH] Now all locks are symmetric Test fixes Fix test Fix comment --- .github/workflows/build.yml | 1 - examples/big-struct-opt.rs | 48 +++++++++ examples/cfg-whole-task.rs | 12 +-- examples/cfg.rs | 6 +- examples/destructure.rs | 4 +- examples/generics.rs | 8 +- examples/late.rs | 8 +- examples/lock.rs | 12 ++- examples/resource-user-struct.rs | 17 ++-- examples/resource.rs | 17 ++-- examples/shared-with-init.rs | 45 --------- examples/static.rs | 8 +- examples/t-late-not-send.rs | 7 +- examples/t-resource.rs | 27 ++--- examples/task-local.rs | 16 +-- examples/types.rs | 2 +- macros/src/codegen/hardware_tasks.rs | 9 +- macros/src/codegen/idle.rs | 3 +- macros/src/codegen/init.rs | 3 +- macros/src/codegen/resources.rs | 12 ++- macros/src/codegen/resources_struct.rs | 132 +++++++++---------------- macros/src/codegen/software_tasks.rs | 9 +- 22 files changed, 183 insertions(+), 223 deletions(-) create mode 100644 examples/big-struct-opt.rs delete mode 100644 examples/shared-with-init.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3581bb4884..739c7bd9e2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -252,7 +252,6 @@ jobs: types not-sync - shared-with-init generics cfg diff --git a/examples/big-struct-opt.rs b/examples/big-struct-opt.rs new file mode 100644 index 0000000000..85ec5e6144 --- /dev/null +++ b/examples/big-struct-opt.rs @@ -0,0 +1,48 @@ +//! examples/big-struct-opt.rs +//! +//! Example on how to initialize a large struct without needing to copy it via `LateResources`, +//! effectively saving stack space needed for the copies. + +#![no_main] +#![no_std] + +use panic_halt as _; + +/// Some big struct +pub struct BigStruct { + /// Big content + pub data: [u8; 2048], +} + +impl BigStruct { + fn new() -> Self { + BigStruct { data: [22; 2048] } + } +} + +#[rtic::app(device = lm3s6965)] +mod app { + use super::BigStruct; + use core::mem::MaybeUninit; + + #[resources] + struct Resources { + big_struct: &'static mut BigStruct, + } + + #[init] + fn init(_: init::Context) -> init::LateResources { + let big_struct = unsafe { + static mut BIG_STRUCT: MaybeUninit = MaybeUninit::uninit(); + + // write directly into the static storage + BIG_STRUCT.as_mut_ptr().write(BigStruct::new()); + &mut *BIG_STRUCT.as_mut_ptr() + }; + + init::LateResources { + // assign the reference so we can use the resource + big_struct, + } + } +} diff --git a/examples/cfg-whole-task.rs b/examples/cfg-whole-task.rs index 7a7fc48be4..e225da3312 100644 --- a/examples/cfg-whole-task.rs +++ b/examples/cfg-whole-task.rs @@ -41,12 +41,12 @@ mod app { } #[task(capacity = 2, resources = [count])] - fn foo(_cx: foo::Context) { + fn foo(mut _cx: foo::Context) { #[cfg(debug_assertions)] { - *_cx.resources.count += 1; + _cx.resources.count.lock(|count| *count += 1); - log::spawn(*_cx.resources.count).unwrap(); + log::spawn(_cx.resources.count.lock(|count| *count)).unwrap(); } // this wouldn't compile in `release` mode @@ -59,12 +59,12 @@ mod app { // currently still present in the Tasks enum #[cfg(never)] #[task(capacity = 2, resources = [count])] - fn foo2(_cx: foo2::Context) { + fn foo2(mut _cx: foo2::Context) { #[cfg(debug_assertions)] { - *_cx.resources.count += 10; + _cx.resources.count.lock(|count| *count += 10); - log::spawn(*_cx.resources.count).unwrap(); + log::spawn(_cx.resources.count.lock(|count| *count)).unwrap(); } // this wouldn't compile in `release` mode diff --git a/examples/cfg.rs b/examples/cfg.rs index f900286b71..fae13e5bcf 100644 --- a/examples/cfg.rs +++ b/examples/cfg.rs @@ -38,12 +38,12 @@ mod app { } #[task(capacity = 2, resources = [count])] - fn foo(_cx: foo::Context) { + fn foo(mut _cx: foo::Context) { #[cfg(debug_assertions)] { - *_cx.resources.count += 1; + _cx.resources.count.lock(|count| *count += 1); - log::spawn(*_cx.resources.count).unwrap(); + log::spawn(_cx.resources.count.lock(|count| *count)).unwrap(); } // this wouldn't compile in `release` mode diff --git a/examples/destructure.rs b/examples/destructure.rs index 3c5eabf7c0..d843978b4f 100644 --- a/examples/destructure.rs +++ b/examples/destructure.rs @@ -32,7 +32,7 @@ mod app { } // Direct destructure - #[task(binds = UART0, resources = [a, b, c])] + #[task(binds = UART0, resources = [&a, &b, &c])] fn uart0(cx: uart0::Context) { let a = cx.resources.a; let b = cx.resources.b; @@ -42,7 +42,7 @@ mod app { } // De-structure-ing syntax - #[task(binds = UART1, resources = [a, b, c])] + #[task(binds = UART1, resources = [&a, &b, &c])] fn uart1(cx: uart1::Context) { let uart1::Resources { a, b, c } = cx.resources; diff --git a/examples/generics.rs b/examples/generics.rs index 16327fb3db..f3829a0634 100644 --- a/examples/generics.rs +++ b/examples/generics.rs @@ -13,7 +13,6 @@ use rtic::Mutex; mod app { use cortex_m_semihosting::{debug, hprintln}; use lm3s6965::Interrupt; - use rtic::Exclusive; #[resources] struct Resources { @@ -49,11 +48,8 @@ mod app { hprintln!("UART1(STATE = {})", *STATE).unwrap(); - // just to show that `shared` can be accessed directly - *c.resources.shared += 0; - - // second argument has type `Exclusive` - super::advance(STATE, Exclusive(c.resources.shared)); + // second argument has type `resources::shared` + super::advance(STATE, c.resources.shared); } } diff --git a/examples/late.rs b/examples/late.rs index d20a69c553..d4efaba367 100644 --- a/examples/late.rs +++ b/examples/late.rs @@ -35,9 +35,9 @@ mod app { } #[idle(resources = [c])] - fn idle(c: idle::Context) -> ! { + fn idle(mut c: idle::Context) -> ! { loop { - if let Some(byte) = c.resources.c.dequeue() { + if let Some(byte) = c.resources.c.lock(|c| c.dequeue()) { hprintln!("received message: {}", byte).unwrap(); debug::exit(debug::EXIT_SUCCESS); @@ -48,7 +48,7 @@ mod app { } #[task(binds = UART0, resources = [p])] - fn uart0(c: uart0::Context) { - c.resources.p.enqueue(42).unwrap(); + fn uart0(mut c: uart0::Context) { + c.resources.p.lock(|p| p.enqueue(42).unwrap()); } } diff --git a/examples/lock.rs b/examples/lock.rs index c4930a28c9..2fbf760833 100644 --- a/examples/lock.rs +++ b/examples/lock.rs @@ -52,11 +52,15 @@ mod app { } #[task(binds = GPIOB, priority = 2, resources = [shared])] - fn gpiob(c: gpiob::Context) { - // the higher priority task does *not* need a critical section - *c.resources.shared += 1; + fn gpiob(mut c: gpiob::Context) { + // the higher priority task does still need a critical section + let shared = c.resources.shared.lock(|shared| { + *shared += 1; - hprintln!("D - shared = {}", *c.resources.shared).unwrap(); + *shared + }); + + hprintln!("D - shared = {}", shared).unwrap(); } #[task(binds = GPIOC, priority = 3)] diff --git a/examples/resource-user-struct.rs b/examples/resource-user-struct.rs index ca4ca2af0f..a550bb2a38 100644 --- a/examples/resource-user-struct.rs +++ b/examples/resource-user-struct.rs @@ -47,18 +47,23 @@ mod app { // `shared` can be accessed from this context #[task(binds = UART0, resources = [shared])] - fn uart0(cx: uart0::Context) { - let shared: &mut u32 = cx.resources.shared; - *shared += 1; + fn uart0(mut cx: uart0::Context) { + let shared = cx.resources.shared.lock(|shared| { + *shared += 1; + *shared + }); hprintln!("UART0: shared = {}", shared).unwrap(); } // `shared` can be accessed from this context #[task(binds = UART1, resources = [shared])] - fn uart1(cx: uart1::Context) { - *cx.resources.shared += 1; + fn uart1(mut cx: uart1::Context) { + let shared = cx.resources.shared.lock(|shared| { + *shared += 1; + *shared + }); - hprintln!("UART1: shared = {}", cx.resources.shared).unwrap(); + hprintln!("UART1: shared = {}", shared).unwrap(); } } diff --git a/examples/resource.rs b/examples/resource.rs index 87ba336705..d86d46fbfc 100644 --- a/examples/resource.rs +++ b/examples/resource.rs @@ -42,18 +42,23 @@ mod app { // `shared` can be accessed from this context #[task(binds = UART0, resources = [shared])] - fn uart0(cx: uart0::Context) { - let shared: &mut u32 = cx.resources.shared; - *shared += 1; + fn uart0(mut cx: uart0::Context) { + let shared = cx.resources.shared.lock(|shared| { + *shared += 1; + *shared + }); hprintln!("UART0: shared = {}", shared).unwrap(); } // `shared` can be accessed from this context #[task(binds = UART1, resources = [shared])] - fn uart1(cx: uart1::Context) { - *cx.resources.shared += 1; + fn uart1(mut cx: uart1::Context) { + let shared = cx.resources.shared.lock(|shared| { + *shared += 1; + *shared + }); - hprintln!("UART1: shared = {}", cx.resources.shared).unwrap(); + hprintln!("UART1: shared = {}", shared).unwrap(); } } diff --git a/examples/shared-with-init.rs b/examples/shared-with-init.rs deleted file mode 100644 index ec0558862a..0000000000 --- a/examples/shared-with-init.rs +++ /dev/null @@ -1,45 +0,0 @@ -//! `examples/shared-with-init.rs` - -#![deny(unsafe_code)] -#![deny(warnings)] -#![no_main] -#![no_std] - -use panic_halt as _; -use rtic::app; - -pub struct MustBeSend; - -#[app(device = lm3s6965)] -mod app { - use super::MustBeSend; - use cortex_m_semihosting::debug; - use lm3s6965::Interrupt; - - #[resources] - struct Resources { - #[init(None)] - shared: Option, - } - - #[init(resources = [shared])] - fn init(c: init::Context) -> init::LateResources { - // this `message` will be sent to task `UART0` - let message = MustBeSend; - *c.resources.shared = Some(message); - - rtic::pend(Interrupt::UART0); - - init::LateResources {} - } - - #[task(binds = UART0, resources = [shared])] - fn uart0(c: uart0::Context) { - if let Some(message) = c.resources.shared.take() { - // `message` has been received - drop(message); - - debug::exit(debug::EXIT_SUCCESS); - } - } -} diff --git a/examples/static.rs b/examples/static.rs index cd46145a4a..7626c71276 100644 --- a/examples/static.rs +++ b/examples/static.rs @@ -36,9 +36,9 @@ mod app { } #[idle(resources = [c])] - fn idle(c: idle::Context) -> ! { + fn idle(mut c: idle::Context) -> ! { loop { - if let Some(byte) = c.resources.c.dequeue() { + if let Some(byte) = c.resources.c.lock(|c| c.dequeue()) { hprintln!("received message: {}", byte).unwrap(); debug::exit(debug::EXIT_SUCCESS); @@ -49,9 +49,9 @@ mod app { } #[task(binds = UART0, resources = [p])] - fn uart0(c: uart0::Context) { + fn uart0(mut c: uart0::Context) { static mut KALLE: u32 = 0; *KALLE += 1; - c.resources.p.enqueue(42).unwrap(); + c.resources.p.lock(|p| p.enqueue(42).unwrap()); } } diff --git a/examples/t-late-not-send.rs b/examples/t-late-not-send.rs index 8b7b986ffa..ce3bcbac68 100644 --- a/examples/t-late-not-send.rs +++ b/examples/t-late-not-send.rs @@ -23,11 +23,8 @@ mod app { y: Option, } - #[init(resources = [y])] - fn init(c: init::Context) -> init::LateResources { - // equivalent to late resource initialization - *c.resources.y = Some(NotSend { _0: PhantomData }); - + #[init] + fn init(_: init::Context) -> init::LateResources { init::LateResources { x: NotSend { _0: PhantomData }, } diff --git a/examples/t-resource.rs b/examples/t-resource.rs index 91950d3e00..0a9f3bad5b 100644 --- a/examples/t-resource.rs +++ b/examples/t-resource.rs @@ -31,29 +31,18 @@ mod app { s3: u32, // idle & uart0 } - #[init(resources = [o1, o4, o5, o6, s3])] - fn init(c: init::Context) -> init::LateResources { - // owned by `init` == `&'static mut` - let _: &'static mut u32 = c.resources.o1; - - // owned by `init` == `&'static` if read-only - let _: &'static u32 = c.resources.o6; - - // `init` has exclusive access to all resources - let _: &mut u32 = c.resources.o4; - let _: &mut u32 = c.resources.o5; - let _: &mut u32 = c.resources.s3; - + #[init] + fn init(_: init::Context) -> init::LateResources { init::LateResources {} } #[idle(resources = [o2, &o4, s1, &s3])] fn idle(mut c: idle::Context) -> ! { // owned by `idle` == `&'static mut` - let _: &'static mut u32 = c.resources.o2; + let _: resources::o2 = c.resources.o2; // owned by `idle` == `&'static` if read-only - let _: &'static u32 = c.resources.o4; + let _: &u32 = c.resources.o4; // shared with `idle` == `Mutex` c.resources.s1.lock(|_| {}); @@ -69,13 +58,13 @@ mod app { #[task(binds = UART0, resources = [o3, s1, s2, &s3])] fn uart0(c: uart0::Context) { // owned by interrupt == `&mut` - let _: &mut u32 = c.resources.o3; + let _: resources::o3 = c.resources.o3; // no `Mutex` proxy when access from highest priority task - let _: &mut u32 = c.resources.s1; + let _: resources::s1 = c.resources.s1; // no `Mutex` proxy when co-owned by cooperative (same priority) tasks - let _: &mut u32 = c.resources.s2; + let _: resources::s2 = c.resources.s2; // `&` if read-only let _: &u32 = c.resources.s3; @@ -87,6 +76,6 @@ mod app { let _: &u32 = c.resources.o5; // no `Mutex` proxy when co-owned by cooperative (same priority) tasks - let _: &mut u32 = c.resources.s2; + let _: resources::s2 = c.resources.s2; } } diff --git a/examples/task-local.rs b/examples/task-local.rs index 462a0d313d..e86197a0cb 100644 --- a/examples/task-local.rs +++ b/examples/task-local.rs @@ -61,9 +61,11 @@ mod app { // l2 ok (task_local) // e1 ok (lock_free) #[task(priority = 1, binds = UART0, resources = [shared, l2, e1])] - fn uart0(cx: uart0::Context) { - let shared: &mut u32 = cx.resources.shared; - *shared += 1; + fn uart0(mut cx: uart0::Context) { + let shared = cx.resources.shared.lock(|shared| { + *shared += 1; + *shared + }); *cx.resources.e1 += 10; hprintln!("UART0: shared = {}", shared).unwrap(); hprintln!("UART0:l2 = {}", cx.resources.l2).unwrap(); @@ -73,9 +75,11 @@ mod app { // `shared` can be accessed from this context // e1 ok (lock_free) #[task(priority = 1, binds = UART1, resources = [shared, e1])] - fn uart1(cx: uart1::Context) { - let shared: &mut u32 = cx.resources.shared; - *shared += 1; + fn uart1(mut cx: uart1::Context) { + let shared = cx.resources.shared.lock(|shared| { + *shared += 1; + *shared + }); hprintln!("UART1: shared = {}", shared).unwrap(); hprintln!("UART1:e1 = {}", cx.resources.e1).unwrap(); diff --git a/examples/types.rs b/examples/types.rs index 815d309afb..cb93a36f58 100644 --- a/examples/types.rs +++ b/examples/types.rs @@ -45,7 +45,7 @@ mod app { #[task(priority = 2, resources = [shared])] fn foo(cx: foo::Context) { let _: cyccnt::Instant = cx.scheduled; - let _: &mut u32 = cx.resources.shared; + let _: resources::shared = cx.resources.shared; let _: foo::Resources = cx.resources; } diff --git a/macros/src/codegen/hardware_tasks.rs b/macros/src/codegen/hardware_tasks.rs index e6fa5ed117..bc2e120310 100644 --- a/macros/src/codegen/hardware_tasks.rs +++ b/macros/src/codegen/hardware_tasks.rs @@ -70,13 +70,8 @@ pub fn codegen( // `${task}Resources` if !task.args.resources.is_empty() { - let (item, constructor) = resources_struct::codegen( - Context::HardwareTask(name), - priority, - &mut needs_lt, - app, - analysis, - ); + let (item, constructor) = + resources_struct::codegen(Context::HardwareTask(name), &mut needs_lt, app); root.push(item); diff --git a/macros/src/codegen/idle.rs b/macros/src/codegen/idle.rs index 36c69471b5..c8c8955df8 100644 --- a/macros/src/codegen/idle.rs +++ b/macros/src/codegen/idle.rs @@ -37,8 +37,7 @@ pub fn codegen( let name = &idle.name; if !idle.args.resources.is_empty() { - let (item, constructor) = - resources_struct::codegen(Context::Idle, 0, &mut needs_lt, app, analysis); + let (item, constructor) = resources_struct::codegen(Context::Idle, &mut needs_lt, app); root_idle.push(item); mod_app = Some(constructor); diff --git a/macros/src/codegen/init.rs b/macros/src/codegen/init.rs index 0bb9b987c1..6376ce31c4 100644 --- a/macros/src/codegen/init.rs +++ b/macros/src/codegen/init.rs @@ -82,8 +82,7 @@ pub fn codegen(app: &App, analysis: &Analysis, extra: &Extra) -> CodegenResult { let mut mod_app = None; if !init.args.resources.is_empty() { - let (item, constructor) = - resources_struct::codegen(Context::Init, 0, &mut needs_lt, app, analysis); + let (item, constructor) = resources_struct::codegen(Context::Init, &mut needs_lt, app); root_init.push(item); mod_app = Some(constructor); diff --git a/macros/src/codegen/resources.rs b/macros/src/codegen/resources.rs index 0db4f728e2..76871e598a 100644 --- a/macros/src/codegen/resources.rs +++ b/macros/src/codegen/resources.rs @@ -49,7 +49,8 @@ pub fn codegen( )); } - if let Some(Ownership::Contended { ceiling }) = analysis.ownerships.get(name) { + let r_prop = &res.properties; + if !r_prop.task_local && !r_prop.lock_free { mod_resources.push(quote!( #[allow(non_camel_case_types)] #(#cfgs)* @@ -83,13 +84,20 @@ pub fn codegen( ) }; + let ceiling = match analysis.ownerships.get(name) { + Some(Ownership::Owned { priority }) => *priority, + Some(Ownership::CoOwned { priority }) => *priority, + Some(Ownership::Contended { ceiling }) => *ceiling, + None => 0, + }; + mod_app.push(util::impl_mutex( extra, cfgs, true, name, quote!(#ty), - *ceiling, + ceiling, ptr, )); } diff --git a/macros/src/codegen/resources_struct.rs b/macros/src/codegen/resources_struct.rs index ffc727573d..bffe9431a5 100644 --- a/macros/src/codegen/resources_struct.rs +++ b/macros/src/codegen/resources_struct.rs @@ -2,15 +2,9 @@ use proc_macro2::TokenStream as TokenStream2; use quote::quote; use rtic_syntax::{ast::App, Context}; -use crate::{analyze::Analysis, codegen::util}; +use crate::codegen::util; -pub fn codegen( - ctxt: Context, - priority: u8, - needs_lt: &mut bool, - app: &App, - analysis: &Analysis, -) -> (TokenStream2, TokenStream2) { +pub fn codegen(ctxt: Context, needs_lt: &mut bool, app: &App) -> (TokenStream2, TokenStream2) { let mut lt = None; let resources = match ctxt { @@ -30,6 +24,7 @@ pub fn codegen( let cfgs = &res.cfgs; has_cfgs |= !cfgs.is_empty(); + // access hold if the resource is [x] (exclusive) or [&x] (shared) let mut_ = if access.is_exclusive() { Some(quote!(mut)) } else { @@ -38,99 +33,66 @@ pub fn codegen( let ty = &res.ty; let mangled_name = util::mangle_ident(&name); - if ctxt.is_init() { - if !analysis.ownerships.contains_key(name) { - // Owned by `init` - fields.push(quote!( - #(#cfgs)* - pub #name: &'static #mut_ #ty - )); + // let ownership = &analysis.ownerships[name]; + let r_prop = &res.properties; - values.push(quote!( - #(#cfgs)* - #name: &#mut_ #mangled_name - )); - } else { - // Owned by someone else + if !r_prop.task_local && !r_prop.lock_free { + if access.is_shared() { lt = Some(quote!('a)); fields.push(quote!( #(#cfgs)* - pub #name: &'a mut #ty + pub #name: &'a #ty + )); + } else { + // Resource proxy + lt = Some(quote!('a)); + + fields.push(quote!( + #(#cfgs)* + pub #name: resources::#name<'a> )); values.push(quote!( #(#cfgs)* - #name: &mut #mangled_name + #name: resources::#name::new(priority) + )); + + // continue as the value has been filled, + continue; } } else { - let ownership = &analysis.ownerships[name]; - - if ownership.needs_lock(priority) { - if mut_.is_none() { - lt = Some(quote!('a)); - - fields.push(quote!( - #(#cfgs)* - pub #name: &'a #ty - )); - } else { - // Resource proxy - lt = Some(quote!('a)); - - fields.push(quote!( - #(#cfgs)* - pub #name: resources::#name<'a> - )); - - values.push(quote!( - #(#cfgs)* - #name: resources::#name::new(priority) - - )); - - continue; - } + let lt = if ctxt.runs_once() { + quote!('static) } else { - let lt = if ctxt.runs_once() { - quote!('static) - } else { - lt = Some(quote!('a)); - quote!('a) - }; + lt = Some(quote!('a)); + quote!('a) + }; - if ownership.is_owned() || mut_.is_none() { - fields.push(quote!( - #(#cfgs)* - pub #name: &#lt #mut_ #ty - )); - } else { - fields.push(quote!( - #(#cfgs)* - pub #name: &#lt mut #ty - )); - } - } + fields.push(quote!( + #(#cfgs)* + pub #name: &#lt #mut_ #ty + )); + } - let is_late = expr.is_none(); - if is_late { - let expr = if mut_.is_some() { - quote!(&mut *#mangled_name.as_mut_ptr()) - } else { - quote!(&*#mangled_name.as_ptr()) - }; - - values.push(quote!( - #(#cfgs)* - #name: #expr - )); + let is_late = expr.is_none(); + if is_late { + let expr = if access.is_exclusive() { + quote!(&mut *#mangled_name.as_mut_ptr()) } else { - values.push(quote!( - #(#cfgs)* - #name: &#mut_ #mangled_name - )); - } + quote!(&*#mangled_name.as_ptr()) + }; + + values.push(quote!( + #(#cfgs)* + #name: #expr + )); + } else { + values.push(quote!( + #(#cfgs)* + #name: &#mut_ #mangled_name + )); } } diff --git a/macros/src/codegen/software_tasks.rs b/macros/src/codegen/software_tasks.rs index 18cecdf2f7..7cccd8c4cc 100644 --- a/macros/src/codegen/software_tasks.rs +++ b/macros/src/codegen/software_tasks.rs @@ -82,13 +82,8 @@ pub fn codegen( // `${task}Resources` let mut needs_lt = false; if !task.args.resources.is_empty() { - let (item, constructor) = resources_struct::codegen( - Context::SoftwareTask(name), - task.args.priority, - &mut needs_lt, - app, - analysis, - ); + let (item, constructor) = + resources_struct::codegen(Context::SoftwareTask(name), &mut needs_lt, app); root.push(item);