Skip to content

Commit

Permalink
Auto merge of rust-lang#123488 - oli-obk:required_items_query, r=<try>
Browse files Browse the repository at this point in the history
Create a separate query for required and mentioned items instead of tracking them in the MIR body

implements rust-lang#122862 (comment)

May permit further improvements without sacrificing perf... iff this PR isn't horrible for perf 🙃
  • Loading branch information
bors committed Apr 5, 2024
2 parents ea40fa2 + 27f29d1 commit fdf29c5
Show file tree
Hide file tree
Showing 27 changed files with 662 additions and 153 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

// Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
if M::POST_MONO_CHECKS {
for &const_ in &body.required_consts {
for &const_ in &self.tcx.required_and_mentioned_items(instance.def).required_consts {
let c = self
.instantiate_from_current_frame_and_normalize_erasing_regions(const_.const_)?;
c.eval(*self.tcx, self.param_env, const_.span).map_err(|err| {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,15 @@ provide! { tcx, def_id, other, cdata,
object_lifetime_default => { table }
thir_abstract_const => { table }
optimized_mir => { table }
required_and_mentioned_items_of_item => {
cdata
.root
.tables
.required_and_mentioned_items_of_item
.get(cdata, def_id.index)
.map(|lazy| lazy.decode((cdata, tcx)))
.unwrap_or_default()
}
mir_for_ctfe => { table }
closure_saved_names_of_captured_variables => { table }
mir_coroutine_witnesses => { table }
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_hir::definitions::DefPathData;
use rustc_hir_pretty::id_to_string;
use rustc_middle::middle::dependency_format::Linkage;
use rustc_middle::middle::exported_symbols::metadata_symbol_name;
use rustc_middle::mir::interpret;
use rustc_middle::mir::{interpret, RequiredAndMentionedItems};
use rustc_middle::query::LocalCrate;
use rustc_middle::query::Providers;
use rustc_middle::traits::specialization_graph;
Expand Down Expand Up @@ -1648,6 +1648,12 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
for (def_id, encode_const, encode_opt) in keys_and_jobs {
debug_assert!(encode_const || encode_opt);

let items = tcx.required_and_mentioned_items_of_item(def_id);
let RequiredAndMentionedItems { required_consts, mentioned_items } = items;
if !required_consts.is_empty() || !mentioned_items.is_empty() {
record!(self.tables.required_and_mentioned_items_of_item[def_id.to_def_id()] <- items);
}

debug!("EntryBuilder::encode_mir({:?})", def_id);
if encode_opt {
record!(self.tables.optimized_mir[def_id.to_def_id()] <- tcx.optimized_mir(def_id));
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ define_tables! {
const_param_default: Table<DefIndex, LazyValue<ty::EarlyBinder<rustc_middle::ty::Const<'static>>>>,
object_lifetime_default: Table<DefIndex, LazyValue<ObjectLifetimeDefault>>,
optimized_mir: Table<DefIndex, LazyValue<mir::Body<'static>>>,
required_and_mentioned_items_of_item: Table<DefIndex, LazyValue<mir::RequiredAndMentionedItems<'static>>>,
mir_for_ctfe: Table<DefIndex, LazyValue<mir::Body<'static>>>,
closure_saved_names_of_captured_variables: Table<DefIndex, LazyValue<IndexVec<FieldIdx, Symbol>>>,
mir_coroutine_witnesses: Table<DefIndex, LazyValue<mir::CoroutineLayout<'static>>>,
Expand Down
42 changes: 2 additions & 40 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use rustc_hir::{
ImplicitSelfKind,
};
use rustc_session::Session;
use rustc_span::source_map::Spanned;
use rustc_target::abi::{FieldIdx, VariantIdx};

use polonius_engine::Atom;
Expand Down Expand Up @@ -61,6 +60,7 @@ pub mod mono;
pub mod patch;
pub mod pretty;
mod query;
mod required_items;
mod statement;
mod syntax;
pub mod tcx;
Expand All @@ -77,6 +77,7 @@ pub use self::pretty::{
};
pub use consts::*;
use pretty::pretty_print_const_value;
pub use required_items::{MentionedItem, RequiredAndMentionedItems};
pub use statement::*;
pub use syntax::*;
pub use terminator::*;
Expand Down Expand Up @@ -308,21 +309,6 @@ impl<'tcx> CoroutineInfo<'tcx> {
}
}

/// Some item that needs to monomorphize successfully for a MIR body to be considered well-formed.
#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, HashStable, TyEncodable, TyDecodable)]
#[derive(TypeFoldable, TypeVisitable)]
pub enum MentionedItem<'tcx> {
/// A function that gets called. We don't necessarily know its precise type yet, since it can be
/// hidden behind a generic.
Fn(Ty<'tcx>),
/// A type that has its drop shim called.
Drop(Ty<'tcx>),
/// Unsizing casts might require vtables, so we have to record them.
UnsizeCast { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> },
/// A closure that is coerced to a function pointer.
Closure(Ty<'tcx>),
}

/// The lowered representation of a single function.
#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable, TypeFoldable, TypeVisitable)]
pub struct Body<'tcx> {
Expand Down Expand Up @@ -384,26 +370,6 @@ pub struct Body<'tcx> {
/// A span representing this MIR, for error reporting.
pub span: Span,

/// Constants that are required to evaluate successfully for this MIR to be well-formed.
/// We hold in this field all the constants we are not able to evaluate yet.
///
/// This is soundness-critical, we make a guarantee that all consts syntactically mentioned in a
/// function have successfully evaluated if the function ever gets executed at runtime.
pub required_consts: Vec<ConstOperand<'tcx>>,

/// Further items that were mentioned in this function and hence *may* become monomorphized,
/// depending on optimizations. We use this to avoid optimization-dependent compile errors: the
/// collector recursively traverses all "mentioned" items and evaluates all their
/// `required_consts`.
///
/// This is *not* soundness-critical and the contents of this list are *not* a stable guarantee.
/// All that's relevant is that this set is optimization-level-independent, and that it includes
/// everything that the collector would consider "used". (For example, we currently compute this
/// set after drop elaboration, so some drop calls that can never be reached are not considered
/// "mentioned".) See the documentation of `CollectionMode` in
/// `compiler/rustc_monomorphize/src/collector.rs` for more context.
pub mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>,

/// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
///
/// Note that this does not actually mean that this body is not computable right now.
Expand Down Expand Up @@ -479,8 +445,6 @@ impl<'tcx> Body<'tcx> {
spread_arg: None,
var_debug_info,
span,
required_consts: Vec::new(),
mentioned_items: Vec::new(),
is_polymorphic: false,
injection_phase: None,
tainted_by_errors,
Expand Down Expand Up @@ -509,8 +473,6 @@ impl<'tcx> Body<'tcx> {
arg_count: 0,
spread_arg: None,
span: DUMMY_SP,
required_consts: Vec::new(),
mentioned_items: Vec::new(),
var_debug_info: Vec::new(),
is_polymorphic: false,
injection_phase: None,
Expand Down
68 changes: 68 additions & 0 deletions compiler/rustc_middle/src/mir/required_items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use rustc_span::source_map::Spanned;

use crate::ty::{self, Ty, TyCtxt};

use super::ConstOperand;

#[derive(Debug, HashStable, TyEncodable, TyDecodable, Default)]
pub struct RequiredAndMentionedItems<'tcx> {
/// Constants that are required to evaluate successfully for this MIR to be well-formed.
/// We hold in this field all the constants we are not able to evaluate yet.
///
/// This is soundness-critical, we make a guarantee that all consts syntactically mentioned in a
/// function have successfully evaluated if the function ever gets executed at runtime.
pub required_consts: Vec<ConstOperand<'tcx>>,

/// Further items that were mentioned in this function and hence *may* become monomorphized,
/// depending on optimizations. We use this to avoid optimization-dependent compile errors: the
/// collector recursively traverses all "mentioned" items and evaluates all their
/// `required_consts`.
///
/// This is *not* soundness-critical and the contents of this list are *not* a stable guarantee.
/// All that's relevant is that this set is optimization-level-independent, and that it includes
/// everything that the collector would consider "used". (For example, we currently compute this
/// set after drop elaboration, so some drop calls that can never be reached are not considered
/// "mentioned".) See the documentation of `CollectionMode` in
/// `compiler/rustc_monomorphize/src/collector.rs` for more context.
pub mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>,
}

/// Some item that needs to monomorphize successfully for a MIR body to be considered well-formed.
#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, HashStable, TyEncodable, TyDecodable)]
#[derive(TypeFoldable, TypeVisitable)]
pub enum MentionedItem<'tcx> {
/// A function that gets called. We don't necessarily know its precise type yet, since it can be
/// hidden behind a generic.
Fn(Ty<'tcx>),
/// A type that has its drop shim called.
Drop(Ty<'tcx>),
/// Unsizing casts might require vtables, so we have to record them.
UnsizeCast { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> },
/// A closure that is coerced to a function pointer.
Closure(Ty<'tcx>),
}

impl<'tcx> TyCtxt<'tcx> {
pub fn required_and_mentioned_items(
self,
key: ty::InstanceDef<'tcx>,
) -> &'tcx RequiredAndMentionedItems<'tcx> {
match key {
ty::InstanceDef::Item(id) => self.required_and_mentioned_items_of_item(id),
ty::InstanceDef::Intrinsic(_)
| ty::InstanceDef::VTableShim(_)
| ty::InstanceDef::ReifyShim(..)
| ty::InstanceDef::FnPtrShim(_, _)
| ty::InstanceDef::Virtual(_, _)
| ty::InstanceDef::ClosureOnceShim { .. }
| ty::InstanceDef::ConstructCoroutineInClosureShim { .. }
| ty::InstanceDef::CoroutineKindShim { .. }
| ty::InstanceDef::ThreadLocalShim(_)
| ty::InstanceDef::DropGlue(_, _)
| ty::InstanceDef::CloneShim(_, _)
| ty::InstanceDef::FnPtrAddrShim(_, _) => {
self.required_and_mentioned_items_of_shim(key)
}
}
}
}
5 changes: 0 additions & 5 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,11 +1050,6 @@ macro_rules! super_body {
}

$self.visit_span($(& $mutability)? $body.span);

for const_ in &$($mutability)? $body.required_consts {
let location = Location::START;
$self.visit_constant(const_, location);
}
}
}

Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,21 @@ rustc_queries! {
separate_provide_extern
}

/// Cross-crate cache of `required_and_mentioned_items`. Do not call directly.
query required_and_mentioned_items_of_item(key: DefId) -> &'tcx mir::RequiredAndMentionedItems<'tcx> {
desc { |tcx| "computing required and mentioned items for `{}`", tcx.def_path_str(key) }
cache_on_disk_if { key.is_local() }
arena_cache
separate_provide_extern
}

/// Internal implementation detail of `required_and_mentioned_items`. Do not call directly.
query required_and_mentioned_items_of_shim(key: ty::InstanceDef<'tcx>) -> &'tcx mir::RequiredAndMentionedItems<'tcx> {
desc { |tcx| "computing required and mentioned items for `{}`", tcx.def_path_str(key.def_id()) }
cache_on_disk_if { true }
arena_cache
}

/// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass
/// (for compiler option `-Cinstrument-coverage`), after MIR optimizations
/// have had a chance to potentially remove some of them.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/parameterized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ macro_rules! parameterized_over_tcx {
parameterized_over_tcx! {
crate::middle::exported_symbols::ExportedSymbol,
crate::mir::Body,
crate::mir::RequiredAndMentionedItems,
crate::mir::CoroutineLayout,
crate::mir::interpret::ConstAllocation,
ty::Ty,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_mir_build/src/build/custom/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ pub(super) fn build_custom_mir<'tcx>(
spread_arg: None,
var_debug_info: Vec::new(),
span,
required_consts: Vec::new(),
mentioned_items: Vec::new(),
is_polymorphic: false,
tainted_by_errors: None,
injection_phase: None,
Expand Down
34 changes: 1 addition & 33 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,8 +566,7 @@ impl<'tcx> Inliner<'tcx> {
mut callee_body: Body<'tcx>,
) {
let terminator = caller_body[callsite.block].terminator.take().unwrap();
let TerminatorKind::Call { func, args, destination, unwind, target, .. } = terminator.kind
else {
let TerminatorKind::Call { args, destination, unwind, target, .. } = terminator.kind else {
bug!("unexpected terminator kind {:?}", terminator.kind);
};

Expand Down Expand Up @@ -706,37 +705,6 @@ impl<'tcx> Inliner<'tcx> {
source_info: callsite.source_info,
kind: TerminatorKind::Goto { target: integrator.map_block(START_BLOCK) },
});

// Copy only unevaluated constants from the callee_body into the caller_body.
// Although we are only pushing `ConstKind::Unevaluated` consts to
// `required_consts`, here we may not only have `ConstKind::Unevaluated`
// because we are calling `instantiate_and_normalize_erasing_regions`.
caller_body.required_consts.extend(callee_body.required_consts.iter().copied().filter(
|&ct| match ct.const_ {
Const::Ty(_) => {
bug!("should never encounter ty::UnevaluatedConst in `required_consts`")
}
Const::Val(..) | Const::Unevaluated(..) => true,
},
));
// Now that we incorporated the callee's `required_consts`, we can remove the callee from
// `mentioned_items` -- but we have to take their `mentioned_items` in return. This does
// some extra work here to save the monomorphization collector work later. It helps a lot,
// since monomorphization can avoid a lot of work when the "mentioned items" are similar to
// the actually used items. By doing this we can entirely avoid visiting the callee!
// We need to reconstruct the `required_item` for the callee so that we can find and
// remove it.
let callee_item = MentionedItem::Fn(func.ty(caller_body, self.tcx));
if let Some(idx) =
caller_body.mentioned_items.iter().position(|item| item.node == callee_item)
{
// We found the callee, so remove it and add its items instead.
caller_body.mentioned_items.remove(idx);
caller_body.mentioned_items.extend(callee_body.mentioned_items);
} else {
// If we can't find the callee, there's no point in adding its items.
// Probably it already got removed by being inlined elsewhere in the same function.
}
}

fn make_call_args(
Expand Down
Loading

0 comments on commit fdf29c5

Please sign in to comment.