Skip to content

Commit

Permalink
Make armv6m-atomic-hack less annoying (#1635)
Browse files Browse the repository at this point in the history
Currently, the `armv6m-atomic-hack` crate is, to use a technical term,
*annoying*. Its extension traits only exist when `#[cfg(armv6m)]` is
set. This means that, in downstream crates that wish to _use_
`armv6m-atomic-hack` to support v6m targets, the extension traits may
only be imported when `#[cfg(armv6m)]` is set. Because `RUSTFLAGS` cfgs
don't propagate across dependency boundaries, this means that _each_
dependent of `armv6m-atomic-hack` must add a build script with a call to
`build_utils::expose_m_profile()` so that the cfg is set properly when
building for a v6m board. This is, as previously stated, annoying ---
especially when using crates like `counters`, whose `#[derive(Count)]`
attribute expands to a use of `AtomicU32Ext`, and thus requires any
crate that derives `Count` to add a build script.

This branch changes `armv6m-atomic-hack` so that the extension traits
`AtomicU32Ext` and `AtomicBoolExt` are always defined, regardless of
whether `#[cfg(armv6m)]` is set. Now, only the *implementation* of those
traits differs based on the presence of the `armv6m` cfg --- if it is
not set, an implementation is provided that simply forwards to the
corresponding methods on the `AtomicU32` and `AtomicBool` types in
libcore. This way, dependents of `armv6m-atomic-hack` can just import
the extension traits without needing to add a build script with
`expose_m_profile()`; now, only `armv6m-atomic-hack` itself needs the
build script. Now, the `armv6m-atomic-hack` crate can no longer be
considered annoying.
  • Loading branch information
hawkw authored Feb 29, 2024
1 parent 08c2fd6 commit 2f23c58
Show file tree
Hide file tree
Showing 14 changed files with 37 additions and 46 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions drv/i2c-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,3 @@ enum-kinds.workspace = true

derive-idol-err.path = "../../lib/derive-idol-err"
counters.path = "../../lib/counters"

[build-dependencies]
build-util.path = "../../build/util"
9 changes: 0 additions & 9 deletions drv/i2c-types/build.rs

This file was deleted.

29 changes: 26 additions & 3 deletions lib/armv6m-atomic-hack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@
#![no_std]

#[cfg(armv6m)]
use core::sync::atomic::{AtomicBool, AtomicU32, Ordering};

#[cfg(armv6m)]
pub trait AtomicU32Ext {
fn swap(&self, val: u32, order: Ordering) -> u32;
fn fetch_add(&self, val: u32, order: Ordering) -> u32;
Expand Down Expand Up @@ -66,7 +64,24 @@ impl AtomicU32Ext for AtomicU32 {
}
}

#[cfg(armv6m)]
#[cfg(not(armv6m))]
impl AtomicU32Ext for AtomicU32 {
#[inline]
fn swap(&self, val: u32, order: Ordering) -> u32 {
core::sync::atomic::AtomicU32::swap(self, val, order)
}

#[inline]
fn fetch_add(&self, val: u32, order: Ordering) -> u32 {
core::sync::atomic::AtomicU32::fetch_add(self, val, order)
}

#[inline]
fn fetch_sub(&self, val: u32, order: Ordering) -> u32 {
core::sync::atomic::AtomicU32::fetch_sub(self, val, order)
}
}

pub trait AtomicBoolExt {
fn swap(&self, val: bool, order: Ordering) -> bool;
}
Expand All @@ -82,6 +97,14 @@ impl AtomicBoolExt for AtomicBool {
}
}

#[cfg(not(armv6m))]
impl AtomicBoolExt for AtomicBool {
#[inline]
fn swap(&self, new: bool, order: Ordering) -> bool {
core::sync::atomic::AtomicBool::swap(self, new, order)
}
}

#[cfg(armv6m)]
fn rmw_ordering(o: Ordering) -> (Ordering, Ordering) {
match o {
Expand Down
2 changes: 0 additions & 2 deletions lib/counters/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ default = ["derive"]

[dependencies]
counters-derive = { path = "derive", optional = true }

[target.'cfg(target_arch = "arm")'.dependencies]
armv6m-atomic-hack = { path = "../armv6m-atomic-hack" }

[lib]
Expand Down
3 changes: 2 additions & 1 deletion lib/counters/derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ impl<'input> CountGenerator<'input> {
};

fn count(&self, counters: &Self::Counters) {
#[cfg(all(target_arch = "arm", armv6m))]
// This extension trait may not be used on non-v6m targets.
#[allow(unused_imports)]
use counters::armv6m_atomic_hack::AtomicU32Ext;

match self {
Expand Down
4 changes: 1 addition & 3 deletions lib/counters/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@
//! and the [`counters!`] macro, which declares a set of static counters
#![no_std]
pub use armv6m_atomic_hack;
#[cfg(feature = "derive")]
pub use counters_derive::Count;

#[cfg(target_arch = "arm")]
pub use armv6m_atomic_hack;

///
/// A countable event.
///
Expand Down
11 changes: 0 additions & 11 deletions lib/static-cell/build.rs

This file was deleted.

8 changes: 3 additions & 5 deletions lib/static-cell/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

#![no_std]

use armv6m_atomic_hack::AtomicBoolExt;
use core::cell::UnsafeCell;
use core::sync::atomic::{AtomicBool, Ordering};

#[cfg(armv6m)]
use armv6m_atomic_hack::AtomicBoolExt;

/// A RefCell-style container that can be used in a static for cases where only
/// a single borrow needs to happen at any given time.
///
Expand All @@ -36,7 +33,8 @@ impl<T> StaticCell<T> {
/// If a `StaticRef` for `self` still exists anywhere in the program, this
/// will panic.
pub fn borrow_mut(&self) -> StaticRef<'_, T> {
let already_borrowed = self.borrowed.swap(true, Ordering::Acquire);
let already_borrowed =
AtomicBoolExt::swap(&self.borrowed, true, Ordering::Acquire);
if already_borrowed {
panic!();
}
Expand Down
1 change: 0 additions & 1 deletion task/gimlet-inspector/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ drv-gimlet-seq-api = { path = "../../drv/gimlet-seq-api" }
userlib = { path = "../../sys/userlib", features = ["panic-messages"] }
counters = { path = "../../lib/counters" }


[build-dependencies]
build-util = { path = "../../build/util" }

Expand Down
1 change: 0 additions & 1 deletion task/hiffy/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

fn main() {
build_util::expose_m_profile().unwrap();
build_util::expose_target_board();
}
7 changes: 3 additions & 4 deletions task/hiffy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@
#![no_std]
#![no_main]

// This trait may not be needed, if compiling for a non-armv6m target.
#[allow(unused_imports)]
use armv6m_atomic_hack::AtomicU32Ext;
use core::sync::atomic::{AtomicU32, Ordering};
use hif::*;
use static_cell::*;
use userlib::*;

#[cfg(armv6m)]
use armv6m_atomic_hack::AtomicU32Ext;

mod common;

cfg_if::cfg_if! {
Expand Down
1 change: 0 additions & 1 deletion task/jefe/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ fn main() -> Result<()> {
)
.unwrap();

build_util::expose_m_profile().unwrap();
build_util::expose_target_board();
build_util::build_notifications()?;

Expand Down
3 changes: 2 additions & 1 deletion task/jefe/src/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
use crate::{Disposition, TaskStatus};
use core::sync::atomic::{AtomicU32, Ordering};

#[cfg(armv6m)]
// This trait may not be needed, if compiling for a non-armv6m target.
#[allow(unused_imports)]
use armv6m_atomic_hack::AtomicU32Ext;

use ringbuf::*;
Expand Down

0 comments on commit 2f23c58

Please sign in to comment.