Merge #693

693: Generate atomic module outside of MSP430 r=burrbull a=YuhanLiin

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

Co-authored-by: YuhanLiin <[email protected]>

Author: bors[bot]

.github/workflows/ci.yml

@@ -53,29 +53,36 @@ jobs:
           - { rust: stable, vendor: Freescale, options: all }
           - { rust: stable, vendor: Freescale, options: "" }
           - { rust: stable, vendor: Fujitsu, options: "" }
+          - { rust: stable, vendor: Fujitsu, options: "--atomics" }
           - { rust: stable, vendor: GD32, options: all }
           - { rust: stable, vendor: GD32, options: "" }
           - { rust: stable, vendor: Holtek, options: all }
           - { rust: stable, vendor: Holtek, options: "" }
           - { rust: stable, vendor: Microchip, options: "" }
+          - { rust: stable, vendor: Microchip, options: "--atomics" }
           - { rust: stable, vendor: Nordic, options: all }
           - { rust: stable, vendor: Nordic, options: "" }
           - { rust: stable, vendor: Nuvoton, options: "" }
+          - { rust: stable, vendor: Nuvoton, options: "--atomics" }
           - { rust: stable, vendor: NXP, options: all }
           - { rust: stable, vendor: NXP, options: "" }
           - { rust: stable, vendor: RISC-V, options: "" }
+          - { rust: stable, vendor: RISC-V, options: "--atomics" }
           - { rust: stable, vendor: SiliconLabs, options: all }
           - { rust: stable, vendor: SiliconLabs, options: "" }
           - { rust: stable, vendor: Spansion, options: "" }
+          - { rust: stable, vendor: Spansion, options: "--atomics" }
           - { rust: stable, vendor: STMicro, options: "" }
-          - { rust: stable, vendor: STM32-patched, options: "--strict --const_generic --derive_more --pascal_enum_values --max_cluster_size" }
+          - { rust: stable, vendor: STMicro, options: "--atomics" }
+          - { rust: stable, vendor: STM32-patched, options: "--strict --const_generic --derive_more --pascal_enum_values --max_cluster_size --atomics" }
           - { rust: stable, vendor: Toshiba, options: all }
           - { rust: stable, vendor: Toshiba, options: "" }
           # Test MSRV
           - { rust: 1.61.0, vendor: Nordic, options: "" }
           # Use nightly for architectures which don't support stable
-          - { rust: nightly, vendor: MSP430, options: "--nightly" }
+          - { rust: nightly, vendor: MSP430, options: "--atomics" }
           - { rust: nightly, vendor: MSP430, options: "" }
+          - { rust: nightly, vendor: Espressif, options: "--atomics" }
           - { rust: nightly, vendor: Espressif, options: "" }
 
     steps:

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+- Generate atomic register code for non-MSP430 targets
+- Change --nightly flag to --atomics
+
 ## [v0.27.2] - 2022-11-06
 
 - mark alternate register accessors with `const`, bump `pac` MSRV to 1.61

ci/script.sh

@@ -31,7 +31,7 @@ main() {
 
     case $OPTIONS in
         all)
-            options="--const_generic --strict --derive_more"
+            options="--const_generic --strict --derive_more --atomics"
             ;;
         *)
             options=$OPTIONS
@@ -46,6 +46,9 @@ main() {
     if [[ "$options" == *"--derive_more"* ]]; then
         echo 'derive_more = "0.99"' >> $td/Cargo.toml
     fi
+    if [[ "$options" == *"--atomics"* ]]; then
+        echo 'portable-atomic = { version = "0.3.16", default-features = false }' >> $td/Cargo.toml
+    fi
     echo '[profile.dev]' >> $td/Cargo.toml
     echo 'incremental = false' >> $td/Cargo.toml
 
@@ -465,9 +468,6 @@ main() {
             echo '[dependencies.msp430]' >> $td/Cargo.toml
             echo 'version = "0.4.0"' >> $td/Cargo.toml
 
-            echo '[dependencies.portable-atomic]' >> $td/Cargo.toml
-            echo 'version = "0.3.15"' >> $td/Cargo.toml
-
             # Test MSP430
             test_svd_for_target msp430 https://raw.githubusercontent.com/pftbest/msp430g2553/v0.3.0-svd/msp430g2553.svd
             test_svd_for_target msp430 https://raw.githubusercontent.com/YuhanLiin/msp430fr2355/master/msp430fr2355.svd

ci/svd2rust-regress/src/main.rs

@@ -29,9 +29,9 @@ struct Opt {
     bin_path: Option<PathBuf>,
 
     // TODO: Consider using the same strategy cargo uses for passing args to rustc via `--`
-    /// Run svd2rust with `--nightly`
-    #[structopt(long = "nightly")]
-    nightly: bool,
+    /// Run svd2rust with `--atomics`
+    #[structopt(long = "atomics")]
+    atomics: bool,
 
     /// Filter by chip name, case sensitive, may be combined with other filters
     #[structopt(short = "c", long = "chip", raw(validator = "validate_chips"))]
@@ -240,7 +240,7 @@ fn main() {
     tests.par_iter().for_each(|t| {
         let start = Instant::now();
 
-        match svd_test::test(t, &bin_path, rustfmt_bin_path, opt.nightly, opt.verbose) {
+        match svd_test::test(t, &bin_path, rustfmt_bin_path, opt.atomics, opt.verbose) {
             Ok(s) => {
                 if let Some(stderrs) = s {
                     let mut buf = String::new();

ci/svd2rust-regress/src/svd_test.rs

@@ -7,7 +7,8 @@ use std::process::{Command, Output};
 
 const CRATES_ALL: &[&str] = &["critical-section = \"1.0\"", "vcell = \"0.1.2\""];
 const CRATES_MSP430: &[&str] = &["msp430 = \"0.4.0\"", "msp430-rt = \"0.4.0\""];
-const CRATES_MSP430_NIGHTLY: &[&str] = &["portable-atomic = \"0.3.15\""];
+const CRATES_ATOMICS: &[&str] =
+    &["portable-atomic = { version = \"0.3.16\", default-features = false }"];
 const CRATES_CORTEX_M: &[&str] = &["cortex-m = \"0.7.6\"", "cortex-m-rt = \"0.6.13\""];
 const CRATES_RISCV: &[&str] = &["riscv = \"0.9.0\"", "riscv-rt = \"0.9.0\""];
 const CRATES_XTENSALX: &[&str] = &["xtensa-lx-rt = \"0.9.0\"", "xtensa-lx = \"0.6.0\""];
@@ -84,7 +85,7 @@ pub fn test(
     t: &TestCase,
     bin_path: &PathBuf,
     rustfmt_bin_path: Option<&PathBuf>,
-    nightly: bool,
+    atomics: bool,
     verbosity: u8,
 ) -> Result<Option<Vec<PathBuf>>> {
     let user = match std::env::var("USER") {
@@ -136,11 +137,8 @@ pub fn test(
             Msp430 => CRATES_MSP430.iter(),
             XtensaLX => CRATES_XTENSALX.iter(),
         })
-        .chain(if nightly {
-            match &t.arch {
-                Msp430 => CRATES_MSP430_NIGHTLY.iter(),
-                _ => [].iter(),
-            }
+        .chain(if atomics {
+            CRATES_ATOMICS.iter()
         } else {
             [].iter()
         })
@@ -179,8 +177,8 @@ pub fn test(
         XtensaLX => "xtensa-lx",
     };
     let mut svd2rust_bin = Command::new(bin_path);
-    if nightly {
-        svd2rust_bin.arg("--nightly");
+    if atomics {
+        svd2rust_bin.arg("--atomics");
     }
 
     let output = svd2rust_bin

ci/svd2rust-regress/src/tests.rs

@@ -4195,7 +4195,7 @@ pub const TESTS: &[&TestCase] = &[
         mfgr: TexasInstruments,
         chip: "msp430fr2355",
         svd_url: Some(
-            "https://raw.githubusercontent.com/YuhanLiin/msp430fr2355/rt-up/msp430fr2355.svd",
+            "https://raw.githubusercontent.com/YuhanLiin/msp430fr2355/master/msp430fr2355.svd",
         ),
         should_pass: true,
         run_when: Always,

src/generate/device.rs

@@ -147,13 +147,13 @@ pub fn render(d: &Device, config: &Config, device_x: &mut String) -> Result<Toke
     }
 
     let generic_file = include_str!("generic.rs");
-    let msp430_atomic_file = include_str!("generic_msp430_atomic.rs");
+    let generic_atomic_file = include_str!("generic_atomic.rs");
     let array_proxy = include_str!("array_proxy.rs");
     if config.generic_mod {
         let mut file = File::create(config.output_dir.join("generic.rs"))?;
         writeln!(file, "{}", generic_file)?;
-        if config.target == Target::Msp430 && config.nightly {
-            writeln!(file, "\n{}", msp430_atomic_file)?;
+        if config.atomics {
+            writeln!(file, "\n{}", generic_atomic_file)?;
         }
         if config.const_generic {
             writeln!(file, "{}", array_proxy)?;
@@ -169,8 +169,8 @@ pub fn render(d: &Device, config: &Config, device_x: &mut String) -> Result<Toke
         }
     } else {
         let mut tokens = syn::parse_file(generic_file)?.into_token_stream();
-        if config.target == Target::Msp430 && config.nightly {
-            syn::parse_file(msp430_atomic_file)?.to_tokens(&mut tokens);
+        if config.atomics {
+            syn::parse_file(generic_atomic_file)?.to_tokens(&mut tokens);
         }
         if config.const_generic {
             syn::parse_file(array_proxy)?.to_tokens(&mut tokens);

src/generate/generic_msp430_atomic.rs renamed to src/generate/generic_atomic.rs

@@ -1,5 +1,5 @@
 mod atomic {
-    use portable_atomic::{AtomicU16, AtomicU8, Ordering};
+    use portable_atomic::Ordering;
 
     pub trait AtomicOperations {
         unsafe fn atomic_or(ptr: *mut Self, val: Self);
@@ -11,21 +11,30 @@ mod atomic {
         ($U:ty, $Atomic:ty) => {
             impl AtomicOperations for $U {
                 unsafe fn atomic_or(ptr: *mut Self, val: Self) {
-                    (*(ptr as *const $Atomic)).fetch_or(val, Ordering::SeqCst);
+                    (*(ptr as *const $Atomic)).or(val, Ordering::SeqCst);
                 }
 
                 unsafe fn atomic_and(ptr: *mut Self, val: Self) {
-                    (*(ptr as *const $Atomic)).fetch_and(val, Ordering::SeqCst);
+                    (*(ptr as *const $Atomic)).and(val, Ordering::SeqCst);
                 }
 
                 unsafe fn atomic_xor(ptr: *mut Self, val: Self) {
-                    (*(ptr as *const $Atomic)).fetch_xor(val, Ordering::SeqCst);
+                    (*(ptr as *const $Atomic)).xor(val, Ordering::SeqCst);
                 }
             }
         };
     }
-    impl_atomics!(u8, AtomicU8);
-    impl_atomics!(u16, AtomicU16);
+
+    impl_atomics!(u8, portable_atomic::AtomicU8);
+    impl_atomics!(u16, portable_atomic::AtomicU16);
+
+    // Exclude 16-bit archs from 32-bit atomics
+    #[cfg(not(target_pointer_width = "16"))]
+    impl_atomics!(u32, portable_atomic::AtomicU32);
+
+    // Enable 64-bit atomics for 64-bit RISCV
+    #[cfg(any(target_pointer_width = "64", target_has_atomic = "64"))]
+    impl_atomics!(u64, portable_atomic::AtomicU64);
 }
 use atomic::AtomicOperations;
 

src/lib.rs

@@ -122,21 +122,18 @@
 //! - [`msp430-rt`](https://crates.io/crates/msp430-rt) v0.4.x
 //! - [`vcell`](https://crates.io/crates/vcell) v0.1.x
 //!
-//! If the `--nightly` flag is provided to `svd2rust`, then `portable-atomic` v0.3.15 is also needed.
-//! Furthermore the "device" feature of `msp430-rt` must be enabled when the `rt` feature is
+//! The "device" feature of `msp430-rt` must be enabled when the `rt` feature is
 //! enabled. The `Cargo.toml` of the device crate will look like this:
 //!
 //! ``` toml
 //! [dependencies]
 //! critical-section = { version = "1.0", optional = true }
 //! msp430 = "0.4.0"
-//! portable-atomic = "0.3.15" # Only when using the --nightly flag
 //! msp430-rt = { version = "0.4.0", optional = true }
 //! vcell = "0.1.0"
 //!
 //! [features]
 //! rt = ["msp430-rt/device"]
-//! unstable = ["portable-atomic"]
 //! ```
 //!
 //! ## Other targets
@@ -487,17 +484,16 @@
 //!
 //! [`interrupt`]: https://docs.rs/cortex-m-rt-macros/0.1/cortex_m_rt_macros/attr.interrupt.html
 //!
-//! ## the `--nightly` flag
+//! ## the `--atomics` flag
 //!
-//! The `--nightly` flag can be passed to `svd2rust` to enable features in the generated api that are only available to a nightly
-//! compiler. The following features are gated by the `--nightly` flag:
+//! The `--atomics` flag can be passed to `svd2rust` to extends the register API with operations to
+//! atomically set, clear, and toggle specific bits.  The atomic operations allow limited
+//! modification of register bits without read-modify-write sequences. As such, they can be
+//! concurrently called on different bits in the same register without data races. This flag won't
+//! work for RISCV chips without the atomic extension.
 //!
-//! ### MSP430
-//!
-//! Extends the register API with operations to atomically set, clear, and toggle specific bits.
-//! The atomic operations allow limited modification of register bits without read-modify-write
-//! sequences. As such, they can be concurrently called on different bits in the same register
-//! without data races.
+//! `portable-atomic` v0.3.16 must be added to the dependencies, with default features off to
+//! disable the `fallback` feature.
 //!
 //! Usage examples:
 //!

src/main.rs

@@ -67,10 +67,10 @@ fn run() -> Result<()> {
                 .value_name("ARCH"),
         )
         .arg(
-            Arg::new("nightly")
-                .long("nightly")
+            Arg::new("atomics")
+                .long("atomics")
                 .action(ArgAction::SetTrue)
-                .help("Enable features only available to nightly rustc"),
+                .help("Generate atomic register modification API"),
         )
         .arg(
             Arg::new("const_generic")

src/util.rs

@@ -27,7 +27,7 @@ pub struct Config {
     #[cfg_attr(feature = "serde", serde(default))]
     pub target: Target,
     #[cfg_attr(feature = "serde", serde(default))]
-    pub nightly: bool,
+    pub atomics: bool,
     #[cfg_attr(feature = "serde", serde(default))]
     pub generic_mod: bool,
     #[cfg_attr(feature = "serde", serde(default))]
@@ -68,7 +68,7 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             target: Target::default(),
-            nightly: false,
+            atomics: false,
             generic_mod: false,
             make_mod: false,
             const_generic: false,