Generate atomic module outside of MSP430

[YuhanLiin]

Currently the --nightly flag generates atomic register modification code for MSP430. This PR expands this code generation to all architectures.

[rust-highfive]

r? @reitermarkus

(rust-highfive has picked a reviewer for you, use r? to override)

[burrbull]

1. It is better to rename flag to atomics (with note in docs that it requires nightly)
2. The trait is implemented for u8 and u16 only. Other architectures usually support bigger registers.

[burrbull]

We don't use signed integers as raw registers.

what about riscv? they can be with/without atomic extention or can have 64 bits

[YuhanLiin]

I didn't know we even supported 64-bit registers. I think I can detect it using target_ptr_width.

I'm not sure how to detect the atomic extension. Maybe I can mention it in the documentation and tell users not to use the atomic flag without the extension.

[adamgreig]

I left this comment on the previous PR but it's probably more useful here:

I haven't compared them in detail, but for other targets it might make more sense to use atomic-polyfill which builds on critical-section and is quite widely used in other embedded Rust projects. portable-atomic can only use a global disable-interrupts to simulate CAS operations (when enabled with --cfg portable_atomic_unsafe_assume_single_core), while atomic-polyfill uses critical-section which allows pluggable implementations (and is used in svd2rust for Peripherals::take()).

At the moment I don't think atomic-polyfill supports MSP430 at all, so it's not an option here (yet?).

Maybe it's possible to add msp430 to atomic-polyfill so one crate can be used by all architectures (@Dirbaio?).

That said, are you sure this is a valid way to operate on MMIO? At least in ARMv7-M, the spec says "LDREX and STREX operations must be performed only on memory with the Normal memory attribute", which doesn't include Device memory for MMIO. In practice on Cortex-M4 it seems like the usual implementation of the global monitor doesn't care about the memory type, so it may end up working despite the spec. The accesses also don't get marked as volatile, so their interaction with the Rust memory model is a bit harder to be sure about, and the compiler might be within its rights to completely optimise out some accesses, either now or in the future. There's some ongoing discussion in these issues.

Overall I think this probably needs a bit more thought before it's generated for other platforms, around whether it even makes sense to polyfill atomics on platforms without them (armv6-m for example) and whether it's even legitimate to just stick some AtomicU32 types onto the MMIO memory map.

[YuhanLiin]

For this PR I'm aiming to only add atomic register modification for platforms with instruction-level atomic support. This is why I disabled the fallback feature in portable-atomic. If a target requires a polyfill for atomic operations then the generate atomics code won't even compile, since atomic operations won't be supported by portable-atomic for that platform. This guarantees that the generated atomic code is low-cost.

Are the concerns about MMIO memory type still relevant in this case? I'm not familiar with ARM so I don't know. As for volatile accesses, according to this issue Rust doesn't support memory accesses with both volatile and atomic semantics, though I believe it's achievable using inline ASM with the right flags (like in msp430-atomics).

[YuhanLiin]

Ok it turns out that portable-atomics doesn't actually use ASM instructions for most MSP430 atomic operations, but instead uses the interrupt-disable method. This is only an issue for MSP430 and AVR (I don't think we support this one), so it seems like we still need msp430-atomic after all.

[burrbull]

@adamgreig what should we do with this PR? Merge or make it draft?

[YuhanLiin]

I'm gonna make it into a draft and wait for the next release of msp430-atomic

[YuhanLiin]

I modified the atomic code to match the newest version of portable-atomic, which supports single-instruction atomics on MSP430. This is now ready for review.

[burrbull]

bors r+

[bors[bot]]

Build succeeded:

• CI
• Rustfmt