Skip to content

Commit

Permalink
Auto merge of rust-lang#122862 - RalfJung:collect-all-mono, r=<try>
Browse files Browse the repository at this point in the history
collector: always consider all monomorphic functions to be 'mentioned'

This would fix rust-lang#122814. But it's probably not going to be cheap...

Ideally we'd avoid building the optimized MIR for these new roots, and only request `mir_drops_elaborated_and_const_checked` -- but that MIR is often getting stolen so I don't see a way to do that.

TODO before landing:
- [ ] Figure out if there is a testcase [here](rust-lang#122814 (comment)).

r? `@oli-obk` `@tmiasko`
  • Loading branch information
bors committed Mar 22, 2024
2 parents cdb683f + 59803ef commit d0df954
Show file tree
Hide file tree
Showing 19 changed files with 267 additions and 60 deletions.
104 changes: 73 additions & 31 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1513,16 +1513,26 @@ fn collect_const_value<'tcx>(
// Find all non-generic items by walking the HIR. These items serve as roots to
// start monomorphizing from.
#[instrument(skip(tcx, mode), level = "debug")]
fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionStrategy) -> Vec<MonoItem<'_>> {
fn collect_roots(
tcx: TyCtxt<'_>,
mode: MonoItemCollectionStrategy,
) -> Vec<(MonoItem<'_>, CollectionMode)> {
debug!("collecting roots");
let mut roots = Vec::new();
let mut used_roots = MonoItems::new();
let mut mentioned_roots = MonoItems::new();

{
let entry_fn = tcx.entry_fn(());

debug!("collect_roots: entry_fn = {:?}", entry_fn);

let mut collector = RootCollector { tcx, strategy: mode, entry_fn, output: &mut roots };
let mut collector = RootCollector {
tcx,
strategy: mode,
entry_fn,
used_roots: &mut used_roots,
mentioned_roots: &mut mentioned_roots,
};

let crate_items = tcx.hir_crate_items(());

Expand All @@ -1537,21 +1547,30 @@ fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionStrategy) -> Vec<MonoI
collector.push_extra_entry_roots();
}

// Chain the two root lists together. Used items go first, to make it
// more likely that when we visit a mentioned item, we can stop immediately as it was already used.
// We can only codegen items that are instantiable - items all of
// whose predicates hold. Luckily, items that aren't instantiable
// can't actually be used, so we can just skip codegenning them.
roots
used_roots
.into_iter()
.filter_map(|Spanned { node: mono_item, .. }| {
mono_item.is_instantiable(tcx).then_some(mono_item)
})
.map(|mono_item| (mono_item.node, CollectionMode::UsedItems))
.chain(
mentioned_roots
.into_iter()
.map(|mono_item| (mono_item.node, CollectionMode::MentionedItems)),
)
.filter(|(mono_item, _mode)| mono_item.is_instantiable(tcx))
.collect()
}

struct RootCollector<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
strategy: MonoItemCollectionStrategy,
output: &'a mut MonoItems<'tcx>,
// `MonoItems` includes spans we don't actually want... but this lets us reuse some of the
// collector's functions.
used_roots: &'a mut MonoItems<'tcx>,
mentioned_roots: &'a mut MonoItems<'tcx>,
entry_fn: Option<(DefId, EntryFnType)>,
}

Expand All @@ -1565,33 +1584,33 @@ impl<'v> RootCollector<'_, 'v> {
debug!("RootCollector: ADT drop-glue for `{id:?}`",);

let ty = self.tcx.type_of(id.owner_id.to_def_id()).no_bound_vars().unwrap();
visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.output);
visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.used_roots);
}
}
DefKind::GlobalAsm => {
debug!(
"RootCollector: ItemKind::GlobalAsm({})",
self.tcx.def_path_str(id.owner_id)
);
self.output.push(dummy_spanned(MonoItem::GlobalAsm(id)));
self.used_roots.push(dummy_spanned(MonoItem::GlobalAsm(id)));
}
DefKind::Static { .. } => {
let def_id = id.owner_id.to_def_id();
debug!("RootCollector: ItemKind::Static({})", self.tcx.def_path_str(def_id));
self.output.push(dummy_spanned(MonoItem::Static(def_id)));
self.used_roots.push(dummy_spanned(MonoItem::Static(def_id)));
}
DefKind::Const => {
// const items only generate mono items if they are
// actually used somewhere. Just declaring them is insufficient.

// but even just declaring them must collect the items they refer to
if let Ok(val) = self.tcx.const_eval_poly(id.owner_id.to_def_id()) {
collect_const_value(self.tcx, val, self.output);
collect_const_value(self.tcx, val, self.used_roots);
}
}
DefKind::Impl { .. } => {
if self.strategy == MonoItemCollectionStrategy::Eager {
create_mono_items_for_default_impls(self.tcx, id, self.output);
create_mono_items_for_default_impls(self.tcx, id, self.used_roots);
}
}
DefKind::Fn => {
Expand All @@ -1607,31 +1626,54 @@ impl<'v> RootCollector<'_, 'v> {
}
}

fn is_root(&self, def_id: LocalDefId) -> bool {
!self.tcx.generics_of(def_id).requires_monomorphization(self.tcx)
&& match self.strategy {
MonoItemCollectionStrategy::Eager => true,
MonoItemCollectionStrategy::Lazy => {
self.entry_fn.and_then(|(id, _)| id.as_local()) == Some(def_id)
|| self.tcx.is_reachable_non_generic(def_id)
|| self
.tcx
.codegen_fn_attrs(def_id)
.flags
.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
}
/// Determines whether this is an item we start walking, and in which mode. The "real" roots are
/// walked as "used" items, but that set is optimization-dependent. We add all other non-generic
/// items as "mentioned" roots. This makes the set of items where `is_root` return `Some`
/// optimization-independent, which is crucial to ensure that optimized and unoptimized builds
/// evaluate the same constants.
fn is_root(&self, def_id: LocalDefId) -> Option<CollectionMode> {
// Generic things are never roots.
if self.tcx.generics_of(def_id).requires_monomorphization(self.tcx) {
return None;
}
// We have to skip `must_be_overridden` bodies; asking for their MIR ICEs.
if self.tcx.intrinsic(def_id).is_some_and(|i| i.must_be_overridden) {
return None;
}
// The rest is definitely a root, but is it used or merely mentioned?
// Determine whether this item is reachable, which makes it "used".
let is_used_root = match self.strategy {
MonoItemCollectionStrategy::Eager => true,
MonoItemCollectionStrategy::Lazy => {
self.entry_fn.and_then(|(id, _)| id.as_local()) == Some(def_id)
|| self.tcx.is_reachable_non_generic(def_id)
|| self
.tcx
.codegen_fn_attrs(def_id)
.flags
.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
}
};
if is_used_root {
Some(CollectionMode::UsedItems)
} else {
Some(CollectionMode::MentionedItems)
}
}

/// If `def_id` represents a root, pushes it onto the list of
/// outputs. (Note that all roots must be monomorphic.)
#[instrument(skip(self), level = "debug")]
fn push_if_root(&mut self, def_id: LocalDefId) {
if self.is_root(def_id) {
if let Some(mode) = self.is_root(def_id) {
debug!("found root");

let instance = Instance::mono(self.tcx, def_id.to_def_id());
self.output.push(create_fn_mono_item(self.tcx, instance, DUMMY_SP));
let mono_item = create_fn_mono_item(self.tcx, instance, DUMMY_SP);
match mode {
CollectionMode::UsedItems => self.used_roots.push(mono_item),
CollectionMode::MentionedItems => self.mentioned_roots.push(mono_item),
}
}
}

Expand Down Expand Up @@ -1667,7 +1709,7 @@ impl<'v> RootCollector<'_, 'v> {
self.tcx.mk_args(&[main_ret_ty.into()]),
);

self.output.push(create_fn_mono_item(self.tcx, start_instance, DUMMY_SP));
self.used_roots.push(create_fn_mono_item(self.tcx, start_instance, DUMMY_SP));
}
}

Expand Down Expand Up @@ -1772,15 +1814,15 @@ pub fn collect_crate_mono_items(
let state: LRef<'_, _> = &mut state;

tcx.sess.time("monomorphization_collector_graph_walk", || {
par_for_each_in(roots, |root| {
par_for_each_in(roots, |(root, mode)| {
let mut recursion_depths = DefIdMap::default();
collect_items_rec(
tcx,
dummy_spanned(root),
state,
&mut recursion_depths,
recursion_limit,
CollectionMode::UsedItems,
mode,
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion tests/incremental/callee_caller_cross_crate/b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

extern crate a;

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
pub fn call_function0() {
a::function0(77);
}
Expand Down
1 change: 1 addition & 0 deletions tests/incremental/dirty_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ mod y {
//[cfail2]~| ERROR `type_of(y)` should be dirty but is not
//[cfail2]~| ERROR `fn_sig(y)` should be dirty but is not
//[cfail2]~| ERROR `typeck(y)` should be clean but is not
//[cfail2]~| ERROR `optimized_mir(y)` should be clean but is not
x::x();
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/hashes/call_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ mod change_callee_indirectly_function {
#[cfg(not(any(cfail1,cfail4)))]
use super::callee2 as callee;

#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
pub fn change_callee_indirectly_function() {
callee(1, 2)
Expand Down
28 changes: 14 additions & 14 deletions tests/incremental/hashes/indexing_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ fn change_simple_index(slice: &[u32]) -> u32 {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn change_simple_index(slice: &[u32]) -> u32 {
slice[4]
Expand All @@ -40,9 +40,9 @@ fn change_lower_bound(slice: &[u32]) -> &[u32] {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn change_lower_bound(slice: &[u32]) -> &[u32] {
&slice[2..5]
Expand All @@ -57,9 +57,9 @@ fn change_upper_bound(slice: &[u32]) -> &[u32] {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn change_upper_bound(slice: &[u32]) -> &[u32] {
&slice[3..7]
Expand All @@ -74,9 +74,9 @@ fn add_lower_bound(slice: &[u32]) -> &[u32] {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn add_lower_bound(slice: &[u32]) -> &[u32] {
&slice[3..4]
Expand All @@ -91,9 +91,9 @@ fn add_upper_bound(slice: &[u32]) -> &[u32] {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn add_upper_bound(slice: &[u32]) -> &[u32] {
&slice[3..7]
Expand All @@ -108,9 +108,9 @@ fn change_mutability(slice: &mut [u32]) -> u32 {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn change_mutability(slice: &mut [u32]) -> u32 {
(& slice[3..5])[0]
Expand All @@ -125,9 +125,9 @@ fn exclusive_to_inclusive_range(slice: &[u32]) -> &[u32] {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
fn exclusive_to_inclusive_range(slice: &[u32]) -> &[u32] {
&slice[3..=7]
Expand Down
2 changes: 1 addition & 1 deletion tests/incremental/ich_method_call_trait_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod mod3 {
#[cfg(rpass2)]
use Trait2;

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
fn bar() {
().method();
}
Expand Down
2 changes: 1 addition & 1 deletion tests/incremental/ich_resolve_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ mod mod3 {
}

#[rustc_clean(cfg="rpass2")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="rpass3")]
#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="rpass3")]
fn in_type() {
test::<Foo>();
}
Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/struct_add_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ pub struct Y {
pub y: char
}

#[rustc_clean(except="fn_sig,typeck", cfg="rpass2")]
#[rustc_clean(except="fn_sig,typeck,optimized_mir", cfg="rpass2")]
pub fn use_X(x: X) -> u32 {
x.x as u32
}

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
pub fn use_EmbedX(embed: EmbedX) -> u32 {
embed.x.x as u32
}
Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/struct_change_field_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ pub struct Y {
pub y: char
}

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
pub fn use_X() -> u32 {
let x: X = X { x: 22 };
x.x as u32
}

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
pub fn use_EmbedX(x: EmbedX) -> u32 {
let x: X = X { x: 22 };
x.x as u32
Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/struct_change_field_type_cross_crate/b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ extern crate a;

use a::*;

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
pub fn use_X() -> u32 {
let x: X = X { x: 22 };
x.x as u32
}

#[rustc_clean(except="typeck", cfg="rpass2")]
#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
pub fn use_EmbedX(embed: EmbedX) -> u32 {
embed.x.x as u32
}
Expand Down
Loading

0 comments on commit d0df954

Please sign in to comment.