Skip to content

Commit

Permalink
Address review comments for rust-lang#53031.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelwoerister committed Aug 6, 2018
1 parent 28499b3 commit 603584d
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 7 deletions.
7 changes: 7 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,13 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
sess.err("can't perform LTO when compiling incrementally");
}

// Since we don't know if code in an rlib will be linked to statically or
// dynamically downstream, rustc generates `__imp_` symbols that help the
// MSVC linker deal with this lack of knowledge (#27438). Unfortunately,
// these manually generated symbols confuse LLD when it tries to merge
// bitcode during ThinLTO. Therefore we disallow dynamic linking on MSVC
// when compiling for LLD ThinLTO. This way we can validly just not generate
// the `dllimport` attributes and `__imp_` symbols in that case.
if sess.opts.debugging_opts.cross_lang_lto.enabled() &&
sess.opts.cg.prefer_dynamic &&
sess.target.target.options.is_like_msvc {
Expand Down
10 changes: 5 additions & 5 deletions src/librustc_codegen_llvm/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ fn link_staticlib(sess: &Session,
});
ab.add_rlib(path,
&name.as_str(),
is_full_lto_enabled(sess) &&
are_upstream_rust_objects_already_included(sess) &&
!ignored_for_lto(sess, &codegen_results.crate_info, cnum),
skip_object_files).unwrap();

Expand Down Expand Up @@ -1446,7 +1446,7 @@ fn add_upstream_rust_crates(cmd: &mut dyn Linker,
lib.kind == NativeLibraryKind::NativeStatic && !relevant_lib(sess, lib)
});

if (!is_full_lto_enabled(sess) ||
if (!are_upstream_rust_objects_already_included(sess) ||
ignored_for_lto(sess, &codegen_results.crate_info, cnum)) &&
crate_type != config::CrateType::Dylib &&
!skip_native {
Expand Down Expand Up @@ -1500,7 +1500,7 @@ fn add_upstream_rust_crates(cmd: &mut dyn Linker,
// file, then we don't need the object file as it's part of the
// LTO module. Note that `#![no_builtins]` is excluded from LTO,
// though, so we let that object file slide.
let skip_because_lto = is_full_lto_enabled(sess) &&
let skip_because_lto = are_upstream_rust_objects_already_included(sess) &&
is_rust_object &&
(sess.target.target.options.no_builtins ||
!codegen_results.crate_info.is_no_builtins.contains(&cnum));
Expand Down Expand Up @@ -1537,7 +1537,7 @@ fn add_upstream_rust_crates(cmd: &mut dyn Linker,
fn add_dynamic_crate(cmd: &mut dyn Linker, sess: &Session, cratepath: &Path) {
// If we're performing LTO, then it should have been previously required
// that all upstream rust dependencies were available in an rlib format.
assert!(!is_full_lto_enabled(sess));
assert!(!are_upstream_rust_objects_already_included(sess));

// Just need to tell the linker about where the library lives and
// what its name is
Expand Down Expand Up @@ -1623,7 +1623,7 @@ fn relevant_lib(sess: &Session, lib: &NativeLibrary) -> bool {
}
}

fn is_full_lto_enabled(sess: &Session) -> bool {
fn are_upstream_rust_objects_already_included(sess: &Session) -> bool {
match sess.lto() {
Lto::Yes |
Lto::Fat => true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ all: staticlib.rs upstream.rs
$(RUSTC) staticlib.rs -Z cross-lang-lto -Ccodegen-units=1 -L. -o $(TMPDIR)/staticlib.a
(cd $(TMPDIR); llvm-ar x ./staticlib.a)
# Make sure the upstream object file was included
ls upstream.*.rcgu.o
ls $(TMPDIR)/upstream.*.rcgu.o

# Cleanup
rm $(TMPDIR)/*
Expand All @@ -20,4 +20,4 @@ all: staticlib.rs upstream.rs
$(RUSTC) upstream.rs -Z cross-lang-lto -Ccodegen-units=1 -Clto=thin
$(RUSTC) staticlib.rs -Z cross-lang-lto -Ccodegen-units=1 -Clto=thin -L. -o $(TMPDIR)/staticlib.a
(cd $(TMPDIR); llvm-ar x ./staticlib.a)
ls upstream.*.rcgu.o
ls $(TMPDIR)/upstream.*.rcgu.o

0 comments on commit 603584d

Please sign in to comment.