From 2f23c58a3b14184733a51abfaa8760e736c4d65d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 28 Feb 2024 16:15:10 -0800 Subject: [PATCH] Make `armv6m-atomic-hack` less annoying (#1635) 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. --- Cargo.lock | 1 - drv/i2c-types/Cargo.toml | 3 --- drv/i2c-types/build.rs | 9 --------- lib/armv6m-atomic-hack/src/lib.rs | 29 ++++++++++++++++++++++++++--- lib/counters/Cargo.toml | 2 -- lib/counters/derive/src/lib.rs | 3 ++- lib/counters/src/lib.rs | 4 +--- lib/static-cell/build.rs | 11 ----------- lib/static-cell/src/lib.rs | 8 +++----- task/gimlet-inspector/Cargo.toml | 1 - task/hiffy/build.rs | 1 - task/hiffy/src/main.rs | 7 +++---- task/jefe/build.rs | 1 - task/jefe/src/external.rs | 3 ++- 14 files changed, 37 insertions(+), 46 deletions(-) delete mode 100644 drv/i2c-types/build.rs delete mode 100644 lib/static-cell/build.rs diff --git a/Cargo.lock b/Cargo.lock index aa6a35cc9..7374bd3cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1012,7 +1012,6 @@ dependencies = [ name = "drv-i2c-types" version = "0.1.0" dependencies = [ - "build-util", "counters", "derive-idol-err", "enum-kinds", diff --git a/drv/i2c-types/Cargo.toml b/drv/i2c-types/Cargo.toml index 66633bc38..bdf967b62 100644 --- a/drv/i2c-types/Cargo.toml +++ b/drv/i2c-types/Cargo.toml @@ -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" diff --git a/drv/i2c-types/build.rs b/drv/i2c-types/build.rs deleted file mode 100644 index 94a411244..000000000 --- a/drv/i2c-types/build.rs +++ /dev/null @@ -1,9 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -fn main() { - if let Err(e) = build_util::expose_m_profile() { - println!("cargo:warning={e}"); - } -} diff --git a/lib/armv6m-atomic-hack/src/lib.rs b/lib/armv6m-atomic-hack/src/lib.rs index 503c5cb0f..bc8cc88ab 100644 --- a/lib/armv6m-atomic-hack/src/lib.rs +++ b/lib/armv6m-atomic-hack/src/lib.rs @@ -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; @@ -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; } @@ -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 { diff --git a/lib/counters/Cargo.toml b/lib/counters/Cargo.toml index c8b27354f..0a7bb5c20 100644 --- a/lib/counters/Cargo.toml +++ b/lib/counters/Cargo.toml @@ -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] diff --git a/lib/counters/derive/src/lib.rs b/lib/counters/derive/src/lib.rs index b47c1afe7..ce60aadf1 100644 --- a/lib/counters/derive/src/lib.rs +++ b/lib/counters/derive/src/lib.rs @@ -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 { diff --git a/lib/counters/src/lib.rs b/lib/counters/src/lib.rs index b1a9c881c..bd5a1e1a3 100644 --- a/lib/counters/src/lib.rs +++ b/lib/counters/src/lib.rs @@ -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. /// diff --git a/lib/static-cell/build.rs b/lib/static-cell/build.rs deleted file mode 100644 index 617e4c6e8..000000000 --- a/lib/static-cell/build.rs +++ /dev/null @@ -1,11 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -fn main() -> Result<(), Box> { - match build_util::expose_m_profile() { - Ok(_) => {} - Err(e) => println!("cargo:warn={e}"), - } - Ok(()) -} diff --git a/lib/static-cell/src/lib.rs b/lib/static-cell/src/lib.rs index e0a65b13b..9c0599b22 100644 --- a/lib/static-cell/src/lib.rs +++ b/lib/static-cell/src/lib.rs @@ -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. /// @@ -36,7 +33,8 @@ impl StaticCell { /// 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!(); } diff --git a/task/gimlet-inspector/Cargo.toml b/task/gimlet-inspector/Cargo.toml index 13327565a..a1ad778a5 100644 --- a/task/gimlet-inspector/Cargo.toml +++ b/task/gimlet-inspector/Cargo.toml @@ -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" } diff --git a/task/hiffy/build.rs b/task/hiffy/build.rs index 9a76ad6b9..280089593 100644 --- a/task/hiffy/build.rs +++ b/task/hiffy/build.rs @@ -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(); } diff --git a/task/hiffy/src/main.rs b/task/hiffy/src/main.rs index e9f0bf44f..ff35cbe9b 100644 --- a/task/hiffy/src/main.rs +++ b/task/hiffy/src/main.rs @@ -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! { diff --git a/task/jefe/build.rs b/task/jefe/build.rs index ae657812b..bbb5e7b43 100644 --- a/task/jefe/build.rs +++ b/task/jefe/build.rs @@ -21,7 +21,6 @@ fn main() -> Result<()> { ) .unwrap(); - build_util::expose_m_profile().unwrap(); build_util::expose_target_board(); build_util::build_notifications()?; diff --git a/task/jefe/src/external.rs b/task/jefe/src/external.rs index 1d77b7535..a17b60a23 100644 --- a/task/jefe/src/external.rs +++ b/task/jefe/src/external.rs @@ -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::*;