From ecfe865c1e5e3dc35d570f8ffb916dbfab73976a Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Thu, 10 Jul 2025 20:09:46 +0200 Subject: [PATCH 1/2] pass the whole config to C compiler construction --- Cargo.lock | 4 +-- crates/intrinsic-test/src/arm/compile.rs | 45 +++++++++++++++--------- crates/intrinsic-test/src/arm/mod.rs | 13 +------ 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 122b3057b2..f287ed6e3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -90,9 +90,9 @@ checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" [[package]] name = "cc" -version = "1.2.29" +version = "1.2.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c1599538de2394445747c8cf7935946e3cc27e9625f889d979bfb2aaf569362" +checksum = "deec109607ca693028562ed836a5f1c4b8bd77755c4e132fc5ce11b0b6211ae7" dependencies = [ "shlex", ] diff --git a/crates/intrinsic-test/src/arm/compile.rs b/crates/intrinsic-test/src/arm/compile.rs index 79cd32dedc..a90caf367c 100644 --- a/crates/intrinsic-test/src/arm/compile.rs +++ b/crates/intrinsic-test/src/arm/compile.rs @@ -1,23 +1,23 @@ +use crate::common::cli::ProcessedCli; use crate::common::compile_c::CompilationCommandBuilder; use crate::common::gen_c::compile_c_programs; -pub fn compile_c_arm( - intrinsics_name_list: &[String], - compiler: &str, - target: &str, - cxx_toolchain_dir: Option<&str>, -) -> bool { +pub fn compile_c_arm(config: &ProcessedCli, intrinsics_name_list: &[String]) -> bool { + let Some(ref cpp_compiler) = config.cpp_compiler else { + return true; + }; + // -ffp-contract=off emulates Rust's approach of not fusing separate mul-add operations let mut command = CompilationCommandBuilder::new() .add_arch_flags(vec!["armv8.6-a", "crypto", "crc", "dotprod", "fp16"]) - .set_compiler(compiler) - .set_target(target) + .set_compiler(cpp_compiler) + .set_target(&config.target) .set_opt_level("2") - .set_cxx_toolchain_dir(cxx_toolchain_dir) + .set_cxx_toolchain_dir(config.cxx_toolchain_dir.as_deref()) .set_project_root("c_programs") .add_extra_flags(vec!["-ffp-contract=off", "-Wno-narrowing"]); - if !target.contains("v7") { + if !config.target.contains("v7") { command = command.add_arch_flags(vec!["faminmax", "lut", "sha3"]); } @@ -30,22 +30,33 @@ pub fn compile_c_arm( * does not work as it gets caught up with `#include_next ` * not existing... */ - if target.contains("aarch64_be") { - command = command - .set_linker( - cxx_toolchain_dir.unwrap_or("").to_string() + "/bin/aarch64_be-none-linux-gnu-g++", + if config.target.contains("aarch64_be") { + let Some(ref cxx_toolchain_dir) = config.cxx_toolchain_dir else { + panic!( + "target `{}` must specify `cxx_toolchain_dir`", + config.target ) - .set_include_paths(vec![ + }; + + let linker = if let Some(ref linker) = config.linker { + linker.to_owned() + } else { + format!("{cxx_toolchain_dir}/bin/aarch64_be-none-linux-gnu-g++") + }; + + trace!("using linker: {linker}"); + + command = command.set_linker(linker).set_include_paths(vec![ "/include", "/aarch64_be-none-linux-gnu/include", "/aarch64_be-none-linux-gnu/include/c++/14.3.1", "/aarch64_be-none-linux-gnu/include/c++/14.3.1/aarch64_be-none-linux-gnu", "/aarch64_be-none-linux-gnu/include/c++/14.3.1/backward", "/aarch64_be-none-linux-gnu/libc/usr/include", - ]); + ]); } - if !compiler.contains("clang") { + if !cpp_compiler.contains("clang") { command = command.add_extra_flag("-flax-vector-conversions"); } diff --git a/crates/intrinsic-test/src/arm/mod.rs b/crates/intrinsic-test/src/arm/mod.rs index 1a66ed4268..a2535dc6a7 100644 --- a/crates/intrinsic-test/src/arm/mod.rs +++ b/crates/intrinsic-test/src/arm/mod.rs @@ -11,7 +11,6 @@ use crate::common::gen_rust::compile_rust_programs; use crate::common::intrinsic::{Intrinsic, IntrinsicDefinition}; use crate::common::intrinsic_helpers::TypeKind; use crate::common::write_file::{write_c_testfiles, write_rust_testfiles}; -use compile::compile_c_arm; use config::{AARCH_CONFIGURATIONS, F16_FORMATTING_DEF, POLY128_OSTREAM_DEF, build_notices}; use intrinsic::ArmIntrinsicType; use json_parser::get_neon_intrinsics; @@ -51,9 +50,7 @@ impl SupportedArchitectureTest for ArmArchitectureTest { } fn build_c_file(&self) -> bool { - let compiler = self.cli_options.cpp_compiler.as_deref(); let target = &self.cli_options.target; - let cxx_toolchain_dir = self.cli_options.cxx_toolchain_dir.as_deref(); let c_target = "aarch64"; let intrinsics_name_list = write_c_testfiles( @@ -69,15 +66,7 @@ impl SupportedArchitectureTest for ArmArchitectureTest { &[POLY128_OSTREAM_DEF], ); - match compiler { - None => true, - Some(compiler) => compile_c_arm( - intrinsics_name_list.as_slice(), - compiler, - target, - cxx_toolchain_dir, - ), - } + compile::compile_c_arm(&self.cli_options, intrinsics_name_list.as_slice()) } fn build_rust_file(&self) -> bool { From 3735533a37432985a1dccb43e158c282e6e29473 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Thu, 10 Jul 2025 20:30:36 +0200 Subject: [PATCH 2/2] improve cpp compiler execution --- crates/intrinsic-test/src/arm/compile.rs | 68 ++++------ crates/intrinsic-test/src/arm/mod.rs | 4 +- crates/intrinsic-test/src/common/compile_c.rs | 124 +++++++----------- crates/intrinsic-test/src/common/gen_c.rs | 40 +++--- 4 files changed, 92 insertions(+), 144 deletions(-) diff --git a/crates/intrinsic-test/src/arm/compile.rs b/crates/intrinsic-test/src/arm/compile.rs index a90caf367c..48a8ed950e 100644 --- a/crates/intrinsic-test/src/arm/compile.rs +++ b/crates/intrinsic-test/src/arm/compile.rs @@ -1,11 +1,8 @@ use crate::common::cli::ProcessedCli; -use crate::common::compile_c::CompilationCommandBuilder; -use crate::common::gen_c::compile_c_programs; +use crate::common::compile_c::{CompilationCommandBuilder, CppCompilation}; -pub fn compile_c_arm(config: &ProcessedCli, intrinsics_name_list: &[String]) -> bool { - let Some(ref cpp_compiler) = config.cpp_compiler else { - return true; - }; +pub fn build_cpp_compilation(config: &ProcessedCli) -> Option { + let cpp_compiler = config.cpp_compiler.as_ref()?; // -ffp-contract=off emulates Rust's approach of not fusing separate mul-add operations let mut command = CompilationCommandBuilder::new() @@ -21,15 +18,12 @@ pub fn compile_c_arm(config: &ProcessedCli, intrinsics_name_list: &[String]) -> 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 ` - * not existing... - */ + if !cpp_compiler.contains("clang") { + command = command.add_extra_flag("-flax-vector-conversions"); + } + + let mut cpp_compiler = command.into_cpp_compilation(); + if config.target.contains("aarch64_be") { let Some(ref cxx_toolchain_dir) = config.cxx_toolchain_dir else { panic!( @@ -38,38 +32,20 @@ pub fn compile_c_arm(config: &ProcessedCli, intrinsics_name_list: &[String]) -> ) }; - let linker = if let Some(ref linker) = config.linker { - linker.to_owned() - } else { - format!("{cxx_toolchain_dir}/bin/aarch64_be-none-linux-gnu-g++") - }; - - trace!("using linker: {linker}"); - - command = command.set_linker(linker).set_include_paths(vec![ - "/include", - "/aarch64_be-none-linux-gnu/include", - "/aarch64_be-none-linux-gnu/include/c++/14.3.1", - "/aarch64_be-none-linux-gnu/include/c++/14.3.1/aarch64_be-none-linux-gnu", - "/aarch64_be-none-linux-gnu/include/c++/14.3.1/backward", - "/aarch64_be-none-linux-gnu/libc/usr/include", + cpp_compiler.command_mut().args([ + &format!("--sysroot={cxx_toolchain_dir}/aarch64_be-none-linux-gnu/libc"), + "--include-directory", + &format!("{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/include/c++/14.3.1"), + "--include-directory", + &format!("{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/include/c++/14.3.1/aarch64_be-none-linux-gnu"), + "-L", + &format!("{cxx_toolchain_dir}/lib/gcc/aarch64_be-none-linux-gnu/14.3.1"), + "-L", + &format!("{cxx_toolchain_dir}/aarch64_be-none-linux-gnu/libc/usr/lib"), + "-B", + &format!("{cxx_toolchain_dir}/lib/gcc/aarch64_be-none-linux-gnu/14.3.1"), ]); } - if !cpp_compiler.contains("clang") { - command = command.add_extra_flag("-flax-vector-conversions"); - } - - let compiler_commands = intrinsics_name_list - .iter() - .map(|intrinsic_name| { - command - .clone() - .set_input_name(intrinsic_name) - .set_output_name(intrinsic_name) - .make_string() - }) - .collect::>(); - - compile_c_programs(&compiler_commands) + Some(cpp_compiler) } diff --git a/crates/intrinsic-test/src/arm/mod.rs b/crates/intrinsic-test/src/arm/mod.rs index a2535dc6a7..18530e7e6b 100644 --- a/crates/intrinsic-test/src/arm/mod.rs +++ b/crates/intrinsic-test/src/arm/mod.rs @@ -7,6 +7,7 @@ mod types; use crate::common::SupportedArchitectureTest; use crate::common::cli::ProcessedCli; use crate::common::compare::compare_outputs; +use crate::common::gen_c::compile_c_programs; use crate::common::gen_rust::compile_rust_programs; use crate::common::intrinsic::{Intrinsic, IntrinsicDefinition}; use crate::common::intrinsic_helpers::TypeKind; @@ -66,7 +67,8 @@ impl SupportedArchitectureTest for ArmArchitectureTest { &[POLY128_OSTREAM_DEF], ); - compile::compile_c_arm(&self.cli_options, intrinsics_name_list.as_slice()) + let pipeline = compile::build_cpp_compilation(&self.cli_options).unwrap(); + compile_c_programs(&pipeline, &intrinsics_name_list) } fn build_rust_file(&self) -> bool { diff --git a/crates/intrinsic-test/src/common/compile_c.rs b/crates/intrinsic-test/src/common/compile_c.rs index aebb7b111e..8e2eb8e0f4 100644 --- a/crates/intrinsic-test/src/common/compile_c.rs +++ b/crates/intrinsic-test/src/common/compile_c.rs @@ -5,11 +5,7 @@ pub struct CompilationCommandBuilder { cxx_toolchain_dir: Option, arch_flags: Vec, optimization: String, - include_paths: Vec, project_root: Option, - output: String, - input: String, - linker: Option, extra_flags: Vec, } @@ -21,11 +17,7 @@ impl CompilationCommandBuilder { 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(), } } @@ -57,37 +49,12 @@ impl CompilationCommandBuilder { 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 - } - pub fn add_extra_flags(mut self, flags: Vec<&str>) -> Self { let mut flags: Vec = flags.into_iter().map(|f| f.to_string()).collect(); self.extra_flags.append(&mut flags); @@ -100,55 +67,56 @@ impl CompilationCommandBuilder { } impl CompilationCommandBuilder { - pub fn make_string(self) -> String { - let arch_flags = self.arch_flags.join("+"); + pub fn into_cpp_compilation(self) -> CppCompilation { + let mut cpp_compiler = std::process::Command::new(self.compiler); + + if let Some(project_root) = self.project_root { + cpp_compiler.current_dir(project_root); + } + let flags = std::env::var("CPPFLAGS").unwrap_or("".into()); - let project_root = self.project_root.unwrap_or_default(); - let project_root_str = project_root.as_str(); - let mut output = self.output.clone(); - if self.linker.is_some() { - output += ".o" - }; - let mut command = format!( - "{} {flags} -march={arch_flags} \ - -O{} \ - -o {project_root}/{} \ - {project_root}/{}.cpp", - self.compiler, self.optimization, output, self.input, - ); - - command = command + " " + self.extra_flags.join(" ").as_str(); + cpp_compiler.args(flags.split_whitespace()); + + cpp_compiler.arg(format!("-march={}", self.arch_flags.join("+"))); + + cpp_compiler.arg(format!("-O{}", self.optimization)); + + cpp_compiler.args(self.extra_flags); if let Some(target) = &self.target { - command = command + " --target=" + target; + cpp_compiler.arg(format!("--target={target}")); } - if let (Some(linker), Some(cxx_toolchain_dir)) = (&self.linker, &self.cxx_toolchain_dir) { - let include_args = self - .include_paths - .iter() - .map(|path| "--include-directory=".to_string() + cxx_toolchain_dir + path) - .collect::>() - .join(" "); - - command = command - + " -c " - + include_args.as_str() - + " && " - + linker - + " " - + project_root_str - + "/" - + &output - + " -o " - + project_root_str - + "/" - + &self.output - + " && rm " - + project_root_str - + "/" - + &output; - } - command + CppCompilation(cpp_compiler) + } +} + +pub struct CppCompilation(std::process::Command); + +fn clone_command(command: &std::process::Command) -> std::process::Command { + let mut cmd = std::process::Command::new(command.get_program()); + if let Some(current_dir) = command.get_current_dir() { + cmd.current_dir(current_dir); + } + cmd.args(command.get_args()); + + for (key, val) in command.get_envs() { + cmd.env(key, val.unwrap_or_default()); + } + + cmd +} + +impl CppCompilation { + pub fn command_mut(&mut self) -> &mut std::process::Command { + &mut self.0 + } + + pub fn run(&self, inputs: &[String], output: &str) -> std::io::Result { + let mut cmd = clone_command(&self.0); + cmd.args(inputs); + cmd.args(["-o", output]); + + cmd.output() } } diff --git a/crates/intrinsic-test/src/common/gen_c.rs b/crates/intrinsic-test/src/common/gen_c.rs index 1cfb66c39b..84167f2f4a 100644 --- a/crates/intrinsic-test/src/common/gen_c.rs +++ b/crates/intrinsic-test/src/common/gen_c.rs @@ -1,7 +1,8 @@ use itertools::Itertools; use rayon::prelude::*; use std::collections::BTreeMap; -use std::process::Command; + +use crate::common::compile_c::CppCompilation; use super::argument::Argument; use super::indentation::Indentation; @@ -62,29 +63,30 @@ int main(int argc, char **argv) {{ ) } -pub fn compile_c_programs(compiler_commands: &[String]) -> bool { - compiler_commands +pub fn compile_c_programs(pipeline: &CppCompilation, intrinsics: &[String]) -> bool { + intrinsics .par_iter() - .map(|compiler_command| { - let output = Command::new("sh").arg("-c").arg(compiler_command).output(); - if let Ok(output) = output { - if output.status.success() { - true - } else { - error!( - "Failed to compile code for intrinsics: \n\nstdout:\n{}\n\nstderr:\n{}", + .map( + |intrinsic| match pipeline.run(&[format!("{intrinsic}.cpp")], intrinsic) { + Ok(output) if output.status.success() => Ok(()), + Ok(output) => { + let msg = format!( + "Failed to compile code for intrinsic `{intrinsic}`: \n\nstdout:\n{}\n\nstderr:\n{}", std::str::from_utf8(&output.stdout).unwrap_or(""), std::str::from_utf8(&output.stderr).unwrap_or("") ); - false + error!("{msg}"); + + Err(msg) } - } else { - error!("Command failed: {output:#?}"); - false - } - }) - .find_any(|x| !x) - .is_none() + Err(e) => { + error!("command for `{intrinsic}` failed with IO error: {e:?}"); + Err(e.to_string()) + } + }, + ) + .collect::>() + .is_ok() } // Creates directory structure and file path mappings