Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not make local copies of inline fns in debug mode #76896

Merged
merged 3 commits into from
Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::dep_graph::{DepConstructor, DepNode, WorkProduct, WorkProductId};
use crate::ich::{NodeIdHashingMode, StableHashingContext};
use crate::ty::{subst::InternalSubsts, Instance, InstanceDef, SymbolName, TyCtxt};
use rustc_attr::InlineAttr;
use rustc_data_structures::base_n;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -79,14 +78,6 @@ impl<'tcx> MonoItem<'tcx> {
}

pub fn instantiation_mode(&self, tcx: TyCtxt<'tcx>) -> InstantiationMode {
let generate_cgu_internal_copies = tcx
.sess
.opts
.debugging_opts
.inline_in_all_cgus
.unwrap_or_else(|| tcx.sess.opts.optimize != OptLevel::No)
&& !tcx.sess.link_dead_code();

match *self {
MonoItem::Fn(ref instance) => {
let entry_def_id = tcx.entry_fn(LOCAL_CRATE).map(|(id, _)| id);
Expand All @@ -99,21 +90,26 @@ impl<'tcx> MonoItem<'tcx> {
return InstantiationMode::GloballyShared { may_conflict: false };
}

let generate_cgu_internal_copies = tcx
.sess
.opts
.debugging_opts
.inline_in_all_cgus
.unwrap_or_else(|| tcx.sess.opts.optimize != OptLevel::No)
&& !tcx.sess.link_dead_code();

// At this point we don't have explicit linkage and we're an
// inlined function. If we're inlining into all CGUs then we'll
// be creating a local copy per CGU.
// inlined function. If we should generate local copies for each CGU,
// then return `LocalCopy`, otherwise we'll just generate one copy
// and share it with all CGUs in this crate.
if generate_cgu_internal_copies {
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
return InstantiationMode::LocalCopy;
}

// Finally, if this is `#[inline(always)]` we're sure to respect
// that with an inline copy per CGU, but otherwise we'll be
// creating one copy of this `#[inline]` function which may
// conflict with upstream crates as it could be an exported
// symbol.
match tcx.codegen_fn_attrs(instance.def_id()).inline {
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
InlineAttr::Always => InstantiationMode::LocalCopy,
_ => InstantiationMode::GloballyShared { may_conflict: true },
InstantiationMode::LocalCopy
} else {
// Finally, if we've reached this point, then we should optimize for
// compilation speed. In that regard, we will ignore any `#[inline]`
// annotations on the function and simply codegen it as usual. This could
// conflict with upstream crates as it could be an exported symbol.
InstantiationMode::GloballyShared { may_conflict: true }
}
}
MonoItem::Static(..) | MonoItem::GlobalAsm(..) => {
Expand Down
9 changes: 7 additions & 2 deletions src/test/incremental/hygiene/load_cached_hygiene.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// revisions:rpass1 rpass2
// compile-flags: -Z query-dep-graph
// compile-flags: -Z query-dep-graph -O
// aux-build:cached_hygiene.rs

// This tests the folllowing scenario
Expand All @@ -19,7 +19,12 @@
// the metadata. Specifically, we were not resetting `orig_id`
// for an `EpxnData` generate in the current crate, which would cause
// us to serialize the `ExpnId` pointing to a garbage location in
// the metadata.
// the metadata.o

// NOTE: We're explicitly passing the `-O` optimization flag because if optimizations are not
// enabled, then rustc will ignore the `#[inline(always)]` attribute which means we do not load
// the optimized mir for the unmodified function to be loaded and so the CGU containing that
// function will be reused.

#![feature(rustc_attrs)]

Expand Down
9 changes: 8 additions & 1 deletion src/test/incremental/remapped_paths_cc/main.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
// revisions:rpass1 rpass2 rpass3
// compile-flags: -Z query-dep-graph -g
// compile-flags: -Z query-dep-graph -g -O
// aux-build:extern_crate.rs

// ignore-asmjs wasm2js does not support source maps yet

// This test case makes sure that we detect if paths emitted into debuginfo
// are changed, even when the change happens in an external crate.

// NOTE: We're explicitly passing the `-O` optimization flag because if no optimizations are
// requested, rustc will ignore the `#[inline]` attribute. This is a performance optimization for
// non-optimized builds which causes us to generate fewer copies of inlined functions when
// runtime performance doesn't matter. Without this flag, the function will go into a different
// CGU which can be reused by this crate.

#![feature(rustc_attrs)]

#![rustc_partition_reused(module="main", cfg="rpass2")]
Expand Down
7 changes: 6 additions & 1 deletion src/test/run-make-fulldeps/inline-always-many-cgu/Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
-include ../tools.mk

all:
$(RUSTC) foo.rs --emit llvm-ir -C codegen-units=2
$(RUSTC) foo.rs --emit llvm-ir -C codegen-units=2 -C opt-level=0
if ![cat $(TMPDIR)/*.ll | $(CGREP) -e '\bcall\b']; then \
echo "not found call instruction when one was expected"; \
exit 1; \
fi
$(RUSTC) foo.rs --emit llvm-ir -C codegen-units=2 -C opt-level=1
if cat $(TMPDIR)/*.ll | $(CGREP) -e '\bcall\b'; then \
echo "found call instruction when one wasn't expected"; \
exit 1; \
Expand Down