Skip to content

executor: gate AtomicPtr on target_has_atomic, not platform-avr - #7017

Open
SimonIT wants to merge 1 commit into
embassy-rs:mainfrom
SimonIT:executor-atomicptr-target-has-atomic
Open

SimonIT wants to merge 1 commit into
embassy-rs:mainfrom
SimonIT:executor-atomicptr-target-has-atomic

Conversation

@SimonIT

@SimonIT SimonIT commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

TaskHeader::executor/all_tasks_next used core::sync::atomic::AtomicPtr unconditionally, falling back to portable_atomic::AtomicPtr only under feature = "platform-avr". AVR isn't the only target with no native pointer-wide atomics (max-atomic-width: 0), so any other such target using this field hit a hard compile error instead of falling back.

Gate both cfg branches on target_has_atomic = "ptr" instead, matching run_queue/state's existing pattern in the same file, and make portable-atomic a plain (non-optional) dependency since it's now needed outside of platform-avr too.

@Dirbaio

Dirbaio commented Sep 16, 2026

Copy link
Copy Markdown
Member

We only use load/store on this AtomicPtr. target_has_atomic is false for targets that support atomic load/store but not CAS. What we want here is target_has_atomic_load_store which is unstable. rust-lang/rust#94039

thumbv6m is one such target that can do atomic load/store but not CAS. Merging this would hurt perf on that one.

@Dirbaio

Dirbaio commented Sep 16, 2026

Copy link
Copy Markdown
Member

what target are you running into this problem with? can we just add it?

also, never enable critical-section on portable-atomic from a lib. There's other ways to polyfill like unsafe-assume-single-core, the choice should always be up to the end user.

@SimonIT

SimonIT commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

I'm trying to run Embassy on an MC1322x, which appears to lack any atomics and is pretty old. It works with this fix

@SimonIT
SimonIT force-pushed the executor-atomicptr-target-has-atomic branch from 2f77c7f to d207a8b Compare September 17, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants