moved the C compilation commands into a struct for easier handling

madhav-madhusoodanan · Amanieu · commit 5c80b3fb2ace · 2025-05-27T23:27:38.000Z
diff --git a/crates/intrinsic-test/src/arm/functions.rs b/crates/intrinsic-test/src/arm/functions.rs
@@ -2,6 +2,7 @@ use super::config::{AARCH_CONFIGURATIONS, POLY128_OSTREAM_DEF, build_notices};
 use super::intrinsic::ArmIntrinsicType;
 use crate::arm::constraint::Constraint;
 use crate::common::argument::Argument;
+use crate::common::compile_c::CompilationCommandBuilder;
 use crate::common::format::Indentation;
 use crate::common::gen_c::{compile_c, create_c_filenames, generate_c_program};
 use crate::common::gen_rust::{compile_rust, create_rust_filenames, generate_rust_program};
@@ -161,70 +162,55 @@ fn generate_rust_program_arm(
 
 fn compile_c_arm(
     intrinsics_name_list: &Vec<String>,
-    filename_mapping: BTreeMap<&String, String>,
+    _filename_mapping: BTreeMap<&String, String>,
     compiler: &str,
     target: &str,
     cxx_toolchain_dir: Option<&str>,
 ) -> bool {
-    let compiler_commands = intrinsics_name_list.iter().map(|intrinsic_name| {
-        let c_filename = filename_mapping.get(intrinsic_name).unwrap();
-        let flags = std::env::var("CPPFLAGS").unwrap_or("".into());
-        let arch_flags = if target.contains("v7") {
-            "-march=armv8.6-a+crypto+crc+dotprod+fp16"
-        } else {
-            "-march=armv8.6-a+crypto+sha3+crc+dotprod+fp16+faminmax+lut"
-        };
+    let mut command = CompilationCommandBuilder::new()
+        .add_arch_flags(vec!["armv8.6-a", "crypto", "crc", "dotprod", "fp16"])
+        .set_compiler(compiler)
+        .set_target(target)
+        .set_opt_level("2")
+        .set_cxx_toolchain_dir(cxx_toolchain_dir)
+        .set_project_root("c_programs")
+        .add_extra_flags(vec!["-ffp-contract=off", "-Wno-narrowing"]);
 
-        let compiler_command = if target == "aarch64_be-unknown-linux-gnu" {
-            let Some(cxx_toolchain_dir) = cxx_toolchain_dir else {
-                panic!(
-                    "When setting `--target aarch64_be-unknown-linux-gnu` the C++ compilers toolchain directory must be set with `--cxx-toolchain-dir <dest>`"
-                );
-            };
+    if !target.contains("v7") {
+        command = command.add_arch_flags(vec!["faminmax", "lut", "sha3"]);
+    }
 
-            /* clang++ cannot link an aarch64_be object file, so we invoke
-             * aarch64_be-unknown-linux-gnu's C++ linker. This ensures that we
-             * are testing the intrinsics against LLVM.
-             *
-             * Note: setting `--sysroot=<...>` which is the obvious thing to do
-             * does not work as it gets caught up with `#include_next <stdlib.h>`
-             * not existing... */
-            format!(
-                "{compiler} {flags} {arch_flags} \
-                -ffp-contract=off \
-                -Wno-narrowing \
-                -O2 \
-                --target=aarch64_be-unknown-linux-gnu \
-                -I{cxx_toolchain_dir}/include \
-                -I{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/include \
-                -I{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/include/c++/14.2.1 \
-                -I{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/include/c++/14.2.1/aarch64_be-none-linux-gnu \
-                -I{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/include/c++/14.2.1/backward \
-                -I{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/libc/usr/include \
-                -c {c_filename} \
-                -o c_programs/{intrinsic_name}.o && \
-                {cxx_toolchain_dir}/bin/aarch64_be-none-linux-gnu-g++ c_programs/{intrinsic_name}.o -o c_programs/{intrinsic_name} && \
-                rm c_programs/{intrinsic_name}.o",
+    command = if target == "aarch64_be-unknown-linux-gnu" {
+        command
+            .set_linker(
+                cxx_toolchain_dir.unwrap_or("").to_string() + "/bin/aarch64_be-none-linux-gnu-g++",
             )
+            .set_include_paths(vec![
+                "/include",
+                "/aarch64_be-none-linux-gnu/include",
+                "/aarch64_be-none-linux-gnu/include/c++/14.2.1",
+                "/aarch64_be-none-linux-gnu/include/c++/14.2.1/aarch64_be-none-linux-gnu",
+                "/aarch64_be-none-linux-gnu/include/c++/14.2.1/backward",
+                "/aarch64_be-none-linux-gnu/libc/usr/include",
+            ])
+    } else {
+        if compiler.contains("clang") {
+            command.add_extra_flag(format!("-target {target}").as_str())
         } else {
-            // -ffp-contract=off emulates Rust's approach of not fusing separate mul-add operations
-            let base_compiler_command = format!(
-                "{compiler} {flags} {arch_flags} -o c_programs/{intrinsic_name} {c_filename} -ffp-contract=off -Wno-narrowing -O2"
-            );
-
-            /* `-target` can be passed to some c++ compilers, however if we want to
-             *   use a c++ compiler does not support this flag we do not want to pass
-             *   the flag. */
-            if compiler.contains("clang") {
-                format!("{base_compiler_command} -target {target}")
-            } else {
-                format!("{base_compiler_command} -flax-vector-conversions")
-            }
-        };
+            command.add_extra_flag("-flax-vector-conversions")
+        }
+    };
 
-        compiler_command
-    })
-    .collect::<Vec<_>>();
+    let compiler_commands = intrinsics_name_list
+        .iter()
+        .map(|intrinsic_name| {
+            command
+                .clone()
+                .set_input_name(intrinsic_name)
+                .set_output_name(intrinsic_name)
+                .to_string()
+        })
+        .collect::<Vec<_>>();
 
     compile_c(&compiler_commands)
 }
diff --git a/crates/intrinsic-test/src/common/compile_c.rs b/crates/intrinsic-test/src/common/compile_c.rs
@@ -0,0 +1,151 @@
+#[derive(Clone)]
+pub struct CompilationCommandBuilder {
+    compiler: String,
+    target: Option<String>,
+    cxx_toolchain_dir: Option<String>,
+    arch_flags: Vec<String>,
+    optimization: String,
+    include_paths: Vec<String>,
+    project_root: Option<String>,
+    output: String,
+    input: String,
+    linker: Option<String>,
+    extra_flags: Vec<String>,
+}
+
+impl CompilationCommandBuilder {
+    pub fn new() -> Self {
+        Self {
+            compiler: String::new(),
+            target: None,
+            cxx_toolchain_dir: None,
+            arch_flags: Vec::new(),
+            optimization: "2".to_string(),
+            include_paths: Vec::new(),
+            project_root: None,
+            output: String::new(),
+            input: String::new(),
+            linker: None,
+            extra_flags: Vec::new(),
+        }
+    }
+
+    pub fn set_compiler(mut self, compiler: &str) -> Self {
+        self.compiler = compiler.to_string();
+        self
+    }
+
+    pub fn set_target(mut self, target: &str) -> Self {
+        self.target = Some(target.to_string());
+        self
+    }
+
+    pub fn set_cxx_toolchain_dir(mut self, path: Option<&str>) -> Self {
+        self.cxx_toolchain_dir = path.map(|p| p.to_string());
+        self
+    }
+
+    pub fn add_arch_flags(mut self, flags: Vec<&str>) -> Self {
+        let mut new_arch_flags = flags.into_iter().map(|v| v.to_string()).collect();
+        self.arch_flags.append(&mut new_arch_flags);
+
+        self
+    }
+
+    pub fn set_opt_level(mut self, optimization: &str) -> Self {
+        self.optimization = optimization.to_string();
+        self
+    }
+
+    /// Sets a list of include paths for compilation.
+    /// The paths that are passed must be relative to the
+    /// "cxx_toolchain_dir" directory path.
+    pub fn set_include_paths(mut self, paths: Vec<&str>) -> Self {
+        self.include_paths = paths.into_iter().map(|path| path.to_string()).collect();
+        self
+    }
+
+    /// Sets the root path of all the generated test files.
+    pub fn set_project_root(mut self, path: &str) -> Self {
+        self.project_root = Some(path.to_string());
+        self
+    }
+
+    /// The name of the output executable, without any suffixes
+    pub fn set_output_name(mut self, path: &str) -> Self {
+        self.output = path.to_string();
+        self
+    }
+
+    /// The name of the input C file, without any suffixes
+    pub fn set_input_name(mut self, path: &str) -> Self {
+        self.input = path.to_string();
+        self
+    }
+
+    pub fn set_linker(mut self, linker: String) -> Self {
+        self.linker = Some(linker);
+        self.output += ".o";
+        self
+    }
+
+    pub fn add_extra_flags(mut self, flags: Vec<&str>) -> Self {
+        let mut flags: Vec<String> = flags.into_iter().map(|f| f.to_string()).collect();
+        self.extra_flags.append(&mut flags);
+        self
+    }
+
+    pub fn add_extra_flag(self, flag: &str) -> Self {
+        self.add_extra_flags(vec![flag])
+    }
+}
+
+impl CompilationCommandBuilder {
+    pub fn to_string(self) -> String {
+        let arch_flags = self.arch_flags.join("+");
+        let flags = std::env::var("CPPFLAGS").unwrap_or("".into());
+        let project_root = self.project_root.unwrap_or(String::new());
+        let project_root_str = project_root.as_str();
+        let mut command = format!(
+            "{} {flags} -march={arch_flags} \
+            -O{} \
+            -o {project_root}/{} \
+            {project_root}/{}.cpp",
+            self.compiler, self.optimization, self.output, self.input,
+        );
+
+        command = command + " " + self.extra_flags.join(" ").as_str();
+
+        if let (Some(linker), Some(cxx_toolchain_dir)) = (&self.linker, &self.cxx_toolchain_dir) {
+            if let Some(target) = &self.target {
+                command = command + " --target=" + target;
+            }
+
+            let include_args = self
+                .include_paths
+                .iter()
+                .map(|path| "--include-directory=".to_string() + cxx_toolchain_dir + path)
+                .collect::<Vec<_>>()
+                .join(" ");
+
+            command = command
+                + " -c "
+                + include_args.as_str()
+                + " && "
+                + linker
+                + project_root_str
+                + "/"
+                + self.output.as_str()
+                + " -o "
+                + project_root_str
+                + "/"
+                + self.output.strip_suffix(".o").unwrap()
+                + " && rm "
+                + project_root_str
+                + "/"
+                + self.output.as_str();
+        }
+
+        command
+    }
+}
diff --git a/crates/intrinsic-test/src/common/mod.rs b/crates/intrinsic-test/src/common/mod.rs
@@ -4,6 +4,7 @@ use std::io::Write;
 
 pub mod argument;
 pub mod compare;
+pub mod compile_c;
 pub mod format;
 pub mod gen_c;
 pub mod gen_rust;
