From d5f8edfa103442e755e1ab3ba76dfd6b0418c043 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 2 Aug 2018 14:16:43 +0200 Subject: [PATCH 01/10] Fix outdated description of -Zcross-lang-lto. --- src/librustc/session/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 3111777f4ad5a..219607a456b49 100644 --- a/src/librustc/session/config.rs +++ b/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"); } From 3a3b3317d99f0b72884002d1e4809bbe87356f8f Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 2 Aug 2018 14:26:27 +0200 Subject: [PATCH 02/10] Fix issue around dllimport and ThinLTO as LLD runs it. --- src/librustc/session/config.rs | 7 ------- src/librustc/session/mod.rs | 20 +++++++++++++++++++- src/librustc_codegen_llvm/back/lto.rs | 4 ++++ src/librustc_codegen_llvm/back/write.rs | 24 ++++++++++++++++++------ src/librustc_codegen_llvm/consts.rs | 15 ++++++++++++++- 5 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 219607a456b49..dddf921aec68c 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -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, diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 0173c4933f821..505511a4a0c1c 100644 --- a/src/librustc/session/mod.rs +++ b/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,27 @@ 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"); + } + + 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. diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index 098676c95a289..d7741230327bb 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/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!(), diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 5e23c6e868f47..9d235eced2eab 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -1351,6 +1351,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 +1363,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 +1398,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 +2377,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::CrateTypeRlib) && + // 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` global for every public static `symbol`. diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 21bf490beb0fb..fafc0e723225d 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/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'. From aa9eeff26370b17e8ba140b6cb285a4613047132 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 2 Aug 2018 14:26:55 +0200 Subject: [PATCH 03/10] Make sure upstream object files are added to staticlibs when compiling with ThinLTO and cross-lang-lto. Normally, when compiling with whole-crate-graph ThinLTO, we expect rustc's LTO step to "uplift" upstream object files/LLVM modules to the current set of compilation artifacts. Therefore the staticlib creation code skips this uplifting. However, when compiling with "cross-language LTO" (i.e. defer LTO to the actual linker), the LTO step in rustc is not performed, so we have to take care of copying upstream object files during archive creation (like we already do when compiling without any LTO). --- src/librustc_codegen_llvm/back/link.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index 108734b67d3b8..72084c170aa71 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/src/librustc_codegen_llvm/back/link.rs @@ -1626,8 +1626,12 @@ fn relevant_lib(sess: &Session, lib: &NativeLibrary) -> bool { fn is_full_lto_enabled(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, } From 54fba3ac1aa308ecec1bf6b25049c669bbf003c3 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 3 Aug 2018 17:06:01 +0200 Subject: [PATCH 04/10] Run cross-lang-lto tests also for MSVC (since there's no reason not to) --- src/test/run-make-fulldeps/cross-lang-lto/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/run-make-fulldeps/cross-lang-lto/Makefile b/src/test/run-make-fulldeps/cross-lang-lto/Makefile index efe1b7072ffb8..6d06fade35296 100644 --- a/src/test/run-make-fulldeps/cross-lang-lto/Makefile +++ b/src/test/run-make-fulldeps/cross-lang-lto/Makefile @@ -1,4 +1,3 @@ -# ignore-msvc -include ../tools.mk From 386e000c5f28dfac697ae145524bcc1c17759993 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 3 Aug 2018 17:07:38 +0200 Subject: [PATCH 05/10] Add test case for omitting dllimport during cross-lang LTO. --- .../codegen/no-dllimport-w-cross-lang-lto.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/test/codegen/no-dllimport-w-cross-lang-lto.rs diff --git a/src/test/codegen/no-dllimport-w-cross-lang-lto.rs b/src/test/codegen/no-dllimport-w-cross-lang-lto.rs new file mode 100644 index 0000000000000..025e7cbf179a7 --- /dev/null +++ b/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 or the MIT license +// , 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: -C no-prepopulate-passes -Z cross-lang-lto + +#![crate_type = "rlib"] + +// CHECK-NOT: @{{.*}}__imp_{{.*}}GLOBAL{{.*}} = global i8* + +pub static GLOBAL: u32 = 0; +pub static mut GLOBAL2: u32 = 0; From 3742f4d9f6eaf7b4b7a8aa246ee3dc2fdb2b80c3 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 3 Aug 2018 17:09:50 +0200 Subject: [PATCH 06/10] Add test case for including upstream object files in staticlibs when doing cross-lang LTO. --- .../cross-lang-lto-upstream-rlibs/Makefile | 23 +++++++++++++++++++ .../staticlib.rs | 18 +++++++++++++++ .../cross-lang-lto-upstream-rlibs/upstream.rs | 13 +++++++++++ 3 files changed, 54 insertions(+) create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/staticlib.rs create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/upstream.rs diff --git a/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile b/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile new file mode 100644 index 0000000000000..de42c6e0eb54e --- /dev/null +++ b/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 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 upstream.*.rcgu.o diff --git a/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/staticlib.rs b/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/staticlib.rs new file mode 100644 index 0000000000000..b370b7b859d87 --- /dev/null +++ b/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 or the MIT license +// , 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(); +} diff --git a/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/upstream.rs b/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/upstream.rs new file mode 100644 index 0000000000000..a79b9bf08fc65 --- /dev/null +++ b/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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_type = "rlib"] + +pub fn foo() {} From f2969ed6c3c067659b5a160de08e9a57450b1047 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 3 Aug 2018 17:04:53 +0200 Subject: [PATCH 07/10] Set 'PrepareForThinLTO' whenever doing cross-language LTO. --- src/librustc_codegen_llvm/back/write.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 9d235eced2eab..47440d864d5e7 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/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")); From b27a161939620626ca5fdcb9f4dd486a6ed1e827 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 2 Aug 2018 18:10:26 +0200 Subject: [PATCH 08/10] Annotate functions in LLVM with target-cpu, same as Clang does. --- src/librustc_codegen_llvm/attributes.rs | 18 +++++++++++++ src/librustc_codegen_llvm/base.rs | 1 + src/librustc_codegen_llvm/context.rs | 3 +++ src/test/codegen/target-cpu-on-functions.rs | 28 +++++++++++++++++++++ 4 files changed, 50 insertions(+) create mode 100644 src/test/codegen/target-cpu-on-functions.rs diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index c52f894410899..714e8914e48c5 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -123,6 +123,15 @@ pub fn llvm_target_features(sess: &Session) -> impl Iterator { .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( diff --git a/src/librustc_codegen_llvm/base.rs b/src/librustc_codegen_llvm/base.rs index 41336165684f6..13e8426155a95 100644 --- a/src/librustc_codegen_llvm/base.rs +++ b/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"); diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 2f557d0b09998..7a308bb6e8823 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/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 } diff --git a/src/test/codegen/target-cpu-on-functions.rs b/src/test/codegen/target-cpu-on-functions.rs new file mode 100644 index 0000000000000..c2765a46caae0 --- /dev/null +++ b/src/test/codegen/target-cpu-on-functions.rs @@ -0,0 +1,28 @@ +// 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 or the MIT license +// , 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. + +// only-x86_64 +// compile-flags: -C no-prepopulate-passes -C panic=abort + +#![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"="x86-64" From 3a7005037726263c0beffd0efc3d5d147b2a86b4 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 6 Aug 2018 11:16:28 +0200 Subject: [PATCH 09/10] Address review comments for #53031 and fix some merge fallout. --- src/librustc/session/mod.rs | 7 +++++++ src/librustc_codegen_llvm/back/link.rs | 10 +++++----- src/librustc_codegen_llvm/back/write.rs | 2 +- src/test/codegen/no-dllimport-w-cross-lang-lto.rs | 2 +- src/test/codegen/target-cpu-on-functions.rs | 4 +++- .../cross-lang-lto-upstream-rlibs/Makefile | 4 ++-- 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 505511a4a0c1c..9a3ce50fcbdce 100644 --- a/src/librustc/session/mod.rs +++ b/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 { diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index 72084c170aa71..50d41d76986fb 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/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, diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 47440d864d5e7..640e1c1f3d4f1 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/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. diff --git a/src/test/codegen/no-dllimport-w-cross-lang-lto.rs b/src/test/codegen/no-dllimport-w-cross-lang-lto.rs index 025e7cbf179a7..0d5d02206a632 100644 --- a/src/test/codegen/no-dllimport-w-cross-lang-lto.rs +++ b/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"] diff --git a/src/test/codegen/target-cpu-on-functions.rs b/src/test/codegen/target-cpu-on-functions.rs index c2765a46caae0..1a6ab22e5685d 100644 --- a/src/test/codegen/target-cpu-on-functions.rs +++ b/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"] diff --git a/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile b/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile index de42c6e0eb54e..0a6f226a027f3 100644 --- a/src/test/run-make-fulldeps/cross-lang-lto-upstream-rlibs/Makefile +++ b/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 From 49972e93ffdd6d70ddfef8486d1642cec69bfaeb Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 9 Aug 2018 09:39:02 +0200 Subject: [PATCH 10/10] Relax the target-cpu-on-function codegen test so it just checks for presence of the attribute. --- src/test/codegen/target-cpu-on-functions.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/codegen/target-cpu-on-functions.rs b/src/test/codegen/target-cpu-on-functions.rs index 1a6ab22e5685d..cd7d061c0de3d 100644 --- a/src/test/codegen/target-cpu-on-functions.rs +++ b/src/test/codegen/target-cpu-on-functions.rs @@ -13,7 +13,6 @@ // no-prefer-dynamic // ignore-tidy-linelength -// only-x86_64 // compile-flags: -C no-prepopulate-passes -C panic=abort -Z cross-lang-lto -Cpasses=name-anon-globals #![crate_type = "staticlib"] @@ -27,4 +26,4 @@ pub extern fn exported() { // CHECK-LABEL: define {{.*}} @_ZN23target_cpu_on_functions12not_exported{{.*}}() {{.*}} #0 fn not_exported() {} -// CHECK: attributes #0 = {{.*}} "target-cpu"="x86-64" +// CHECK: attributes #0 = {{.*}} "target-cpu"="{{.*}}"