From 6a409ddbe40f55dd5f89442d24548b42b0822f60 Mon Sep 17 00:00:00 2001 From: jyn Date: Mon, 25 Dec 2023 07:46:12 -0500 Subject: [PATCH] add a new `optimized_compiler_builtins` option in particular, this makes the `c` feature for compiler-builtins an explicit opt-in, rather than silently detected by whether `llvm-project` is checked out on disk. exposing this is necessary because the `cc` crate doesn't support cross-compiling to MSVC, and we want people to be able to run `x check --target foo` regardless of whether they have a c toolchain available. this also uses the new option in CI, where we *do* want to optimize compiler_builtins. the new option is off by default for the `dev` channel and on otherwise. --- config.example.toml | 8 +++++++ src/bootstrap/src/core/build_steps/compile.rs | 21 ++++++++++++++----- src/bootstrap/src/core/config/config.rs | 6 ++++++ src/bootstrap/src/utils/change_tracker.rs | 5 +++++ .../disabled/dist-x86_64-haiku/Dockerfile | 2 ++ .../host-x86_64/x86_64-gnu-llvm-17/Dockerfile | 1 + src/ci/run.sh | 9 ++++++++ 7 files changed, 47 insertions(+), 5 deletions(-) diff --git a/config.example.toml b/config.example.toml index f1ea6bac3ca..a5ef4022d39 100644 --- a/config.example.toml +++ b/config.example.toml @@ -339,6 +339,14 @@ # on this runtime, such as `-C profile-generate` or `-C instrument-coverage`). #profiler = false +# Use the optimized LLVM C intrinsics for `compiler_builtins`, rather than Rust intrinsics. +# Requires the LLVM submodule to be managed by bootstrap (i.e. not external) so that `compiler-rt` +# sources are available. +# +# Setting this to `false` generates slower code, but removes the requirement for a C toolchain in +# order to run `x check`. +#optimized-compiler-builtins = if rust.channel == "dev" { false } else { true } + # Indicates whether the native libraries linked into Cargo will be statically # linked or not. #cargo-native-static = false diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index dbb64583d56..1769e94473a 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -382,9 +382,7 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car // Determine if we're going to compile in optimized C intrinsics to // the `compiler-builtins` crate. These intrinsics live in LLVM's - // `compiler-rt` repository, but our `src/llvm-project` submodule isn't - // always checked out, so we need to conditionally look for this. (e.g. if - // an external LLVM is used we skip the LLVM submodule checkout). + // `compiler-rt` repository. // // Note that this shouldn't affect the correctness of `compiler-builtins`, // but only its speed. Some intrinsics in C haven't been translated to Rust @@ -395,8 +393,21 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car // If `compiler-rt` is available ensure that the `c` feature of the // `compiler-builtins` crate is enabled and it's configured to learn where // `compiler-rt` is located. - let compiler_builtins_root = builder.src.join("src/llvm-project/compiler-rt"); - let compiler_builtins_c_feature = if compiler_builtins_root.exists() { + let compiler_builtins_c_feature = if builder.config.optimized_compiler_builtins { + // NOTE: this interacts strangely with `llvm-has-rust-patches`. In that case, we enforce `submodules = false`, so this is a no-op. + // But, the user could still decide to manually use an in-tree submodule. + // + // NOTE: if we're using system llvm, we'll end up building a version of `compiler-rt` that doesn't match the LLVM we're linking to. + // That's probably ok? At least, the difference wasn't enforced before. There's a comment in + // the compiler_builtins build script that makes me nervous, though: + // https://github.com/rust-lang/compiler-builtins/blob/31ee4544dbe47903ce771270d6e3bea8654e9e50/build.rs#L575-L579 + builder.update_submodule(&Path::new("src").join("llvm-project")); + let compiler_builtins_root = builder.src.join("src/llvm-project/compiler-rt"); + if !compiler_builtins_root.exists() { + panic!( + "need LLVM sources available to build `compiler-rt`, but they weren't present; consider enabling `build.submodules = true` or disabling `optimized-compiler-builtins`" + ); + } // Note that `libprofiler_builtins/build.rs` also computes this so if // you're changing something here please also change that. cargo.env("RUST_COMPILER_RT_ROOT", &compiler_builtins_root); diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index a997eccb39a..8716ba53903 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -178,6 +178,8 @@ pub struct Config { pub patch_binaries_for_nix: Option, pub stage0_metadata: Stage0Metadata, pub android_ndk: Option, + /// Whether to use the `c` feature of the `compiler_builtins` crate. + pub optimized_compiler_builtins: bool, pub stdout_is_tty: bool, pub stderr_is_tty: bool, @@ -848,6 +850,7 @@ struct Build { // NOTE: only parsed by bootstrap.py, `--feature build-metrics` enables metrics unconditionally metrics: Option = "metrics", android_ndk: Option = "android-ndk", + optimized_compiler_builtins: Option = "optimized-compiler-builtins", } } @@ -1396,6 +1399,7 @@ fn get_table(option: &str) -> Result { // This field is only used by bootstrap.py metrics: _, android_ndk, + optimized_compiler_builtins, } = toml.build.unwrap_or_default(); if let Some(file_build) = build { @@ -1916,6 +1920,8 @@ fn get_table(option: &str) -> Result { config.rust_debuginfo_level_std = with_defaults(debuginfo_level_std); config.rust_debuginfo_level_tools = with_defaults(debuginfo_level_tools); config.rust_debuginfo_level_tests = debuginfo_level_tests.unwrap_or(DebuginfoLevel::None); + config.optimized_compiler_builtins = + optimized_compiler_builtins.unwrap_or(config.channel != "dev"); let download_rustc = config.download_rustc_commit.is_some(); // See https://github.com/rust-lang/compiler-team/issues/326 diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 25efa5079c8..327b4674acf 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -106,4 +106,9 @@ pub fn find_recent_config_change_ids(current_id: usize) -> Vec { severity: ChangeSeverity::Info, summary: "The dist.missing-tools config option was deprecated, as it was unused. If you are using it, remove it from your config, it will be removed soon.", }, + ChangeInfo { + change_id: 102579, + severity: ChangeSeverity::Warning, + summary: "A new `optimized-compiler-builtins` option has been introduced. Whether to build llvm's `compiler-rt` from source is no longer implicitly controlled by git state. See the PR for more details.", + }, ]; diff --git a/src/ci/docker/host-x86_64/disabled/dist-x86_64-haiku/Dockerfile b/src/ci/docker/host-x86_64/disabled/dist-x86_64-haiku/Dockerfile index 5ddd3f18039..637b5fa22f9 100644 --- a/src/ci/docker/host-x86_64/disabled/dist-x86_64-haiku/Dockerfile +++ b/src/ci/docker/host-x86_64/disabled/dist-x86_64-haiku/Dockerfile @@ -47,4 +47,6 @@ ENV RUST_CONFIGURE_ARGS --disable-jemalloc \ --set=$TARGET.cc=x86_64-unknown-haiku-gcc \ --set=$TARGET.cxx=x86_64-unknown-haiku-g++ \ --set=$TARGET.llvm-config=/bin/llvm-config-haiku +ENV EXTERNAL_LLVM 1 + ENV SCRIPT python3 ../x.py dist --host=$HOST --target=$HOST diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile index f1d6b9a4ef2..fe30a953441 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile @@ -40,6 +40,7 @@ RUN sh /scripts/sccache.sh # We are disabling CI LLVM since this builder is intentionally using a host # LLVM, rather than the typical src/llvm-project LLVM. ENV NO_DOWNLOAD_CI_LLVM 1 +ENV EXTERNAL_LLVM 1 # Using llvm-link-shared due to libffi issues -- see #34486 ENV RUST_CONFIGURE_ARGS \ diff --git a/src/ci/run.sh b/src/ci/run.sh index e48fac1a087..e9c274ca4e3 100755 --- a/src/ci/run.sh +++ b/src/ci/run.sh @@ -80,6 +80,15 @@ fi # space required for CI artifacts. RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --dist-compression-formats=xz" +# Enable the `c` feature for compiler_builtins, but only when the `compiler-rt` source is available +# (to avoid spending a lot of time cloning llvm) +if [ "$EXTERNAL_LLVM" = "" ]; then + RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set build.optimized-compiler-builtins" +elif [ "$DEPLOY$DEPLOY_ALT" = "1" ]; then + echo "error: dist builds should always use optimized compiler-rt!" >&2 + exit 1 +fi + if [ "$DIST_SRC" = "" ]; then RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --disable-dist-src" fi