app! macro take 2 #34

japaric · 2017-07-04T16:32:39Z

Update

More ports from RTFM v1 to this macro:

^ This is semantically the same example presented in #31, but with this new
macro 33% less code is required.

Next iteration of the rtfm! macro presented in #31. The most notable change wrt
the previous version is that the macro has become a ~~compiler plugin~~ procedural macro.

Pros

Automatic ceiling derivation. The ceiling will be computed from the
information about which tasks use which resources.
Simpler task and resource specification. There's no split between interrupts
used as tasks and exceptions used as tasks; there's only tasks now. Likewise
there's no distinction between peripherals used as resources and data
resources; there's only resources now.
More flexible syntax. The fields can be shuffled around without breaking the
parser.
Simpler signatures as there's no Peripherals / Resources split.
Adaptive signatures. The signature of idle will be fn() -> ! if it makes no
uses of resources and has no local data, but it will change to
fn(idle::Local) if local data is used, etc.
Less claims. Full access to resources is given to tasks whose priority matches
the resource ceiling.
More control over error messages related to the parsing and expansion of
rtfm!.

Cons

rtfm! is now a compiler plugin. This makes the chance of it breaking across
nightly updates greater than zero. However, I've implemented the compiler plugin
on top of token streams so we should be safe from compiler changes related to
the language syntax and parser. Once function style macros becomes available via
the more stable proc-macros feature it should be straightforward to move to it.
And yes, I tried proc-macro-hack which allows function style proc macros on
top of custom derive proc macros but it doesn't work for no-std / cross
compilation because the hack requires a proc-macro crate compiled for the target
(which is not possible for thumb because proc-macro contains Strings).

Other changes

I realized that the change to scope based safety fixes Interoperation with cortex_m::peripheral::Peripheral.borrow is unsound #13 so I've added a
borrow / borrow_mut method to resources to get direct access to them
within a global critical section.
Given that we pretty much generate the implementation of the claim methods in
the application crate I have changed things such that (NVIC) PRIORITY_BITS is
no longer in the cortex-m-rtfm, but rather it's expected to be provided by the
device crate. This way we don't need a bunch of P{2,3,..} Cargo features in
the cortex-m-rtfm crate. OTOH, device generated crates now must provide a
NVIC_PRIO_BITS constant that provides this information. SVD files contain
this information in the form of the optional <nvicPrioBits> field.
svd2rust will be taught to generate the NVIC_PRIO_BITS constant.

The declaration of task local data and the task function path has been moved
into a task! macro. This macro doesn't have to be in the root of the crate.

cc @whitequark

whitequark · 2017-07-04T17:04:34Z

macros/src/syntax/parse.rs

+                    pieces.push(tt);
+                }
+            } else {
+                panic!("expected type, found EOM");


Nit: maybe expand EOM to end of macro? It took me a few moments to recognize.

whitequark · 2017-07-04T17:05:06Z

macros/src/trans.rs

+                interrupts.push(quote! {
+                    let prio_bits = #device::NVIC_PRIO_BITS;
+                    let hw = ((1 << prio_bits) - #priority) << (8 - prio_bits);
+                    nvic.set_priority(#device::Interrupt::#name, hw);


Nit: could this be extracted as another function on NVIC? Seems useful.

you mean adding a set_logical_priority to NVIC or adding a logical2hw function to map a logical priority to a "hardware" value that can be directly written to an NVIC register?

whitequark · 2017-07-04T17:16:01Z

Outstanding work @japaric! You've addressed nearly every concern of mine, except:

Regarding explicit specification of resources for the init function, I still think it's somewhat pointless, but I concede that it's not worth the added implementation complexity. One could argue that there's the benefit of seeing an overview of every resource initialized.
The naming of Local. It's not really important.

japaric · 2017-07-06T00:11:12Z

@whitequark Thanks

Regarding explicit specification of resources for the init function, I still think it's somewhat pointless, but I concede that it's not worth the added implementation complexity.

I was thinking about this and it would be easy to change the implementation such that init has access to all the resources declared in other tasks. The issue I see with this approach is that init is likely going to require more resources, specially peripherals (e.g. RCC), than what the other tasks use so you would still need to list more resources in init.resources.

The problem with giving init access to all the peripherals is that there's no such list that the rtfm! macro can use. At least not right now. I was thinking of adding a Peripherals struct to device crates which contains a reference to each and every peripheral / register block and make its constructor unsafe. With that change the init signature could be changed to fn($device::Peripherals, Resources) to make it have access to all the device peripheral and all the application resources. I'll give this a try.

japaric · 2017-07-06T22:59:01Z

the last commit removes init.resources from rtfm! and makes idle.local optional. The change depends on rust-embedded/svd2rust#123

Diff of the example shown below:

--- a/0-example.rs	2017-07-06 17:54:13.908406632 -0500
+++ b/0-example.rs	2017-07-06 17:55:23.428268225 -0500
@@ -42,25 +41,23 @@

     init: {
         path: init,
-        resources: [AFIO, GPIOA, GPIOC, TIM2, RCC, USART1],
     },

     idle: {
-        local: {},
         path: idle,
         resources: [DWT, SLEEP_CYCLES],
     },
 }

-fn init(r: init::Resources) {
-    let timer = Timer(r.TIM2);
-    let serial = Serial(r.USART1);
+fn init(p: init::Peripherals, _: init::Resources) {
+    let timer = Timer(p.TIM2);
+    let serial = Serial(p.USART1);

-    led::init(r.GPIOC, r.RCC);
+    led::init(p.GPIOC, p.RCC);

-    serial.init(BAUD_RATE.invert(), r.AFIO, None, r.GPIOA, r.RCC);
+    serial.init(BAUD_RATE.invert(), p.AFIO, None, p.GPIOA, p.RCC);

-    timer.init(FREQUENCY.invert(), r.RCC);
+    timer.init(FREQUENCY.invert(), p.RCC);
     timer.resume();
 }

@@ -122,4 +119,4 @@

     let byte = serial.read().unwrap();
     serial.write(byte).unwrap();
-}
\ No newline at end of file
+}

- allow trailing commas in list of resources - make task.resources optional - add rtfm::set_pending function which can be used to force an interrupt into the pending state. This is a replacement of the old rtfm::request. rtfm::set_pending takes the Interrupt enum provided by the device crate as argument. (The old rtfm::request took a task function as argument) - the user may want to use types they imported into the root of the crate. These types are not available in e.g. `mod idle` so `idle::Resources` *can't* be defined in that module. To workaround this problem `idle::Resources` will be defined in the root, with some other name, and then be re-exported in the `idle` module. - remove the "a resource only used by one task should be local data" check. In some cases you do want a resource owned by a single task instead of local data since `init` can access resources but not a task local data.

japaric · 2017-07-07T04:46:25Z

I have ported two non trivial applications to the latest rtfm! version from cortex-m-rtfm v0.1.1. I'm quite happy with the result. The Cells and RefCells are gone; the task code is shorter, mainly because no access calls are needed with the new rtfm!; I even saw a drop in CPU usage of 0.5% (15% -> 14.5%) in my most demanding app just from eliminating the RefCells. (I also saw a fixed drop in binary size of about 480 bytes but I think this is mainly due to changes in cortex-m-rt)

whitequark · 2017-07-07T09:44:22Z

nice!

cc japaric/cortex-m-rtfm#34

japaric · 2017-07-11T02:42:26Z

I have now ported all the examples in the blue-pill crate to this macro (japaric/stm32f103xx-hal#19). The result in a reduction in LoC of about 30% in average.

In another news this has already broke due to a change in rustc:

error[E0425]: cannot find function `set_parse_sess` in module `proc_macro::__internal`

whitequark · 2017-07-11T14:28:24Z

@japaric Why are you using proc macro internals?

japaric · 2017-07-11T22:18:37Z

@whitequark I thought that functionlike! proc macros weren't implemented given the existence of proc-macro-hack and the lack of documentation about macros 2.0 so I was using a compiler plugin. But I figured out how to use #![proc_macro] so I'll switch to that.

…porting

japaric · 2017-07-15T02:17:48Z

Some changes:

the rtfm! macro has been renamed to app!
the macro parser has been refactored into its own crate. The main motivation for this is to share code between this crate and the port of the tasks and resources model to the MSP430 architecture. Yup, you can now use this very same macro to write concurrent applications on MSP430 🎉.
I have made the fields init.path and idle.path optional. If omitted these fields default to the paths init and idle respectively, which is what most people will use, I expect.
I have replaced all the panic! / unwraps in the proc macro implementation with proper error handling (error-chain ❤️). You now get nice backtraces when there's an error in the contens of the app! macro. For example:

app! {
    device: blue_pill::stm32f103xx,

    tasks: {
        SYS_TICK: {
            priority: 1,
            resources: { SYST }, // <- wrong delimiter
        },
    },
}

error: proc macro panicked
  --> examples/blinky.rs:16:1
   |
16 | / app! {
17 | |     device: blue_pill::stm32f103xx,
18 | |
19 | |     tasks: {
...  |
24 | |     },
25 | | }
   | |_^
   |
   = help: message: Error: parsing
           Caused by: parsing `tasks`
           Caused by: parsing task `SYS_TICK`
           Caused by: parsing `resources`
           Caused by: expected Bracket, found Brace

I believe this is good as the error messages can get without proper span information.

whitequark · 2017-07-15T03:00:06Z

This is amazing work.

homunkulus · 2017-07-27T20:02:42Z

⌛ Trying commit 7d0c07c with merge 7d0c07c...

homunkulus · 2017-07-27T20:07:17Z

💔 Test failed - status-travis

and move the logic that differentiates interrupts from exceptions from the crate to the procedural macro logic

japaric · 2017-07-29T02:04:00Z

@homunkulus try

japaric · 2017-07-29T05:36:21Z

@homunkulus r+

homunkulus · 2017-07-29T05:36:21Z

📌 Commit 3d2be6d has been approved by japaric

japaric · 2017-07-29T05:38:25Z

@homunkulus r+

homunkulus · 2017-07-29T05:38:25Z

📌 Commit 697c2d6 has been approved by japaric

japaric · 2017-07-29T05:46:49Z

@homunkulus r+

homunkulus · 2017-07-29T05:46:49Z

📌 Commit 2d80f36 has been approved by japaric

homunkulus · 2017-07-29T05:46:52Z

⌛ Testing commit 2d80f36 with merge 2d80f36...

homunkulus · 2017-07-29T05:50:56Z

☀️ Test successful - status-travis
Approved by: japaric
Pushing 2d80f36 to master...

34: [RFC] Mod instead of const r=korken89 a=AfoHT Link to [rendered RFC](https://github.com/AfoHT/rfcs/blob/mod_const/text/0000-use-mod-over-const.md) Implemented and ran the test suites and examples, noticed no side-effecs. https://github.com/AfoHT/rtfm-syntax/tree/mod_const https://github.com/AfoHT/cortex-m-rtfm/tree/mod_const Co-authored-by: Henrik Tjäder <henrik@tjaders.com>

This is one possible solution to the stack overflow problem described in rtic-rs#34. This approach uses a linker wrapper, called [swap-ld], to generate the desired memory layout. See rtic-rs#34 for a description of the desired memory layout and rtic-rs#41 for a description of how `swap-ld` works. The observable effects of this change in cortex-m programs are: - the `_sbss` symbol is now override-able. - there is now a `.stack` linker section that denotes the span of the call stack. `.stack` won't be loaded into the program; it just exists for informative purposes (`swap-ld` uses this information). Given the following program: ``` rust fn main() { static mut X: u32 = 0; static mut Y: u32 = 1; loop { unsafe { ptr::write_volatile(&mut X, X + 1); ptr::write_volatile(&mut Y, Y + 1); } } } ``` If you link this program using the `arm-none-eabi-ld` linker, which is the cortex-m-quickstart default, you'll get the following memory layout: ``` console $ console section size addr .vector_table 0x130 0x8000000 .text 0x94 0x8000130 .rodata 0x0 0x80001c4 .stack 0x5000 0x20000000 .bss 0x4 0x20000000 .data 0x4 0x20000004 ``` Note how the space reserved for the stack (depicted by the `.stack` linker section) overlaps with the space where .bss and .data reside. If you, instead, link this program using `swap-ld` you'll get the following memory layout: ``` console $ arm-none-eabi-size -Ax app section size addr .vector_table 0x130 0x8000000 .text 0x94 0x8000130 .rodata 0x0 0x80001c4 .stack 0x4ff8 0x20000000 .bss 0x4 0x20004ff8 .data 0x4 0x20004ffc ``` Note that no overlap exists in this case and that the call stack size has reduced to accommodate the .bss and .data sections. Unlike rtic-rs#41 the addresses of static variables is now correct: ``` console $ arm-none-eabi-objdump -CD app Disassembly of section .vector_table: 08000000 <_svector_table>: 8000000: 20004ff8 strdcs r4, [r0], -r8 ; initial Stack Pointer 08000004 <cortex_m_rt::RESET_VECTOR>: 8000004: 08000131 stmdaeq r0, {r0, r4, r5, r8} 08000008 <EXCEPTIONS>: 8000008: 080001bd stmdaeq r0, {r0, r2, r3, r4, r5, r7, r8} (..) Disassembly of section .stack: 20000000 <.stack>: ... Disassembly of section .bss: 20004ff8 <cortex_m_quickstart::main::X>: 20004ff8: 00000000 andeq r0, r0, r0 Disassembly of section .data: 20004ffc <_sdata>: 20004ffc: 00000001 andeq r0, r0, r1 ``` closes rtic-rs#34 [swap-ld]: https://github.com/japaric/swap-ld

rtfm! macro take 2

86a360a

whitequark reviewed Jul 4, 2017

View reviewed changes

rtfm!: remove init.resources and make idle.local optional

4b0c3bf

japaric mentioned this pull request Jul 7, 2017

[WIP][RFC] A single macro to declare tasks and resources and a new mechanism to access resources #31

Closed

japaric referenced this pull request in japaric/2wd Jul 8, 2017

port firmware to the rtfm! macro

987d2b8

cc japaric/cortex-m-rtfm#34

japaric mentioned this pull request Jul 8, 2017

[WIP] port firmware to the rtfm! macro japaric/2wd#4

Merged

japaric referenced this pull request in japaric/ws2812b Jul 8, 2017

[WIP] port firmware to the rtfm! macro

87b61b8

cc japaric/cortex-m-rtfm#34

japaric mentioned this pull request Jul 8, 2017

[WIP] port firmware to the rtfm! macro japaric/ws2812b#1

Merged

japaric added 4 commits July 8, 2017 21:44

rename rtfm-macros to cortex-m-rtfm-macros

17b252a

fix unused variable warning around interrupt::free

e18eb96

make the tasks and resources fields optional

b7e43c1

make the init::Resources argument optional

8485a24

japaric mentioned this pull request Jul 11, 2017

Gpio API rust-embedded/embedded-hal#11

Closed

japaric added 3 commits July 11, 2017 23:44

compiler plugin -> proc macro

59afbf0

split macro parser into its own crate and improve error handling / re…

9859655

…porting

rename rtfm! to app! and adapt to changes in rtfm-syntax

e9788ff

japaric mentioned this pull request Jul 18, 2017

RFC: Provide access to read-only registers without asserting data-race freedom #35

Closed

add cfail tests

1f1cf84

japaric added 5 commits July 27, 2017 15:15

fix yet another warning on ARMv6-M

ad2a523

make task.$T.enabled optional

d396da5

and move the logic that differentiates interrupts from exceptions from the crate to the procedural macro logic

make task.$T.path mandatory

b9f50e4

Send-ness check is now in rtfm-core

271df39

update examples

e85d6e5

japaric force-pushed the take2 branch from 3d2be6d to 697c2d6 Compare July 29, 2017 05:38

update examples

2d80f36

japaric force-pushed the take2 branch from 697c2d6 to 2d80f36 Compare July 29, 2017 05:46

homunkulus merged commit 2d80f36 into master Jul 29, 2017

japaric deleted the take2 branch July 29, 2017 05:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

app! macro take 2 #34

app! macro take 2 #34

japaric commented Jul 4, 2017 •

edited

Loading

whitequark Jul 4, 2017

japaric Jul 5, 2017

whitequark Jul 4, 2017

japaric Jul 5, 2017 •

edited

Loading

whitequark Jul 6, 2017

whitequark commented Jul 4, 2017

japaric commented Jul 6, 2017 •

edited

Loading

japaric commented Jul 6, 2017

japaric commented Jul 7, 2017

whitequark commented Jul 7, 2017

japaric commented Jul 11, 2017

whitequark commented Jul 11, 2017

japaric commented Jul 11, 2017

japaric commented Jul 15, 2017

whitequark commented Jul 15, 2017

homunkulus commented Jul 27, 2017

homunkulus commented Jul 27, 2017

japaric commented Jul 29, 2017

japaric commented Jul 29, 2017

homunkulus commented Jul 29, 2017

japaric commented Jul 29, 2017

homunkulus commented Jul 29, 2017

japaric commented Jul 29, 2017

homunkulus commented Jul 29, 2017

homunkulus commented Jul 29, 2017

homunkulus commented Jul 29, 2017

app! macro take 2 #34

app! macro take 2 #34

Conversation

japaric commented Jul 4, 2017 • edited Loading

Update

Pros

Cons

Other changes

whitequark Jul 4, 2017

Choose a reason for hiding this comment

japaric Jul 5, 2017

Choose a reason for hiding this comment

whitequark Jul 4, 2017

Choose a reason for hiding this comment

japaric Jul 5, 2017 • edited Loading

Choose a reason for hiding this comment

whitequark Jul 6, 2017

Choose a reason for hiding this comment

whitequark commented Jul 4, 2017

japaric commented Jul 6, 2017 • edited Loading

japaric commented Jul 6, 2017

japaric commented Jul 7, 2017

whitequark commented Jul 7, 2017

japaric commented Jul 11, 2017

whitequark commented Jul 11, 2017

japaric commented Jul 11, 2017

japaric commented Jul 15, 2017

whitequark commented Jul 15, 2017

homunkulus commented Jul 27, 2017

homunkulus commented Jul 27, 2017

japaric commented Jul 29, 2017

japaric commented Jul 29, 2017

homunkulus commented Jul 29, 2017

japaric commented Jul 29, 2017

homunkulus commented Jul 29, 2017

japaric commented Jul 29, 2017

homunkulus commented Jul 29, 2017

homunkulus commented Jul 29, 2017

homunkulus commented Jul 29, 2017

japaric commented Jul 4, 2017 •

edited

Loading

japaric Jul 5, 2017 •

edited

Loading

japaric commented Jul 6, 2017 •

edited

Loading