Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Make the c feature for compiler-builtins opt-in instead of inferred" #103732

Merged
merged 1 commit into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,6 @@ changelog-seen = 2
# 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).
#optimized-compiler-builtins = false

# Indicates whether the native libraries linked into Cargo will be statically
# linked or not.
#cargo-native-static = false
Expand Down
15 changes: 5 additions & 10 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,9 @@ 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.
// `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).
//
// 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
Expand All @@ -310,15 +312,8 @@ 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_c_feature = if builder.config.optimized_compiler_builtins {
if !builder.is_rust_llvm(target) {
panic!(
"need a managed LLVM submodule for optimized intrinsics support; unset `llvm-config` or `optimized-compiler-builtins`"
);
}

builder.update_submodule(&Path::new("src").join("llvm-project"));
let compiler_builtins_root = builder.src.join("src/llvm-project/compiler-rt");
let compiler_builtins_root = builder.src.join("src/llvm-project/compiler-rt");
let compiler_builtins_c_feature = if compiler_builtins_root.exists() {
// 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);
Expand Down
4 changes: 0 additions & 4 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ pub struct Config {
pub color: Color,
pub patch_binaries_for_nix: bool,
pub stage0_metadata: Stage0Metadata,
/// Whether to use the `c` feature of the `compiler_builtins` crate.
pub optimized_compiler_builtins: bool,

pub on_fail: Option<String>,
pub stage: u32,
Expand Down Expand Up @@ -624,7 +622,6 @@ define_config! {
bench_stage: Option<u32> = "bench-stage",
patch_binaries_for_nix: Option<bool> = "patch-binaries-for-nix",
metrics: Option<bool> = "metrics",
optimized_compiler_builtins: Option<bool> = "optimized-compiler-builtins",
}
}

Expand Down Expand Up @@ -1013,7 +1010,6 @@ impl Config {
set(&mut config.print_step_timings, build.print_step_timings);
set(&mut config.print_step_rusage, build.print_step_rusage);
set(&mut config.patch_binaries_for_nix, build.patch_binaries_for_nix);
set(&mut config.optimized_compiler_builtins, build.optimized_compiler_builtins);

config.verbose = cmp::max(config.verbose, flags.verbose);

Expand Down
32 changes: 17 additions & 15 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1847,21 +1847,23 @@ fn add_env(builder: &Builder<'_>, cmd: &mut Command, target: TargetSelection) {
///
/// Returns whether the files were actually copied.
fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir: &Path) -> bool {
if !builder.is_rust_llvm(target) {
// If the LLVM was externally provided, then we don't currently copy
// artifacts into the sysroot. This is not necessarily the right
// choice (in particular, it will require the LLVM dylib to be in
// the linker's load path at runtime), but the common use case for
// external LLVMs is distribution provided LLVMs, and in that case
// they're usually in the standard search path (e.g., /usr/lib) and
// copying them here is going to cause problems as we may end up
// with the wrong files and isn't what distributions want.
//
// This behavior may be revisited in the future though.
//
// If the LLVM is coming from ourselves (just from CI) though, we
// still want to install it, as it otherwise won't be available.
return false;
if let Some(config) = builder.config.target_config.get(&target) {
if config.llvm_config.is_some() && !builder.config.llvm_from_ci {
// If the LLVM was externally provided, then we don't currently copy
// artifacts into the sysroot. This is not necessarily the right
// choice (in particular, it will require the LLVM dylib to be in
// the linker's load path at runtime), but the common use case for
// external LLVMs is distribution provided LLVMs, and in that case
// they're usually in the standard search path (e.g., /usr/lib) and
// copying them here is going to cause problems as we may end up
// with the wrong files and isn't what distributions want.
//
// This behavior may be revisited in the future though.
//
// If the LLVM is coming from ourselves (just from CI) though, we
// still want to install it, as it otherwise won't be available.
return false;
}
}

// On macOS, rustc (and LLVM tools) link to an unversioned libLLVM.dylib
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,4 @@ 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
2 changes: 0 additions & 2 deletions src/ci/docker/host-x86_64/dist-various-2/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,4 @@ ENV RUST_CONFIGURE_ARGS --enable-extended --enable-lld --disable-docs \
--set target.wasm32-wasi.wasi-root=/wasm32-wasi \
--musl-root-armv7=/musl-armv7

ENV EXTERNAL_LLVM 1

ENV SCRIPT python3 ../x.py dist --host='' --target $TARGETS
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ 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 \
Expand Down
1 change: 0 additions & 1 deletion src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ 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 \
Expand Down
5 changes: 0 additions & 5 deletions src/ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.codegen-units-std=1"
# 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.
if [ "$EXTERNAL_LLVM" = "" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set build.optimized-compiler-builtins"
fi

if [ "$DIST_SRC" = "" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --disable-dist-src"
fi
Expand Down