Address review comments for #53031 and fix some merge fallout.

Author: michaelwoerister

src/librustc/session/mod.rs

@@ -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 {

src/librustc_codegen_llvm/back/link.rs

@@ -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();
 
@@ -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 {
@@ -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));
@@ -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
@@ -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,

src/librustc_codegen_llvm/back/write.rs

@@ -2385,7 +2385,7 @@ fn msvc_imps_needed(tcx: TyCtxt) -> bool {
               tcx.sess.opts.cg.prefer_dynamic));
 
     tcx.sess.target.target.options.is_like_msvc &&
-        tcx.sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateTypeRlib) &&
+        tcx.sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateType::Rlib) &&
     // ThinLTO can't handle this workaround in all cases, so we don't
     // emit the `__imp_` symbols. Instead we make them unnecessary by disallowing
     // dynamic linking when cross-language LTO is enabled.

src/test/codegen/no-dllimport-w-cross-lang-lto.rs

@@ -13,7 +13,7 @@
 
 // no-prefer-dynamic
 // only-msvc
-// compile-flags: -C no-prepopulate-passes -Z cross-lang-lto
+// compile-flags: -Z cross-lang-lto
 
 #![crate_type = "rlib"]
 

src/test/codegen/target-cpu-on-functions.rs

@@ -11,8 +11,10 @@
 // This test makes sure that functions get annotated with the proper
 // "target-cpu" attribute in LLVM.
 
+// no-prefer-dynamic
+// ignore-tidy-linelength
 // only-x86_64
-// compile-flags: -C no-prepopulate-passes -C panic=abort
+// compile-flags: -C no-prepopulate-passes -C panic=abort -Z cross-lang-lto -Cpasses=name-anon-globals
 
 #![crate_type = "staticlib"]
 

src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile

@@ -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)/*
@@ -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