-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Consider adding load/store atomics for targets that don't support compare and swap #45085
Comments
For the record I ended up re-implementing So 👍 to this for me. Though I understand that this is related to the general problem of the "cfg-ability" of the std facade. (Should std compile for a target that doesn't natively support CAS loops? etc.) |
BackgroundARMv6-M and MSP430 support (i.e. have hardware instructions for) atomic loads and stores but What we wantExpose a limited WhyAtomic loads / stores are enough to implement simple lockless data structures like a single producer single consumer queue -- one such implementation can be found in the One way to do itI propose we add a new experimental (i.e. requires nightly) cfg attribute: say This would make available and insta-stabilize the "Isn't it a problem that e.g. @rust-lang/libs thoughts on making Also cc @rust-lang/portability (er, there's no team so cc @jethrogb) I believe @dylanmckay @dvc94ch Does AVR / RISCV natively support CAS? I suspect that there's some RISCV variant that does, but is there any RISCV variant that does not? @dylanmckay @pftbest Do multicore AVR / MSP430 microcontrollers even exist? Because if they don't then we can set both |
@japaric Sounds great, but isn't As I understand it, |
Maybe? I haven't thought much about it. If that doesn't work we could add In any case, It doesn't matter much how we implement it right now as it will be unstable and
I think you are right. The wasm target allows CAS operations though. The compiler will happily compile this: static A: AtomicUsize = AtomicUsize::new(0);
#[no_mangle]
pub extern "C" fn foo() -> usize {
A.swap(1, Ordering::AcqRel)
}
#[no_mangle]
pub extern "C" fn bar() -> usize {
A.swap(2, Ordering::AcqRel)
} into this:
(no loops, AFAICT) I think that's fine on wasm because I believe interrupts / signals / preemption is not possible within the execution of a single wasm module and each wasm module's memory is isolated from the other. But, yeah, I think |
To clarify: It doesn't matter much how we implement it right now, as It's not clear to me if |
@japaric your proposal sounds great to me! I think we'd totally be down for providing stripped-down I think from a stabilization perspective we're all good here as well, it's "no worse" than the situation today and all the appropriate bits will continue to be unstable. FWIW on wasm we actually emit atomic instructions and all to LLVM, but we also tell LLVM that wasm is single threaded which causes it to legalize atomics to nonatomic operations. (aka that's why atomics work on wasm) |
No, there are no multicore MSP430. But using There is one more thing about MSP430, it can do RMW operations atomically. For example, you may have a counter that you can atomically increment by some value. But unfortunately all arithmetic operations on the current atomics are CAS, so mapping them on MSP430 would result in a function call and an interrupt disable/enable instead of a single instruction. To fix this I've made a crate msp430-atomic which is a copy of core::sync::atomic, except that every CAS operation is replaced with RMW counterpart which doesn't return the old value. I did it using inline assembly to be sure that all operations are mapped to a single instruction even in debug builds. I'm not advertising changing the core api, I'm just pointing out that there are other ways to get around the problem. The main benefit of having default atomics working is a code reuse between big and small targets, so cutting out some operations may hurt that goal. I think lowering unsupported operations to a libcalls might be a better solution. |
Since thumbv6 doesn't suppport AtomicUsize provided by rust (unless rust-lang/rust#45085 gets implemented I guess), we create our own based on https://github.com/japaric/heapless/blob/master/src/ring_buffer/mod.rs. This allows it to compile for all of our boards.
PR #51953 implements the proposal in #45085 (comment) |
closes rust-lang#45085 this commit adds an `atomic_cas` target option and an unstable `#[cfg(target_has_atomic_cas)]` attribute to enable a subset of the `Atomic*` API on architectures that don't support atomic CAS natively, like MSP430 and ARMv6-M.
…ichton enable Atomic*.{load,store} for ARMv6-M / MSP430 closes rust-lang#45085 as proposed in rust-lang#45085 (comment) this commit adds an `atomic_cas` target option and extends the `#[cfg(target_has_atomic)]` attribute to enable a subset of the `Atomic*` API on architectures that don't support atomic CAS natively, like MSP430 and ARMv6-M.
enable Atomic*.{load,store} for ARMv6-M / MSP430 closes #45085 as proposed in #45085 (comment) this commit adds an `atomic_cas` target option and extends the `#[cfg(target_has_atomic)]` attribute to enable a subset of the `Atomic*` API on architectures that don't support atomic CAS natively, like MSP430 and ARMv6-M. r? @alexcrichton
enable Atomic*.{load,store} for ARMv6-M / MSP430 closes #45085 as proposed in #45085 (comment) this commit adds an `atomic_cas` target option and extends the `#[cfg(target_has_atomic)]` attribute to enable a subset of the `Atomic*` API on architectures that don't support atomic CAS natively, like MSP430 and ARMv6-M. r? @alexcrichton
Some targets, Armv6-M for example, don't support atomic compare and swap so have "max-atomic-width" set to 0 in their target specs. Often you just need to
AtomicUsize::load
andAtomicUsize::store
and none of the other CAS dependent methods but unfortunately it's not supported by rust even if that targets natively support it.We could add another field to the target spec, "max-atomic-load-store-width", that defaults to the same value as "max-atomic-width", for determining whether to enable
Atomic*
without the CAS dependent methods.cc @japaric who probably would like this for https://github.com/japaric/spscrb
The text was updated successfully, but these errors were encountered: