Skip to content

[wip] Tweak cgu partitioning #71349

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

Closed
Closed
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
31 changes: 30 additions & 1 deletion src/librustc_middle/mir/mono.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::dep_graph::{DepConstructor, DepNode, WorkProduct, WorkProductId};
use crate::ich::{NodeIdHashingMode, StableHashingContext};
use crate::ty::print::obsolete::DefPathBasedNames;
use crate::ty::{subst::InternalSubsts, Instance, InstanceDef, SymbolName, TyCtxt};
use crate::ty::{
subst::GenericArgKind, subst::InternalSubsts, Instance, InstanceDef, SymbolName, TyCtxt,
};
use rustc_attr::InlineAttr;
use rustc_data_structures::base_n;
use rustc_data_structures::fingerprint::Fingerprint;
Expand Down Expand Up @@ -48,6 +50,13 @@ pub enum MonoItem<'tcx> {
}

impl<'tcx> MonoItem<'tcx> {
pub fn def_id(&self) -> Option<DefId> {
match self {
MonoItem::Fn(instance) => Some(instance.def_id()),
MonoItem::Static(def_id) => Some(*def_id),
MonoItem::GlobalAsm(..) => None,
}
}
pub fn size_estimate(&self, tcx: TyCtxt<'tcx>) -> usize {
match *self {
MonoItem::Fn(instance) => {
Expand All @@ -68,6 +77,18 @@ impl<'tcx> MonoItem<'tcx> {
}
}

pub fn has_closure_generic_argument(&self) -> bool {
match *self {
MonoItem::Fn(instance) => {
instance.substs.non_erasable_generics().any(|arg| match arg {
GenericArgKind::Type(ty) => ty.is_closure(),
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
})
}
MonoItem::Static(..) | MonoItem::GlobalAsm(..) => false,
}
}

pub fn symbol_name(&self, tcx: TyCtxt<'tcx>) -> SymbolName {
match *self {
MonoItem::Fn(instance) => tcx.symbol_name(instance),
Expand Down Expand Up @@ -207,6 +228,14 @@ impl<'tcx> MonoItem<'tcx> {
}
.map(|hir_id| tcx.hir().span(hir_id))
}

pub fn is_local(&self) -> bool {
match *self {
MonoItem::Fn(Instance { def, .. }) => def.def_id().is_local(),
MonoItem::Static(def_id) => def_id.is_local(),
MonoItem::GlobalAsm(..) => true,
}
}
}

impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for MonoItem<'tcx> {
Expand Down
50 changes: 40 additions & 10 deletions src/librustc_mir/monomorphize/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ use rustc_middle::mir::mono::{InstantiationMode, MonoItem};
use rustc_middle::ty::print::characteristic_def_id_of_type;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, DefIdTree, InstanceDef, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_span::symbol::{Symbol, SymbolStr};

use crate::monomorphize::collector::InliningMap;
Expand Down Expand Up @@ -137,6 +138,8 @@ where

initial_partitioning.codegen_units.iter_mut().for_each(|cgu| cgu.estimate_size(tcx));

initial_partitioning.codegen_units.sort_by_key(|cgu| cgu.name().as_str());

debug_dump(tcx, "INITIAL PARTITIONING:", initial_partitioning.codegen_units.iter());

// Merge until we have at most `max_cgu_count` codegen units.
Expand All @@ -152,7 +155,9 @@ where
// local functions the definition of which is marked with `#[inline]`.
let mut post_inlining = {
let _prof_timer = tcx.prof.generic_activity("cgu_partitioning_place_inline_items");
place_inlined_mono_items(initial_partitioning, inlining_map)
let is_debug_incremental =
tcx.sess.opts.optimize == OptLevel::No && tcx.sess.opts.incremental.is_some();
place_inlined_mono_items(initial_partitioning, inlining_map, is_debug_incremental)
};

post_inlining.codegen_units.iter_mut().for_each(|cgu| cgu.estimate_size(tcx));
Expand Down Expand Up @@ -206,6 +211,7 @@ where
let mut roots = FxHashSet::default();
let mut codegen_units = FxHashMap::default();
let is_incremental_build = tcx.sess.opts.incremental.is_some();
let is_debug_incremental = is_incremental_build && tcx.sess.opts.optimize == OptLevel::No;
let mut internalization_candidates = FxHashSet::default();

// Determine if monomorphizations instantiated in this crate will be made
Expand All @@ -223,18 +229,30 @@ where
InstantiationMode::LocalCopy => continue,
}

let characteristic_def_id = characteristic_def_id_of_mono_item(tcx, mono_item);
let characteristic_def_id =
characteristic_def_id_of_mono_item(tcx, mono_item, is_debug_incremental);
let is_volatile = is_incremental_build && mono_item.is_generic_fn();

let codegen_unit_name = match characteristic_def_id {
Some(def_id) => compute_codegen_unit_name(
let codegen_unit_name = match (characteristic_def_id, mono_item.is_local()) {
(Some(def_id), false) if is_debug_incremental => {
let crate_name = tcx.crate_name(def_id.krate);
cgu_name_builder.build_cgu_name(
LOCAL_CRATE,
&[
&*crate_name.as_str(),
if mono_item.has_closure_generic_argument() { "has_closure" } else { "" },
],
Some("cgu"),
)
}
(Some(def_id), _) => compute_codegen_unit_name(
tcx,
cgu_name_builder,
def_id,
is_volatile,
cgu_name_cache,
),
None => fallback_cgu_name(cgu_name_builder),
(None, _) => fallback_cgu_name(cgu_name_builder),
};

let codegen_unit = codegen_units
Expand Down Expand Up @@ -459,7 +477,9 @@ fn merge_codegen_units<'tcx>(
assert!(target_cgu_count >= 1);
let codegen_units = &mut initial_partitioning.codegen_units;

if tcx.is_compiler_builtins(LOCAL_CRATE) {
if tcx.is_compiler_builtins(LOCAL_CRATE)
|| (tcx.dep_graph.is_fully_enabled() && tcx.sess.opts.optimize == OptLevel::No)
{
// Compiler builtins require some degree of control over how mono items
// are partitioned into compilation units. Provide it by keeping the
// original partitioning when compiling the compiler builtins crate.
Expand Down Expand Up @@ -555,6 +575,7 @@ fn merge_codegen_units<'tcx>(
fn place_inlined_mono_items<'tcx>(
initial_partitioning: PreInliningPartitioning<'tcx>,
inlining_map: &InliningMap<'tcx>,
is_debug_incremental: bool,
) -> PostInliningPartitioning<'tcx> {
let mut new_partitioning = Vec::new();
let mut mono_item_placements = FxHashMap::default();
Expand Down Expand Up @@ -587,10 +608,14 @@ fn place_inlined_mono_items<'tcx>(
);
}

// This is a CGU-private copy.
new_codegen_unit
.items_mut()
.insert(mono_item, (Linkage::Internal, Visibility::Default));
// In debug-incremental, do not create CGU-private copies unless it's for an external symbol
// FIXME: put external symbols in a separate codegen unit
if !is_debug_incremental || !mono_item.is_local() {
// This is a CGU-private copy.
new_codegen_unit
.items_mut()
.insert(mono_item, (Linkage::Internal, Visibility::Default));
}
}

if !single_codegen_unit {
Expand Down Expand Up @@ -707,7 +732,12 @@ fn internalize_symbols<'tcx>(
fn characteristic_def_id_of_mono_item<'tcx>(
tcx: TyCtxt<'tcx>,
mono_item: MonoItem<'tcx>,
is_debug_incremental: bool,
) -> Option<DefId> {
if is_debug_incremental {
return mono_item.def_id();
}

match mono_item {
MonoItem::Fn(instance) => {
let def_id = match instance.def {
Expand Down
19 changes: 18 additions & 1 deletion src/librustc_ty/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use rustc_data_structures::svh::Svh;
use rustc_hir as hir;
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE};
use rustc_middle::hir::map as hir_map;
use rustc_middle::mir::StatementKind;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, WithConstness};
use rustc_session::CrateDisambiguator;
Expand Down Expand Up @@ -299,7 +300,23 @@ fn instance_def_size_estimate<'tcx>(
match instance_def {
InstanceDef::Item(..) | InstanceDef::DropGlue(..) => {
let mir = tcx.instance_mir(instance_def);
mir.basic_blocks().iter().map(|bb| bb.statements.len()).sum()
mir.basic_blocks()
.iter()
.map(|bb| {
bb.statements
.iter()
.filter(|stmt| match stmt.kind {
StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
| StatementKind::AscribeUserType(..)
| StatementKind::FakeRead(..)
| StatementKind::Retag(..)
| StatementKind::Nop => false,
_ => true,
})
.count()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall there maybe be something like

let term = blk.terminator();
let mut is_drop = false;
match term.kind {
TerminatorKind::Drop { ref location, target, unwind }
| TerminatorKind::DropAndReplace { ref location, target, unwind, .. } => {
is_drop = true;
work_list.push(target);
// If the location doesn't actually need dropping, treat it like
// a regular goto.
let ty = location.ty(callee_body, tcx).subst(tcx, callsite.substs).ty;
if ty.needs_drop(tcx, param_env) {
cost += CALL_PENALTY;
if let Some(unwind) = unwind {
cost += LANDINGPAD_PENALTY;
work_list.push(unwind);
}
} else {
cost += INSTR_COST;
}
}
TerminatorKind::Unreachable | TerminatorKind::Call { destination: None, .. }
if first_block =>
{
// If the function always diverges, don't inline
// unless the cost is zero
threshold = 0;
}
TerminatorKind::Call { func: Operand::Constant(ref f), cleanup, .. } => {
if let ty::FnDef(def_id, _) = f.literal.ty.kind {
// Don't give intrinsics the extra penalty for calls
let f = tcx.fn_sig(def_id);
if f.abi() == Abi::RustIntrinsic || f.abi() == Abi::PlatformIntrinsic {
cost += INSTR_COST;
} else {
cost += CALL_PENALTY;
}
} else {
cost += CALL_PENALTY;
}
if cleanup.is_some() {
cost += LANDINGPAD_PENALTY;
}
}
TerminatorKind::Assert { cleanup, .. } => {
cost += CALL_PENALTY;
if cleanup.is_some() {
cost += LANDINGPAD_PENALTY;
}
}
TerminatorKind::Resume => cost += RESUME_PENALTY,
_ => cost += INSTR_COST,
}

or at least a +1 so the terminator is counted in some way

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can maybe be simplified to only add extra for landingpads(10 llvm ir instructions) and resume (9 llvm ir instructions)

})
.sum()
}
// Estimate the size of other compiler-generated shims to be 1.
_ => 1,
Expand Down