Fix build and CLI behaviour for stdarch-gen-arm.

crates/intrinsic-test/Cargo.toml

@@ -19,4 +19,4 @@ log = "0.4.11"
 pretty_env_logger = "0.5.0"
 rayon = "1.5.0"
 diff = "0.1.12"
-itertools = "0.11.0"
+itertools = "0.14.0"

crates/stdarch-gen-arm/Cargo.toml

@@ -12,7 +12,7 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-itertools = "0.10"
+itertools = "0.14.0"
 lazy_static = "1.4.0"
 proc-macro2 = "1.0"
 quote = "1.0"

crates/stdarch-gen-arm/src/input.rs

@@ -253,6 +253,7 @@ impl IntrinsicInput {
             .into_iter()
             .map(|v| v.into_iter())
             .multi_cartesian_product()
+            .filter(|set| !set.is_empty())
             .map(|set| InputSet(set.into_iter().flatten().collect_vec()));
         Ok(it)
     }

crates/stdarch-gen-arm/src/intrinsic.rs

@@ -1,5 +1,5 @@
 use itertools::Itertools;
-use proc_macro2::{Punct, Spacing, TokenStream};
+use proc_macro2::{Delimiter, Group, Punct, Spacing, TokenStream};
 use quote::{format_ident, quote, ToTokens, TokenStreamExt};
 use serde::{Deserialize, Serialize};
 use serde_with::{DeserializeFromStr, SerializeDisplay};
@@ -417,35 +417,32 @@ impl ToTokens for LLVMLinkAttribute {
     fn to_tokens(&self, tokens: &mut TokenStream) {
         let LLVMLinkAttribute { arch, link } = self;
         let link = link.to_string();
-        let archs: Vec<&str> = arch.split(',').collect();
-        let arch_len = archs.len();
 
-        if arch_len == 1 {
-            tokens.append_all(quote! {
-                #[cfg_attr(target_arch = #arch, link_name = #link)]
-            })
-        } else {
-            tokens.append(Punct::new('#', Spacing::Alone));
-            tokens.append(Punct::new('[', Spacing::Alone));
-            tokens.append_all(quote! { cfg_attr });
-            tokens.append(Punct::new('(', Spacing::Alone));
-            tokens.append_all(quote! { any });
-            tokens.append(Punct::new('(', Spacing::Alone));
-            let mut i = 0;
-            while i < arch_len {
-                let arch = archs[i].to_string();
-                tokens.append_all(quote! { target_arch = #arch });
-                if i + 1 != arch_len {
-                    tokens.append(Punct::new(',', Spacing::Alone));
-                }
-                i += 1;
+        // For example:
+        //
+        //      #[cfg_attr(target_arch = "arm", link_name = "llvm.ctlz.v4i16")]
+        //
+        //      #[cfg_attr(
+        //          any(target_arch = "aarch64", target_arch = "arm64ec"),
+        //          link_name = "llvm.aarch64.neon.suqadd.i32"
+        //      )]
+
+        let mut cfg_attr_cond = TokenStream::new();
+        let mut single_arch = true;
+        for arch in arch.split(',') {
+            if !cfg_attr_cond.is_empty() {
+                single_arch = false;
+                cfg_attr_cond.append(Punct::new(',', Spacing::Alone));
             }
-            tokens.append(Punct::new(')', Spacing::Alone));
-            tokens.append(Punct::new(',', Spacing::Alone));
-            tokens.append_all(quote! { link_name = #link });
-            tokens.append(Punct::new(')', Spacing::Alone));
-            tokens.append(Punct::new(']', Spacing::Alone));
+            cfg_attr_cond.append_all(quote! { target_arch = #arch });
+        }
+        assert!(!cfg_attr_cond.is_empty());
+        if !single_arch {
+            cfg_attr_cond = quote! { any( #cfg_attr_cond ) };
         }
+        tokens.append_all(quote! {
+            #[cfg_attr(#cfg_attr_cond, link_name = #link)]
+        })
     }
 }
 
@@ -1560,10 +1557,10 @@ impl ToTokens for Intrinsic {
             /* Target feature will get added here */
             let attr_expressions = &mut attr.iter().peekable();
             while let Some(ex) = attr_expressions.next() {
+                let mut inner = TokenStream::new();
+                ex.to_tokens(&mut inner);
                 tokens.append(Punct::new('#', Spacing::Alone));
-                tokens.append(Punct::new('[', Spacing::Alone));
-                ex.to_tokens(tokens);
-                tokens.append(Punct::new(']', Spacing::Alone));
+                tokens.append(Group::new(Delimiter::Bracket, inner));
             }
         } else {
             tokens.append_all(quote! {
@@ -1586,30 +1583,34 @@ impl ToTokens for Intrinsic {
             tokens.append_all(quote! { unsafe });
         }
         tokens.append_all(quote! { #signature });
-        tokens.append(Punct::new('{', Spacing::Alone));
-
-        let mut body_unsafe = false;
-        let mut expressions = self.compose.iter().peekable();
-        while let Some(ex) = expressions.next() {
-            if !body_unsafe && safety.is_safe() && ex.requires_unsafe_wrapper(&fn_name) {
-                body_unsafe = true;
-                tokens.append_all(quote! { unsafe });
-                tokens.append(Punct::new('{', Spacing::Alone));
+
+        // If the intrinsic function is explicitly unsafe, we populate `body_default_safety` with
+        // the implementation. No explicit unsafe blocks are required.
+        //
+        // If the intrinsic is safe, we fill `body_default_safety` until we encounter an expression
+        // that requires an unsafe wrapper, then switch to `body_unsafe`. Since the unsafe
+        // operation (e.g. memory access) is typically the last step, this tends to minimises the
+        // amount of unsafe code required.
+        let mut body_default_safety = TokenStream::new();
+        let mut body_unsafe = TokenStream::new();
+        let mut body_current = &mut body_default_safety;
+        for (pos, ex) in self.compose.iter().with_position() {
+            if safety.is_safe() && ex.requires_unsafe_wrapper(&fn_name) {
+                body_current = &mut body_unsafe;
             }
-            // If it's not the last and not a LLVM link, add a trailing semicolon
-            if expressions.peek().is_some() && !matches!(ex, Expression::LLVMLink(_)) {
-                tokens.append_all(quote! { #ex; })
-            } else {
-                ex.to_tokens(tokens)
+            ex.to_tokens(body_current);
+            let is_last = matches!(pos, itertools::Position::Last | itertools::Position::Only);
+            let is_llvm_link = matches!(ex, Expression::LLVMLink(_));
+            if !is_last && !is_llvm_link {
+                body_current.append(Punct::new(';', Spacing::Alone));
             }
         }
-        if body_unsafe {
-            tokens.append(Punct::new('}', Spacing::Alone));
+        let mut body = body_default_safety;
+        if !body_unsafe.is_empty() {
+            body.append_all(quote! { unsafe { #body_unsafe } });
         }
 
-        tokens.append(Punct::new('}', Spacing::Alone));
-        tokens.append(Punct::new('\n', Spacing::Alone));
-        tokens.append(Punct::new('\n', Spacing::Alone));
+        tokens.append(Group::new(Delimiter::Brace, body));
     }
 }
 

crates/stdarch-gen-arm/src/load_store_tests.rs

@@ -91,11 +91,11 @@ pub fn generate_load_store_tests(
         format!(
             "// This code is automatically generated. DO NOT MODIFY.
 //
-// Instead, modify `crates/stdarch-gen2/spec/sve` and run the following command to re-generate this
-// file:
+// Instead, modify `crates/stdarch-gen-arm/spec/sve` and run the following command to re-generate
+// this file:
 //
 // ```
-// cargo run --bin=stdarch-gen2 -- crates/stdarch-gen2/spec
+// cargo run --bin=stdarch-gen-arm -- crates/stdarch-gen-arm/spec
 // ```
 {}",
             quote! { #preamble #(#tests)* #manual_tests }

crates/stdarch-gen-arm/src/main.rs

@@ -104,7 +104,10 @@ fn parse_args() -> Vec<(PathBuf, Option<PathBuf>)> {
     let mut args_it = std::env::args().skip(1);
     assert!(
         1 <= args_it.len() && args_it.len() <= 2,
-        "Usage: cargo run -p stdarch-gen2 -- INPUT_DIR [OUTPUT_DIR]"
+        "Usage: cargo run -p stdarch-gen-arm -- INPUT_DIR [OUTPUT_DIR]\n\
+        where:\n\
+        - INPUT_DIR contains a tree like: INPUT_DIR/<feature>/<arch>.spec.yml\n\
+        - OUTPUT_DIR is a directory like: crates/core_arch/src/"
     );
 
     let in_path = Path::new(args_it.next().unwrap().as_str()).to_path_buf();
@@ -124,7 +127,7 @@ fn parse_args() -> Vec<(PathBuf, Option<PathBuf>)> {
         std::env::current_exe()
             .map(|mut f| {
                 f.pop();
-                f.push("../../crates/core_arch/src/aarch64/");
+                f.push("../../crates/core_arch/src/");
                 f.exists().then_some(f)
             })
             .ok()
@@ -147,10 +150,10 @@ fn generate_file(
         out,
         r#"// This code is automatically generated. DO NOT MODIFY.
 //
-// Instead, modify `crates/stdarch-gen2/spec/` and run the following command to re-generate this file:
+// Instead, modify `crates/stdarch-gen-arm/spec/` and run the following command to re-generate this file:
 //
 // ```
-// cargo run --bin=stdarch-gen2 -- crates/stdarch-gen2/spec
+// cargo run --bin=stdarch-gen-arm -- crates/stdarch-gen-arm/spec
 // ```
 #![allow(improper_ctypes)]
 
@@ -183,17 +186,19 @@ pub fn format_code(
     output.write_all(proc.wait_with_output()?.stdout.as_slice())
 }
 
-/// Derive an output file name from an input file and an output directory.
+/// Derive an output file path from an input file path and an output directory.
 ///
-/// The name is formed by:
+/// `in_filepath` is expected to have a structure like:
+///     .../<feature>/<arch>.spec.yml
 ///
-///   - ... taking in_filepath.file_name() (dropping all directory components),
-///   - ... dropping a .yml or .yaml extension (if present),
-///   - ... then dropping a .spec extension (if present).
+/// The resulting output path will have a structure like:
+///     <out_dirpath>/<arch>/<feature>/generated.rs
 ///
 /// Panics if the resulting name is empty, or if file_name() is not UTF-8.
 fn make_output_filepath(in_filepath: &Path, out_dirpath: &Path) -> PathBuf {
-    make_filepath(in_filepath, out_dirpath, |name: &str| format!("{name}.rs"))
+    make_filepath(in_filepath, out_dirpath, |_name: &str| {
+        format!("generated.rs")
+    })
 }
 
 fn make_tests_filepath(in_filepath: &Path, out_dirpath: &Path) -> PathBuf {
@@ -207,22 +212,27 @@ fn make_filepath<F: FnOnce(&str) -> String>(
     out_dirpath: &Path,
     name_formatter: F,
 ) -> PathBuf {
-    let mut parts = in_filepath.iter();
-    let name = parts
-        .next_back()
-        .and_then(|f| f.to_str())
-        .expect("Inputs must have valid, UTF-8 file_name()");
-    let dir = parts.next_back().unwrap();
+    let mut parts = in_filepath.components().rev().map(|f| {
+        f.as_os_str()
+            .to_str()
+            .expect("Inputs must have valid, UTF-8 file_name()")
+    });
+    let yml = parts.next().expect("Not enough input path elements.");
+    let feature = parts.next().expect("Not enough input path elements.");
 
-    let name = name
-        .trim_end_matches(".yml")
-        .trim_end_matches(".yaml")
-        .trim_end_matches(".spec");
-    assert!(!name.is_empty());
+    let arch = yml
+        .strip_suffix(".yml")
+        .expect("Expected .yml file input.")
+        .strip_suffix(".spec")
+        .expect("Expected .spec.yml file input.");
+    if arch.is_empty() {
+        panic!("Extended ARCH.spec.yml file input.");
+    }
 
     let mut output = out_dirpath.to_path_buf();
-    output.push(dir);
-    output.push(name_formatter(name));
+    output.push(arch);
+    output.push(feature);
+    output.push(name_formatter(arch));
     output
 }
 
@@ -233,47 +243,68 @@ mod tests {
     #[test]
     fn infer_output_file() {
         macro_rules! t {
-            ($src:expr, $outdir:expr, $dst:expr) => {
+            ($src:expr, $outdir:expr, $dst:expr, $ldst:expr) => {
                 let src: PathBuf = $src.iter().collect();
                 let outdir: PathBuf = $outdir.iter().collect();
                 let dst: PathBuf = $dst.iter().collect();
+                let ldst: PathBuf = $ldst.iter().collect();
                 assert_eq!(make_output_filepath(&src, &outdir), dst);
+                assert_eq!(make_tests_filepath(&src, &outdir), ldst);
             };
         }
         // Documented usage.
-        t!(["x", "NAME.spec.yml"], [""], ["x", "NAME.rs"]);
         t!(
-            ["x", "NAME.spec.yml"],
-            ["a", "b"],
-            ["a", "b", "x", "NAME.rs"]
+            ["FEAT", "ARCH.spec.yml"],
+            [""],
+            ["ARCH", "FEAT", "generated.rs"],
+            ["ARCH", "FEAT", "ld_st_tests_ARCH.rs"]
         );
         t!(
-            ["x", "y", "NAME.spec.yml"],
+            ["x", "y", "FEAT", "ARCH.spec.yml"],
             ["out"],
-            ["out", "y", "NAME.rs"]
+            ["out", "ARCH", "FEAT", "generated.rs"],
+            ["out", "ARCH", "FEAT", "ld_st_tests_ARCH.rs"]
         );
-        t!(["x", "NAME.spec.yaml"], ["out"], ["out", "x", "NAME.rs"]);
-        t!(["x", "NAME.spec"], ["out"], ["out", "x", "NAME.rs"]);
-        t!(["x", "NAME.yml"], ["out"], ["out", "x", "NAME.rs"]);
-        t!(["x", "NAME.yaml"], ["out"], ["out", "x", "NAME.rs"]);
-        // Unrecognised extensions get treated as part of the stem.
         t!(
-            ["x", "NAME.spac.yml"],
-            ["out"],
-            ["out", "x", "NAME.spac.rs"]
+            ["p", "q", "FEAT", "ARCH.spec.yml"],
+            ["a", "b"],
+            ["a", "b", "ARCH", "FEAT", "generated.rs"],
+            ["a", "b", "ARCH", "FEAT", "ld_st_tests_ARCH.rs"]
         );
-        t!(["x", "NAME.txt"], ["out"], ["out", "x", "NAME.txt.rs"]);
-        // Always take the top-level directory from the input path
+        // Extra extensions get treated as part of the stem.
         t!(
-            ["x", "y", "z", "NAME.spec.yml"],
+            ["FEAT", "ARCH.variant.spec.yml"],
             ["out"],
-            ["out", "z", "NAME.rs"]
+            ["out", "ARCH.variant", "FEAT", "generated.rs"],
+            ["out", "ARCH.variant", "FEAT", "ld_st_tests_ARCH.variant.rs"]
         );
     }
 
     #[test]
     #[should_panic]
     fn infer_output_file_no_stem() {
-        make_output_filepath(Path::new(".spec.yml"), Path::new(""));
+        let src = PathBuf::from("FEAT/.spec.yml");
+        make_output_filepath(&src, Path::new(""));
+    }
+
+    #[test]
+    #[should_panic]
+    fn infer_output_file_no_feat() {
+        let src = PathBuf::from("ARCH.spec.yml");
+        make_output_filepath(&src, Path::new(""));
+    }
+
+    #[test]
+    #[should_panic]
+    fn infer_output_file_ldst_no_stem() {
+        let src = PathBuf::from("FEAT/.spec.yml");
+        make_tests_filepath(&src, Path::new(""));
+    }
+
+    #[test]
+    #[should_panic]
+    fn infer_output_file_ldst_no_feat() {
+        let src = PathBuf::from("ARCH.spec.yml");
+        make_tests_filepath(&src, Path::new(""));
     }
 }