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

Create a separate query for required and mentioned items instead of tracking them in the MIR body #123488

Closed
wants to merge 2 commits into from
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
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,
},
));
Copy link
Member

Choose a reason for hiding this comment

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

Not incorporating inlined consts can lead to ICEs in the interpreter: we may be inlining a function that contains a const that fails. The interpreter will evaluate all required_consts (which would not include the inlined const) and then assert that all const-eval during function evaluation succeeds.

// 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.
}
Copy link
Member

Choose a reason for hiding this comment

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

Removing this optimization will also not be great for perf, as it means the "mentioned items" traversal has to visit all the functions that got inlined.

}

fn make_call_args(
Expand Down
Loading
Loading