From 72e60bceea63b24caa0dd310569a53fbc23ca0f6 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 12 Aug 2022 12:06:31 +0200 Subject: [PATCH] valgrind fix for newer intel processors --- crates/cli/src/build.rs | 6 ------ crates/cli/src/lib.rs | 12 ------------ crates/cli/tests/cli_run.rs | 11 ++++++----- crates/compiler/build/src/link.rs | 18 ++++-------------- crates/linker/src/lib.rs | 9 +-------- 5 files changed, 11 insertions(+), 45 deletions(-) diff --git a/crates/cli/src/build.rs b/crates/cli/src/build.rs index 11129068b0..cf9c8038f6 100644 --- a/crates/cli/src/build.rs +++ b/crates/cli/src/build.rs @@ -46,7 +46,6 @@ pub fn build_file<'a>( link_type: LinkType, linking_strategy: LinkingStrategy, precompiled: bool, - target_valgrind: bool, threading: Threading, wasm_dev_stack_bytes: Option, ) -> Result> { @@ -152,7 +151,6 @@ pub fn build_file<'a>( target, exposed_values, exposed_closure_types, - target_valgrind, ); // TODO try to move as much of this linking as possible to the precompiled @@ -377,7 +375,6 @@ fn spawn_rebuild_thread( target: &Triple, exported_symbols: Vec, exported_closure_types: Vec, - target_valgrind: bool, ) -> std::thread::JoinHandle { let thread_local_target = target.clone(); std::thread::spawn(move || { @@ -395,7 +392,6 @@ fn spawn_rebuild_thread( &thread_local_target, host_input_path.as_path(), None, - target_valgrind, ); preprocess_host_wasm32(host_dest.as_path(), &preprocessed_host_path); @@ -408,7 +404,6 @@ fn spawn_rebuild_thread( preprocessed_host_path.as_path(), exported_symbols, exported_closure_types, - target_valgrind, ); } LinkingStrategy::Legacy => { @@ -417,7 +412,6 @@ fn spawn_rebuild_thread( &thread_local_target, host_input_path.as_path(), None, - target_valgrind, ); } } diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index cab9a56900..b4e43f3474 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -57,7 +57,6 @@ pub const FLAG_TARGET: &str = "target"; pub const FLAG_TIME: &str = "time"; pub const FLAG_LINKER: &str = "linker"; pub const FLAG_PRECOMPILED: &str = "precompiled-host"; -pub const FLAG_VALGRIND: &str = "valgrind"; pub const FLAG_CHECK: &str = "check"; pub const FLAG_WASM_STACK_SIZE_KB: &str = "wasm-stack-size-kb"; pub const ROC_FILE: &str = "ROC_FILE"; @@ -96,11 +95,6 @@ pub fn build_app<'a>() -> Command<'a> { .help("Store LLVM debug information in the generated program.") .required(false); - let flag_valgrind = Arg::new(FLAG_VALGRIND) - .long(FLAG_VALGRIND) - .help("Some assembly instructions are not supported by valgrind, this flag prevents those from being output when building the host.") - .required(false); - let flag_time = Arg::new(FLAG_TIME) .long(FLAG_TIME) .help("Prints detailed compilation time information.") @@ -152,7 +146,6 @@ pub fn build_app<'a>() -> Command<'a> { .arg(flag_time.clone()) .arg(flag_linker.clone()) .arg(flag_precompiled.clone()) - .arg(flag_valgrind.clone()) .arg(flag_wasm_stack_size_kb.clone()) .arg( Arg::new(FLAG_TARGET) @@ -192,7 +185,6 @@ pub fn build_app<'a>() -> Command<'a> { .arg(flag_time.clone()) .arg(flag_linker.clone()) .arg(flag_precompiled.clone()) - .arg(flag_valgrind.clone()) .arg( Arg::new(ROC_FILE) .help("The .roc file for the main module") @@ -215,7 +207,6 @@ pub fn build_app<'a>() -> Command<'a> { .arg(flag_time.clone()) .arg(flag_linker.clone()) .arg(flag_precompiled.clone()) - .arg(flag_valgrind.clone()) .arg(roc_file_to_run.clone()) .arg(args_for_app.clone()) ) @@ -282,7 +273,6 @@ pub fn build_app<'a>() -> Command<'a> { .arg(flag_time) .arg(flag_linker) .arg(flag_precompiled) - .arg(flag_valgrind) .arg(roc_file_to_run.required(false)) .arg(args_for_app); @@ -531,7 +521,6 @@ pub fn build( .and_then(|s| s.parse::().ok()) .map(|x| x * 1024); - let target_valgrind = matches.is_present(FLAG_VALGRIND); let res_binary_path = build_file( &arena, &triple, @@ -542,7 +531,6 @@ pub fn build( link_type, linking_strategy, precompiled, - target_valgrind, threading, wasm_dev_stack_bytes, ); diff --git a/crates/cli/tests/cli_run.rs b/crates/cli/tests/cli_run.rs index 13bfcd16fa..ed5f2e777e 100644 --- a/crates/cli/tests/cli_run.rs +++ b/crates/cli/tests/cli_run.rs @@ -25,7 +25,6 @@ mod cli_run { use strum_macros::EnumIter; const OPTIMIZE_FLAG: &str = concatcp!("--", roc_cli::FLAG_OPTIMIZE); - const VALGRIND_FLAG: &str = concatcp!("--", roc_cli::FLAG_VALGRIND); const LINKER_FLAG: &str = concatcp!("--", roc_cli::FLAG_LINKER); const CHECK_FLAG: &str = concatcp!("--", roc_cli::FLAG_CHECK); const PRECOMPILED_HOST: &str = concatcp!("--", roc_cli::FLAG_PRECOMPILED, "=true"); @@ -137,14 +136,16 @@ mod cli_run { expected_ending: &str, use_valgrind: bool, ) { + // valgrind does not yet support avx512 instructions, see #1963. + // we can't enable this only when testing with valgrind because of host re-use between tests + if is_x86_feature_detected!("avx512f") { + std::env::set_var("NO_AVX512", "1"); + } + for cli_mode in CliMode::iter() { let flags = { let mut vec = flags.to_vec(); - if use_valgrind { - vec.push(VALGRIND_FLAG); - } - vec.push("--max-threads=1"); vec.into_iter() diff --git a/crates/compiler/build/src/link.rs b/crates/compiler/build/src/link.rs index 14654c503a..dd829846c0 100644 --- a/crates/compiler/build/src/link.rs +++ b/crates/compiler/build/src/link.rs @@ -113,7 +113,6 @@ pub fn build_zig_host_native( target: &str, opt_level: OptLevel, shared_lib_path: Option<&Path>, - _target_valgrind: bool, ) -> Output { let mut command = Command::new(&zig_executable()); command @@ -149,13 +148,10 @@ pub fn build_zig_host_native( target, ]); - // use single threaded testing for cli_run and enable this code if valgrind fails with unhandled instruction bytes, see #1963. - /*if target_valgrind { - command.args(&[ - "-mcpu", - "x86_64" - ]); - }*/ + // valgrind does not yet support avx512 instructions, see #1963. + if env::var("NO_AVX512").is_ok() { + command.args(&["-mcpu", "x86_64"]); + } if matches!(opt_level, OptLevel::Optimize) { command.args(&["-O", "ReleaseSafe"]); @@ -177,7 +173,6 @@ pub fn build_zig_host_native( target: &str, opt_level: OptLevel, shared_lib_path: Option<&Path>, - _target_valgrind: bool, ) -> Output { let mut command = Command::new(&zig_executable()); command @@ -234,7 +229,6 @@ pub fn build_zig_host_native( opt_level: OptLevel, shared_lib_path: Option<&Path>, // For compatibility with the non-macOS def above. Keep these in sync. - _target_valgrind: bool, ) -> Output { use serde_json::Value; @@ -463,7 +457,6 @@ pub fn rebuild_host( target: &Triple, host_input_path: &Path, shared_lib_path: Option<&Path>, - target_valgrind: bool, ) -> PathBuf { let c_host_src = host_input_path.with_file_name("host.c"); let c_host_dest = host_input_path.with_file_name("c_host.o"); @@ -535,7 +528,6 @@ pub fn rebuild_host( "native", opt_level, shared_lib_path, - target_valgrind, ) } Architecture::X86_32(_) => { @@ -549,7 +541,6 @@ pub fn rebuild_host( "i386-linux-musl", opt_level, shared_lib_path, - target_valgrind, ) } @@ -564,7 +555,6 @@ pub fn rebuild_host( target_zig_str(target), opt_level, shared_lib_path, - target_valgrind, ) } _ => panic!("Unsupported architecture {:?}", target.architecture), diff --git a/crates/linker/src/lib.rs b/crates/linker/src/lib.rs index 6423bc1925..e5d12fbfe4 100644 --- a/crates/linker/src/lib.rs +++ b/crates/linker/src/lib.rs @@ -79,17 +79,10 @@ pub fn build_and_preprocess_host( preprocessed_host_path: &Path, exposed_to_host: Vec, exported_closure_types: Vec, - target_valgrind: bool, ) { let dummy_lib = host_input_path.with_file_name("libapp.so"); generate_dynamic_lib(target, exposed_to_host, exported_closure_types, &dummy_lib); - rebuild_host( - opt_level, - target, - host_input_path, - Some(&dummy_lib), - target_valgrind, - ); + rebuild_host(opt_level, target, host_input_path, Some(&dummy_lib)); let dynhost = host_input_path.with_file_name("dynhost"); let metadata = host_input_path.with_file_name("metadata"); // let prehost = host_input_path.with_file_name("preprocessedhost");