Skip to content

Commit

Permalink
Rollup merge of rust-lang#104748 - lqd:download_lld, r=jyn514
Browse files Browse the repository at this point in the history
Ensure `lld` is supported with `download-ci-llvm`

This PR:
- ensures LLD's step in bootstrap's dist, but it's not strictly necessary since dist will already package it when it's present.
- makes bootstrap's `native::LLD` step support using the packaged `ci-llvm/bin/lld`, instead of building it from source (which would most likely not be available today, nor in the future where `download-ci-llvm = if-available` is the default).

If I understand correctly, `--enable-full-tools` will also enable `rust.lld`, and this is why LLD is already packaged today in the `rust-dev` component on the main targets (and why `-Zgcc-ld=lld` does work there).

That means it's likely that this PR will not be able to land before I've reworked and landed rust-lang#101792: if LLD is available in `download-ci-llvm`, the `needs-rust-lld` tests should start being executed on the x64 macOS test builders, and CI would fail today.

I've tested locally that building with `download-ci-llvm = true` and `lld = true` with the LLVM submodule unregistered was successful, and that `rust-lld` and the various `lld-wrapper`s are present and `-Zgcc-ld=lld` works as well, on a few different platforms:
- `x86_64-unknown-linux-gnu`
- `aarch64-apple-darwin`
- `x86_64-pc-windows-msvc` (with `-Clinker=rust-lld` rather than `-Zgcc-ld=lld`)
- `x86_64-apple-darwin`, with the `MACOSX_DEPLOYMENT_TARGET` workaround for rust-lang#101653

I don't think we really need to bump the `download-ci-llvm-stamp` in this case, since `./build/$triple/ci-llvm/bin/lld` is present on all the above targets already, but have added it mechanically, and it should probably be removed to avoid unnecessary downloads/churn.

Fixes rust-lang#98340
Supersedes rust-lang#100010
  • Loading branch information
compiler-errors authored Jan 4, 2023
2 parents c757267 + e0f5c6d commit 752c0f5
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
3 changes: 3 additions & 0 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2067,6 +2067,9 @@ impl Step for RustDev {

builder.ensure(crate::native::Llvm { target });

// We want to package `lld` to use it with `download-ci-llvm`.
builder.ensure(crate::native::Lld { target });

let src_bindir = builder.llvm_out(target).join("bin");
// If updating this list, you likely want to change
// src/bootstrap/download-ci-llvm-stamp as well, otherwise local users
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/download-ci-llvm-stamp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Change this file to make users of the `download-ci-llvm` configuration download
a new version of LLVM from CI, even if the LLVM submodule hasn’t changed.

Last change is for: https://github.com/rust-lang/rust/pull/102790
Last change is for: https://github.com/rust-lang/rust/pull/104748
31 changes: 22 additions & 9 deletions src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ impl LdFlags {
}
}

// This returns whether we've already previously built LLVM.
//
// It's used to avoid busting caches during x.py check -- if we've already built
// LLVM, it's fine for us to not try to avoid doing so.
//
// This will return the llvm-config if it can get it (but it will not build it
// if not).
/// This returns whether we've already previously built LLVM.
///
/// It's used to avoid busting caches during x.py check -- if we've already built
/// LLVM, it's fine for us to not try to avoid doing so.
///
/// This will return the llvm-config if it can get it (but it will not build it
/// if not).
pub fn prebuilt_llvm_config(
builder: &Builder<'_>,
target: TargetSelection,
Expand Down Expand Up @@ -823,8 +823,21 @@ impl Step for Lld {
}
let target = self.target;

let LlvmResult { llvm_config, llvm_cmake_dir } =
builder.ensure(Llvm { target: self.target });
let LlvmResult { llvm_config, llvm_cmake_dir } = builder.ensure(Llvm { target });

// The `dist` step packages LLD next to LLVM's binaries for download-ci-llvm. The root path
// we usually expect here is `./build/$triple/ci-llvm/`, with the binaries in its `bin`
// subfolder. We check if that's the case, and if LLD's binary already exists there next to
// `llvm-config`: if so, we can use it instead of building LLVM/LLD from source.
let ci_llvm_bin = llvm_config.parent().unwrap();
if ci_llvm_bin.is_dir() && ci_llvm_bin.file_name().unwrap() == "bin" {
let lld_path = ci_llvm_bin.join(exe("lld", target));
if lld_path.exists() {
// The following steps copying `lld` as `rust-lld` to the sysroot, expect it in the
// `bin` subfolder of this step's out dir.
return ci_llvm_bin.parent().unwrap().to_path_buf();
}
}

let out_dir = builder.lld_out(target);
let done_stamp = out_dir.join("lld-finished-building");
Expand Down

0 comments on commit 752c0f5

Please sign in to comment.