Skip to content

Commit

Permalink
Auto merge of #76810 - Mark-Simulacrum:fix-lld-macos, r=alexcrichton
Browse files Browse the repository at this point in the history
Don't dynamically link LLVM tools unless rustc is too

This PR initially tried to support link-shared on all of our target platforms (other than Windows), but ran into a number of difficulties:
 * LLVM doesn't really support a shared link on macOS (llvm-config runs into problems with the version suffix)
 * LLVM doesn't seem to support a shared link when cross-compiling (the libLLVM.so ends up empty and symbols are not found)

So, this PR has now been revised such that we don't attempt to dynamically link LLVM tools (even if that would, otherwise, be supported) on targets where LLVM is statically linked to rustc. Currently that's basically everything except for x86_64-unknown-linux-gnu (where we dynamically link to avoid rerunning ThinLTO in each stage).

Follow-up to #76708.
Fixes #76698.
  • Loading branch information
bors committed Sep 22, 2020
2 parents e0bc267 + 389b7ff commit 0da5800
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 18 deletions.
13 changes: 5 additions & 8 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2400,14 +2400,11 @@ fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir
return;
}

// On macOS for some reason the llvm-config binary behaves differently and
// and fails on missing .a files if run without --link-shared. If run with
// that option, it still fails, but because we only ship a libLLVM.dylib
// rather than libLLVM-11-rust-....dylib file.
//
// For now just don't use llvm-config here on macOS; that will fail to
// support CI-built LLVM, but until we work out the different behavior that
// is fine as it is off by default.
// On macOS, rustc (and LLVM tools) link to an unversioned libLLVM.dylib
// instead of libLLVM-11-rust-....dylib, as on linux. It's not entirely
// clear why this is the case, though. llvm-config will emit the versioned
// paths and we don't want those in the sysroot (as we're expecting
// unversioned paths).
if target.contains("apple-darwin") {
let src_libdir = builder.llvm_out(target).join("lib");
let llvm_dylib_path = src_libdir.join("libLLVM.dylib");
Expand Down
11 changes: 8 additions & 3 deletions src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ impl Step for Llvm {
Err(m) => m,
};

if builder.config.llvm_link_shared && target.contains("windows") {
panic!("shared linking to LLVM is not currently supported on Windows");
if builder.config.llvm_link_shared
&& (target.contains("windows") || target.contains("apple-darwin"))
{
panic!("shared linking to LLVM is not currently supported on {}", target.triple);
}

builder.info(&format!("Building LLVM for {}", target));
Expand Down Expand Up @@ -208,7 +210,10 @@ impl Step for Llvm {
// which saves both memory during parallel links and overall disk space
// for the tools. We don't do this on every platform as it doesn't work
// equally well everywhere.
if builder.llvm_link_tools_dynamically(target) {
//
// If we're not linking rustc to a dynamic LLVM, though, then don't link
// tools to it.
if builder.llvm_link_tools_dynamically(target) && builder.config.llvm_link_shared {
cfg.define("LLVM_LINK_LLVM_DYLIB", "ON");
}

Expand Down
7 changes: 0 additions & 7 deletions src/ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ if [ "$DEPLOY$DEPLOY_ALT" = "1" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.remap-debuginfo"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --debuginfo-level-std=1"

# If we're distributing binaries, we want a shared LLVM link. We're already
# going to link LLVM to the LLVM tools dynamically, so we need to ship a
# libLLVM library anyway.
if !isWindows; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set llvm.link-shared=true"
fi

if [ "$NO_LLVM_ASSERTIONS" = "1" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --disable-llvm-assertions"
elif [ "$DEPLOY_ALT" != "" ]; then
Expand Down

0 comments on commit 0da5800

Please sign in to comment.