From d56fa5dc775baaef2747356f313441ea8ef0d414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 30 Sep 2024 16:17:50 +0000 Subject: [PATCH 1/4] enable rustc debug assertions on -alt builds llvm assertions are already enabled and debug assertions are also useful --- src/ci/run.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ci/run.sh b/src/ci/run.sh index 6980d8220e574..ed1583ae8c102 100755 --- a/src/ci/run.sh +++ b/src/ci/run.sh @@ -134,6 +134,11 @@ if [ "$DEPLOY$DEPLOY_ALT" = "1" ]; then CODEGEN_BACKENDS="${CODEGEN_BACKENDS:-llvm}" RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.codegen-backends=$CODEGEN_BACKENDS" + + # Unless explicitly disabled, we want rustc debug assertions on the -alt builds + if [ "$DEPLOY_ALT" != "" ] && [ "$NO_DEBUG_ASSERTIONS" = "" ]; then + RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-debug-assertions" + fi else # We almost always want debug assertions enabled, but sometimes this takes too # long for too little benefit, so we just turn them off. From 8882ec596637b7d9a2c1834e728b51512fc5626c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 25 Mar 2025 11:25:52 +0000 Subject: [PATCH 2/4] add optional build components for the test environment --- src/tools/opt-dist/src/environment.rs | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/tools/opt-dist/src/environment.rs b/src/tools/opt-dist/src/environment.rs index 90d0ca717b25c..647decb57d33e 100644 --- a/src/tools/opt-dist/src/environment.rs +++ b/src/tools/opt-dist/src/environment.rs @@ -25,6 +25,27 @@ pub struct Environment { prebuilt_rustc_perf: Option, use_bolt: bool, shared_llvm: bool, + /// Additional configuration that bootstrap needs to know only when running tests. + #[builder(default)] + test_config: TestConfig, +} + +/// Builds have optional components, and their presence/absence can enable/disable a subset of +/// tests. When testing the optimized artifacts, bootstrap needs to know about these enabled +/// components to run the expected subset. This structure holds the known components where this +/// matters: currently only whether the build to test is using debug assertions. +/// +/// FIXME: ultimately, this is a temporary band-aid, and opt-dist should be more transparent to the +/// CI config and bootstrap optional components: bootstrap has default values, combinations of flags +/// that cascade into others, etc logic that we'd have to duplicate here otherwise. It's more +/// sensible for opt-dist to never know about the config apart from the minimal set of paths +/// required to configure stage0 tests. +#[derive(Builder, Default, Clone, Debug)] +pub struct TestConfig { + /// Whether the build under test is explicitly using `--enable-debug-assertions`. + /// Note that this flag can be implied from others, like `rust.debug`, and we do not handle any + /// of these subtleties and defaults here, as per the FIXME above. + pub enable_debug_assertions: bool, } impl Environment { @@ -101,6 +122,21 @@ impl Environment { pub fn benchmark_cargo_config(&self) -> &[String] { &self.benchmark_cargo_config } + + pub fn test_config(&self) -> &TestConfig { + &self.test_config + } +} + +impl TestConfig { + /// Returns the test config matching the given `RUST_CONFIGURE_ARGS` for the known optional + /// components for tests. This is obviously extremely fragile and we'd rather opt-dist not + /// handle any optional components. + pub fn from_configure_args(configure_args: &str) -> TestConfig { + let enable_debug_assertions = + configure_args.split(" ").find(|part| *part == "--enable-debug-assertions").is_some(); + TestConfig { enable_debug_assertions } + } } /// What is the extension of binary executables on this platform? From 6a637a0dcc49122b7155b799114de89640b24c0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 25 Mar 2025 11:27:09 +0000 Subject: [PATCH 3/4] parse configure args for debug assertions and use them in the test env this way opt-dist respects debug-assertions on alt builds, so that bootstrap runs the correct set of tests in stage0 post optimization tests. --- src/tools/opt-dist/src/main.rs | 13 +++++++++++++ src/tools/opt-dist/src/tests.rs | 4 +++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/tools/opt-dist/src/main.rs b/src/tools/opt-dist/src/main.rs index ac5d294f07ed1..837758ee04b64 100644 --- a/src/tools/opt-dist/src/main.rs +++ b/src/tools/opt-dist/src/main.rs @@ -1,6 +1,7 @@ use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; use clap::Parser; +use environment::TestConfig; use log::LevelFilter; use utils::io; @@ -148,6 +149,11 @@ fn create_environment(args: Args) -> anyhow::Result<(Environment, Vec)> let is_aarch64 = target_triple.starts_with("aarch64"); + // Parse the optional build components that impact the test environment. + let rust_configure_args = std::env::var("RUST_CONFIGURE_ARGS") + .expect("RUST_CONFIGURE_ARGS environment variable missing"); + let test_config = TestConfig::from_configure_args(&rust_configure_args); + let checkout_dir = Utf8PathBuf::from("/checkout"); let env = EnvironmentBuilder::default() .host_tuple(target_triple) @@ -160,6 +166,7 @@ fn create_environment(args: Args) -> anyhow::Result<(Environment, Vec)> // FIXME: Enable bolt for aarch64 once it's fixed upstream. Broken as of December 2024. .use_bolt(!is_aarch64) .skipped_tests(vec![]) + .test_config(test_config) .build()?; (env, shared.build_args) @@ -168,6 +175,11 @@ fn create_environment(args: Args) -> anyhow::Result<(Environment, Vec)> let target_triple = std::env::var("PGO_HOST").expect("PGO_HOST environment variable missing"); + // Parse the optional build components that impact the test environment. + let rust_configure_args = std::env::var("RUST_CONFIGURE_ARGS") + .expect("RUST_CONFIGURE_ARGS environment variable missing"); + let test_config = TestConfig::from_configure_args(&rust_configure_args); + let checkout_dir: Utf8PathBuf = std::env::current_dir()?.try_into()?; let env = EnvironmentBuilder::default() .host_tuple(target_triple) @@ -179,6 +191,7 @@ fn create_environment(args: Args) -> anyhow::Result<(Environment, Vec)> .shared_llvm(false) .use_bolt(false) .skipped_tests(vec![]) + .test_config(test_config) .build()?; (env, shared.build_args) diff --git a/src/tools/opt-dist/src/tests.rs b/src/tools/opt-dist/src/tests.rs index 53ce772fa7792..6cd20f0048e0a 100644 --- a/src/tools/opt-dist/src/tests.rs +++ b/src/tools/opt-dist/src/tests.rs @@ -72,6 +72,7 @@ change-id = 115898 [rust] channel = "{channel}" verbose-tests = true +debug-assertions = {debug_assertions} [build] rustc = "{rustc}" @@ -83,7 +84,8 @@ llvm-config = "{llvm_config}" "#, rustc = rustc_path.to_string().replace('\\', "/"), cargo = cargo_path.to_string().replace('\\', "/"), - llvm_config = llvm_config.to_string().replace('\\', "/") + llvm_config = llvm_config.to_string().replace('\\', "/"), + debug_assertions = env.test_config().enable_debug_assertions, ); log::info!("Using following `bootstrap.toml` for running tests:\n{config_content}"); From b31a6a91d84908310acc66acd95ea8946c25d108 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Wed, 26 Mar 2025 17:33:33 +0000 Subject: [PATCH 4/4] address review comments --- src/tools/opt-dist/src/environment.rs | 4 ++-- src/tools/opt-dist/src/tests.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/opt-dist/src/environment.rs b/src/tools/opt-dist/src/environment.rs index 647decb57d33e..abb0fcf6fdd23 100644 --- a/src/tools/opt-dist/src/environment.rs +++ b/src/tools/opt-dist/src/environment.rs @@ -45,7 +45,7 @@ pub struct TestConfig { /// Whether the build under test is explicitly using `--enable-debug-assertions`. /// Note that this flag can be implied from others, like `rust.debug`, and we do not handle any /// of these subtleties and defaults here, as per the FIXME above. - pub enable_debug_assertions: bool, + pub rust_debug_assertions: bool, } impl Environment { @@ -135,7 +135,7 @@ impl TestConfig { pub fn from_configure_args(configure_args: &str) -> TestConfig { let enable_debug_assertions = configure_args.split(" ").find(|part| *part == "--enable-debug-assertions").is_some(); - TestConfig { enable_debug_assertions } + TestConfig { rust_debug_assertions: enable_debug_assertions } } } diff --git a/src/tools/opt-dist/src/tests.rs b/src/tools/opt-dist/src/tests.rs index 6cd20f0048e0a..943acac95d024 100644 --- a/src/tools/opt-dist/src/tests.rs +++ b/src/tools/opt-dist/src/tests.rs @@ -85,7 +85,7 @@ llvm-config = "{llvm_config}" rustc = rustc_path.to_string().replace('\\', "/"), cargo = cargo_path.to_string().replace('\\', "/"), llvm_config = llvm_config.to_string().replace('\\', "/"), - debug_assertions = env.test_config().enable_debug_assertions, + debug_assertions = env.test_config().rust_debug_assertions, ); log::info!("Using following `bootstrap.toml` for running tests:\n{config_content}");