-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fiber] Add more concurrency classes #1026
[fiber] Add more concurrency classes #1026
Conversation
97c4cb4
to
d9e13d0
Compare
3526783
to
3f372b6
Compare
It's still missing unit tests for latch, barrier and condition_variable and a bunch of documentation, however, it would be amazing if this could get reviewed with a special focus on C++ "conformity". I also noticed that avrlibstd++ does not have |
3f372b6
to
b662dc3
Compare
I haven't tried to be honest but I expect that the same solution as for Cortex-M0 will work. The header is mostly implemented using compiler builtins. I'd expect it to fall back to calling a library function in case the operation is not atomic on AVR. If you are lucky just dropping in the headers and compiling C++20 atomic wait which requires OS support shouldn't be an issue because it is only enabled when |
I'll take a look at the PR over the weekend. |
c8575fb
to
377553c
Compare
Yup that just works, thanks! |
0d3fa53
to
6803ce0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
I haven't managed to look at all the synchronization primitives, but will do tomorrow.
d9b837a
to
72ab66d
Compare
ae6328c
to
407e1a6
Compare
@chris-durand, I added ARMv8-M stack protection, could you check if the example/generic/fiber still works? I don't have a Cortex-M33 in my collection. |
c067a44
to
1dea124
Compare
When running this on an STM32L5 I get a hardfault. The stack overflow bit (STKOF, bit 4) in UFSR is set. The fault happens inside |
Let me know if there is something specific I can check. |
Hm, so setting MSPLIM is no problem, setting PSPLIM once on context_start is no problem either, but then on the first context switch it fails? ugh. Could you try to change the assembly in context_jump? %% elif with_psplim
- "ldm r1, {r1-r2} \n\t"
- "mov sp, r1 \n\t" // Set PSP to ctx->sp
- "msr psplim, r2 \n\t" // Set PSPLIM to ctx->bottom
+ "ldm r1, {r2-r3} \n\t"
+ "mov sp, r2 \n\t" // Set PSP to ctx->sp
+ "msr psplim, r3 \n\t" // Set PSPLIM to ctx->bottom
%% else %% elif with_psplim
- "ldm r1, {r1-r2} \n\t"
- "mov sp, r1 \n\t" // Set PSP to ctx->sp
- "msr psplim, r2 \n\t" // Set PSPLIM to ctx->bottom
+ "ldr sp, [r1] \n\t" // Set PSP to ctx->sp
+ "ldr r2, [r1, #4] \n\t"
+ "msr psplim, r2 \n\t" // Set PSPLIM to ctx->bottom
%% else %% elif with_psplim
- "ldm r1, {r1-r2} \n\t"
- "mov sp, r1 \n\t" // Set PSP to ctx->sp
- "msr psplim, r2 \n\t" // Set PSPLIM to ctx->bottom
+ "ldm r1, {r2-r3} \n\t"
+ "msr psplim, r3 \n\t" // Set PSPLIM to ctx->bottom
+ "mov sp, r2 \n\t" // Set PSP to ctx->sp
%% else %% elif with_psplim
- "ldm r1, {r1-r2} \n\t"
- "mov sp, r1 \n\t" // Set PSP to ctx->sp
- "msr psplim, r2 \n\t" // Set PSPLIM to ctx->bottom
+ "ldr r2, [r1, #4] \n\t"
+ "msr psplim, r2 \n\t" // Set PSPLIM to ctx->bottom
+ "ldr sp, [r1] \n\t" // Set PSP to ctx->sp
%% else |
No success with any of the options. I've also tried replacing the code in both context_start and context_jump. |
Ok, then I'll descope it for now and will order a bunch of new development boards and debug it some more. Thanks for your help! |
13e0760
to
460bcee
Compare
- modm::fiber -> modm::this_fiber. - modm::fiber::sleep() -> modm::this_fiber::sleep_for().
460bcee
to
39a9f4d
Compare
Fibers need a standard set of efficient constructs.
I modelled these classes after the C++ thread concurrency interfaces. Not sure if useful, but also probably not really much room for improvement anyways.
modm::fiber::sleep
->modm::this_fiber::sleep_for
modm::this_fiber::sleep_until
modm::this_fiber::get_id
modm::fiber::Task
tostd::jthread
interface, incl. stop_token.