Auto merge of #53031 - michaelwoerister:cross-lto, r=alexcrichton

Apply some fixes to cross-language LTO (especially when targeting MSVC)

This PR contains a few fixes that were needed in order to get Firefox compiling with Rust/C++ cross-language ThinLTO on Windows. The commits are self-contained and should be self-explanatory.

r? @alexcrichton

Author: bors

src/librustc/session/config.rs

@@ -831,7 +831,7 @@ macro_rules! options {
         pub const parse_lto: Option<&'static str> =
             Some("one of `thin`, `fat`, or omitted");
         pub const parse_cross_lang_lto: Option<&'static str> =
-            Some("either a boolean (`yes`, `no`, `on`, `off`, etc), `no-link`, \
+            Some("either a boolean (`yes`, `no`, `on`, `off`, etc), \
                   or the path to the linker plugin");
     }
 
@@ -2006,13 +2006,6 @@ pub fn build_session_options_and_crate_config(
         (&None, &None) => None,
     }.map(|m| PathBuf::from(m));
 
-    if cg.lto != Lto::No && incremental.is_some() {
-        early_error(
-            error_format,
-            "can't perform LTO when compiling incrementally",
-        );
-    }
-
     if debugging_opts.profile && incremental.is_some() {
         early_error(
             error_format,

src/librustc/session/mod.rs

@@ -20,7 +20,7 @@ use lint::builtin::BuiltinLintDiagnostics;
 use middle::allocator::AllocatorKind;
 use middle::dependency_format;
 use session::search_paths::PathKind;
-use session::config::{OutputType};
+use session::config::{OutputType, Lto};
 use ty::tls;
 use util::nodemap::{FxHashMap, FxHashSet};
 use util::common::{duration_to_secs_str, ErrorReported};
@@ -1189,9 +1189,34 @@ pub fn build_session_(
         driver_lint_caps: FxHashMap(),
     };
 
+    validate_commandline_args_with_session_available(&sess);
+
     sess
 }
 
+// If it is useful to have a Session available already for validating a
+// commandline argument, you can do so here.
+fn validate_commandline_args_with_session_available(sess: &Session) {
+
+    if sess.lto() != Lto::No && sess.opts.incremental.is_some() {
+        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 {
+        sess.err("Linker plugin based LTO is not supported together with \
+                  `-C prefer-dynamic` when targeting MSVC");
+    }
+}
+
 /// Hash value constructed out of all the `-C metadata` arguments passed to the
 /// compiler. Together with the crate-name forms a unique global identifier for
 /// the crate.

src/librustc_codegen_llvm/attributes.rs

@@ -123,6 +123,15 @@ pub fn llvm_target_features(sess: &Session) -> impl Iterator<Item = &str> {
         .filter(|l| !l.is_empty())
 }
 
+pub fn apply_target_cpu_attr(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value) {
+    let target_cpu = CString::new(cx.tcx.sess.target_cpu().to_string()).unwrap();
+    llvm::AddFunctionAttrStringValue(
+            llfn,
+            llvm::AttributePlace::Function,
+            cstr("target-cpu\0"),
+            target_cpu.as_c_str());
+}
+
 /// Composite function which sets LLVM attributes for function depending on its AST (#[attribute])
 /// attributes.
 pub fn from_fn_attrs(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value, id: DefId) {
@@ -167,6 +176,15 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value, id: DefId) {
         Some(true) | None => {}
     }
 
+    // Always annotate functions with the target-cpu they are compiled for.
+    // Without this, ThinLTO won't inline Rust functions into Clang generated
+    // functions (because Clang annotates functions this way too).
+    // NOTE: For now we just apply this if -Zcross-lang-lto is specified, since
+    //       it introduce a little overhead and isn't really necessary otherwise.
+    if cx.tcx.sess.opts.debugging_opts.cross_lang_lto.enabled() {
+        apply_target_cpu_attr(cx, llfn);
+    }
+
     let features = llvm_target_features(cx.tcx.sess)
         .map(|s| s.to_string())
         .chain(

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,11 +1623,15 @@ 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::Thin |
         Lto::Fat => true,
+        Lto::Thin => {
+            // If we defer LTO to the linker, we haven't run LTO ourselves, so
+            // any upstream object files have not been copied yet.
+            !sess.opts.debugging_opts.cross_lang_lto.enabled()
+        }
         Lto::No |
         Lto::ThinLocal => false,
     }

src/librustc_codegen_llvm/back/lto.rs

@@ -195,6 +195,10 @@ pub(crate) fn run(cgcx: &CodegenContext,
         }
         Lto::Thin |
         Lto::ThinLocal => {
+            if cgcx.opts.debugging_opts.cross_lang_lto.enabled() {
+                unreachable!("We should never reach this case if the LTO step \
+                              is deferred to the linker");
+            }
             thin_lto(&diag_handler, modules, upstream_modules, &arr, timeline)
         }
         Lto::No => unreachable!(),

src/librustc_codegen_llvm/back/write.rs

@@ -552,7 +552,8 @@ unsafe fn optimize(cgcx: &CodegenContext,
                 llvm::LLVMRustAddAnalysisPasses(tm, fpm, llmod);
                 llvm::LLVMRustAddAnalysisPasses(tm, mpm, llmod);
                 let opt_level = config.opt_level.unwrap_or(llvm::CodeGenOptLevel::None);
-                let prepare_for_thin_lto = cgcx.lto == Lto::Thin || cgcx.lto == Lto::ThinLocal;
+                let prepare_for_thin_lto = cgcx.lto == Lto::Thin || cgcx.lto == Lto::ThinLocal ||
+                    (cgcx.lto != Lto::Fat && cgcx.opts.debugging_opts.cross_lang_lto.enabled());
                 have_name_anon_globals_pass = have_name_anon_globals_pass || prepare_for_thin_lto;
                 if using_thin_buffers && !prepare_for_thin_lto {
                     assert!(addpass("name-anon-globals"));
@@ -1351,6 +1352,8 @@ fn execute_work_item(cgcx: &CodegenContext,
         unsafe {
             optimize(cgcx, &diag_handler, &module, config, timeline)?;
 
+            let linker_does_lto = cgcx.opts.debugging_opts.cross_lang_lto.enabled();
+
             // After we've done the initial round of optimizations we need to
             // decide whether to synchronously codegen this module or ship it
             // back to the coordinator thread for further LTO processing (which
@@ -1361,6 +1364,11 @@ fn execute_work_item(cgcx: &CodegenContext,
             let needs_lto = match cgcx.lto {
                 Lto::No => false,
 
+                // If the linker does LTO, we don't have to do it. Note that we
+                // keep doing full LTO, if it is requested, as not to break the
+                // assumption that the output will be a single module.
+                Lto::Thin | Lto::ThinLocal if linker_does_lto => false,
+
                 // Here we've got a full crate graph LTO requested. We ignore
                 // this, however, if the crate type is only an rlib as there's
                 // no full crate graph to process, that'll happen later.
@@ -1391,11 +1399,6 @@ fn execute_work_item(cgcx: &CodegenContext,
             // settings.
             let needs_lto = needs_lto && module.kind != ModuleKind::Metadata;
 
-            // Don't run LTO passes when cross-lang LTO is enabled. The linker
-            // will do that for us in this case.
-            let needs_lto = needs_lto &&
-                !cgcx.opts.debugging_opts.cross_lang_lto.enabled();
-
             if needs_lto {
                 Ok(WorkItemResult::NeedsLTO(module))
             } else {
@@ -2375,8 +2378,18 @@ pub(crate) fn submit_codegened_module_to_llvm(tcx: TyCtxt,
 }
 
 fn msvc_imps_needed(tcx: TyCtxt) -> bool {
+    // This should never be true (because it's not supported). If it is true,
+    // something is wrong with commandline arg validation.
+    assert!(!(tcx.sess.opts.debugging_opts.cross_lang_lto.enabled() &&
+              tcx.sess.target.target.options.is_like_msvc &&
+              tcx.sess.opts.cg.prefer_dynamic));
+
     tcx.sess.target.target.options.is_like_msvc &&
-        tcx.sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateType::Rlib)
+        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.
+    !tcx.sess.opts.debugging_opts.cross_lang_lto.enabled()
 }
 
 // Create a `__imp_<symbol> = &symbol` global for every public static `symbol`.

src/librustc_codegen_llvm/base.rs

@@ -596,6 +596,7 @@ fn maybe_create_entry_wrapper(cx: &CodegenCx) {
 
         // `main` should respect same config for frame pointer elimination as rest of code
         attributes::set_frame_pointer_elimination(cx, llfn);
+        attributes::apply_target_cpu_attr(cx, llfn);
 
         let bx = Builder::new_block(cx, llfn, "top");
 

src/librustc_codegen_llvm/consts.rs

@@ -189,7 +189,20 @@ pub fn get_static(cx: &CodegenCx<'ll, '_>, def_id: DefId) -> &'ll Value {
             llvm::set_thread_local_mode(g, cx.tls_model);
         }
 
-        if cx.use_dll_storage_attrs && !cx.tcx.is_foreign_item(def_id) {
+        let needs_dll_storage_attr =
+            cx.use_dll_storage_attrs && !cx.tcx.is_foreign_item(def_id) &&
+            // ThinLTO can't handle this workaround in all cases, so we don't
+            // emit the attrs. Instead we make them unnecessary by disallowing
+            // dynamic linking when cross-language LTO is enabled.
+            !cx.tcx.sess.opts.debugging_opts.cross_lang_lto.enabled();
+
+        // If this assertion triggers, there's something wrong with commandline
+        // argument validation.
+        debug_assert!(!(cx.tcx.sess.opts.debugging_opts.cross_lang_lto.enabled() &&
+                        cx.tcx.sess.target.target.options.is_like_msvc &&
+                        cx.tcx.sess.opts.cg.prefer_dynamic));
+
+        if needs_dll_storage_attr {
             // This item is external but not foreign, i.e. it originates from an external Rust
             // crate. Since we don't know whether this crate will be linked dynamically or
             // statically in the final application, we always mark such symbols as 'dllimport'.

src/librustc_codegen_llvm/context.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use attributes;
 use common;
 use llvm;
 use rustc::dep_graph::DepGraphSafe;
@@ -381,6 +382,7 @@ impl<'b, 'tcx> CodegenCx<'b, 'tcx> {
                 declare::declare_cfn(self, name, fty)
             }
         };
+        attributes::apply_target_cpu_attr(self, llfn);
         self.eh_personality.set(Some(llfn));
         llfn
     }
@@ -412,6 +414,7 @@ impl<'b, 'tcx> CodegenCx<'b, 'tcx> {
 
         let llfn = declare::declare_fn(self, "rust_eh_unwind_resume", ty);
         attributes::unwind(llfn, true);
+        attributes::apply_target_cpu_attr(self, llfn);
         unwresume.set(Some(llfn));
         llfn
     }

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

@@ -0,0 +1,23 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// This test makes sure that functions get annotated with the proper
+// "target-cpu" attribute in LLVM.
+
+// no-prefer-dynamic
+// only-msvc
+// compile-flags: -Z cross-lang-lto
+
+#![crate_type = "rlib"]
+
+// CHECK-NOT: @{{.*}}__imp_{{.*}}GLOBAL{{.*}} = global i8*
+
+pub static GLOBAL: u32 = 0;
+pub static mut GLOBAL2: u32 = 0;

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

@@ -0,0 +1,29 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// This test makes sure that functions get annotated with the proper
+// "target-cpu" attribute in LLVM.
+
+// no-prefer-dynamic
+// ignore-tidy-linelength
+// compile-flags: -C no-prepopulate-passes -C panic=abort -Z cross-lang-lto -Cpasses=name-anon-globals
+
+#![crate_type = "staticlib"]
+
+// CHECK-LABEL: define {{.*}} @exported() {{.*}} #0
+#[no_mangle]
+pub extern fn exported() {
+    not_exported();
+}
+
+// CHECK-LABEL: define {{.*}} @_ZN23target_cpu_on_functions12not_exported{{.*}}() {{.*}} #0
+fn not_exported() {}
+
+// CHECK: attributes #0 = {{.*}} "target-cpu"="{{.*}}"

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

@@ -0,0 +1,23 @@
+
+-include ../tools.mk
+
+# This test makes sure that we don't loose upstream object files when compiling
+# staticlibs with -Zcross-lang-lto
+
+all: staticlib.rs upstream.rs
+	$(RUSTC) upstream.rs -Z cross-lang-lto -Ccodegen-units=1
+
+	# Check No LTO
+	$(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 $(TMPDIR)/upstream.*.rcgu.o
+
+	# Cleanup
+	rm $(TMPDIR)/*
+
+	# Check ThinLTO
+	$(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 $(TMPDIR)/upstream.*.rcgu.o

src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/staticlib.rs

@@ -0,0 +1,18 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type="staticlib"]
+
+extern crate upstream;
+
+#[no_mangle]
+pub extern fn bar() {
+    upstream::foo();
+}

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

@@ -0,0 +1,13 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type = "rlib"]
+
+pub fn foo() {}

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

@@ -1,4 +1,3 @@
-# ignore-msvc
 
 -include ../tools.mk