Skip to content

Commit

Permalink
Fix issue around dllimport and ThinLTO as LLD runs it.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelwoerister committed Aug 7, 2018
1 parent d5f8edf commit 3a3b331
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 15 deletions.
7 changes: 0 additions & 7 deletions src/librustc/session/config.rs
Expand Up @@ -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,
Expand Down
20 changes: 19 additions & 1 deletion src/librustc/session/mod.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_codegen_llvm/back/lto.rs
Expand Up @@ -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!(),
Expand Down
24 changes: 18 additions & 6 deletions src/librustc_codegen_llvm/back/write.rs
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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> = &symbol` global for every public static `symbol`.
Expand Down
15 changes: 14 additions & 1 deletion src/librustc_codegen_llvm/consts.rs
Expand Up @@ -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'.
Expand Down

0 comments on commit 3a3b331

Please sign in to comment.