bytes icon indicating copy to clipboard operation
bytes copied to clipboard

Support `atomic-polyfill` for targets without native atomics

Open iostat opened this issue 1 year ago • 13 comments

Hi!

It's awesome that this crate is no-std (it's actually being used in some firmware!) -- but we've found that when compiling for architectures without native atomics support such as thumbv6m-none-eabi (i.e., Cortex-M0 such as in the Atmel ATSAMD21 and Raspberry Pi RP2040), well, we can't compile...

While the core::sync::atomic modules/types exist in those targets, they don't actually support compare_exchange/fetch_add/fetch_sub as are used by this crate.

The awesome folks working on Embassy actually have an atomic-polyfill crate for this exact scenario, this PR adds a feature gate to use it, allowing one to build this crate for even more no-std targets.

iostat avatar Oct 03 '22 21:10 iostat

There is also a portable-atomic crate that implements atomic operations without using an external critical-section at all. IIRC either approach would work.

notgull avatar Oct 03 '22 21:10 notgull

See https://github.com/tokio-rs/bytes/issues/461 for previous discussions.

taiki-e avatar Oct 03 '22 21:10 taiki-e

Don't know how I missed that discussion! I see #467 uses portable-atomics -- honestly I see it as preferable to this PR since it seems to have more extensive test coverage too. Is there anything really blocking #467 from being merged?

iostat avatar Oct 03 '22 21:10 iostat

Not sure if there are any blockers, but that PR is marked as a draft, so I haven't reviewed it.

Darksonn avatar Oct 04 '22 10:10 Darksonn

@taiki-e is there any way I can help make #467 for review?

iostat avatar Oct 04 '22 10:10 iostat

Is there anything really blocking https://github.com/tokio-rs/bytes/pull/467 from being merged?

The situation has changed since that PR was opened and I'm wondering about how to update it.

The main issue is how targets without CAS are handled with default features.

  • Now that cfg_target_has_atomic is stable, cfg_attr(target_os = "none", cfg(target_has_atomic = "ptr")) can be used to remove the need of the build script in #467. However, it increases MSRV for target_os = "none" targets.
  • If we do not use cfg_target_has_atomic, I need to implement CI-related logic as mentioned in https://github.com/tokio-rs/bytes/pull/467#issuecomment-1213234032.
  • Alternatively, let the processing of those targets to an external crate such as portable-atomic, avoiding the implementation of our own logic such as #467.

Perhaps it would be preferable to start with the third and then select the first or second in the future if necessary?

taiki-e avatar Oct 07 '22 13:10 taiki-e