Skip to content

Commit

Permalink
Auto merge of rust-lang#122568 - RalfJung:mentioned-items, r=oli-obk
Browse files Browse the repository at this point in the history
recursively evaluate the constants in everything that is 'mentioned'

This is another attempt at fixing rust-lang#107503. The previous attempt at rust-lang#112879 seems stuck in figuring out where the [perf regression](https://perf.rust-lang.org/compare.html?start=c55d1ee8d4e3162187214692229a63c2cc5e0f31&end=ec8de1ebe0d698b109beeaaac83e60f4ef8bb7d1&stat=instructions:u) comes from. In  rust-lang#122258 I learned some things, which informed the approach this PR is taking.

Quoting from the new collector docs, which explain the high-level idea:
```rust
//! One important role of collection is to evaluate all constants that are used by all the items
//! which are being collected. Codegen can then rely on only encountering constants that evaluate
//! successfully, and if a constant fails to evaluate, the collector has much better context to be
//! able to show where this constant comes up.
//!
//! However, the exact set of "used" items (collected as described above), and therefore the exact
//! set of used constants, can depend on optimizations. Optimizing away dead code may optimize away
//! a function call that uses a failing constant, so an unoptimized build may fail where an
//! optimized build succeeds. This is undesirable.
//!
//! To fix this, the collector has the concept of "mentioned" items. Some time during the MIR
//! pipeline, before any optimization-level-dependent optimizations, we compute a list of all items
//! that syntactically appear in the code. These are considered "mentioned", and even if they are in
//! dead code and get optimized away (which makes them no longer "used"), they are still
//! "mentioned". For every used item, the collector ensures that all mentioned items, recursively,
//! do not use a failing constant. This is reflected via the [`CollectionMode`], which determines
//! whether we are visiting a used item or merely a mentioned item.
//!
//! The collector and "mentioned items" gathering (which lives in `rustc_mir_transform::mentioned_items`)
//! need to stay in sync in the following sense:
//!
//! - For every item that the collector gather that could eventually lead to build failure (most
//!   likely due to containing a constant that fails to evaluate), a corresponding mentioned item
//!   must be added. This should use the exact same strategy as the ecollector to make sure they are
//!   in sync. However, while the collector works on monomorphized types, mentioned items are
//!   collected on generic MIR -- so any time the collector checks for a particular type (such as
//!   `ty::FnDef`), we have to just onconditionally add this as a mentioned item.
//! - In `visit_mentioned_item`, we then do with that mentioned item exactly what the collector
//!   would have done during regular MIR visiting. Basically you can think of the collector having
//!   two stages, a pre-monomorphization stage and a post-monomorphization stage (usually quite
//!   literally separated by a call to `self.monomorphize`); the pre-monomorphizationn stage is
//!   duplicated in mentioned items gathering and the post-monomorphization stage is duplicated in
//!   `visit_mentioned_item`.
//! - Finally, as a performance optimization, the collector should fill `used_mentioned_item` during
//!   its MIR traversal with exactly what mentioned item gathering would have added in the same
//!   situation. This detects mentioned items that have *not* been optimized away and hence don't
//!   need a dedicated traversal.

enum CollectionMode {
    /// Collect items that are used, i.e., actually needed for codegen.
    ///
    /// Which items are used can depend on optimization levels, as MIR optimizations can remove
    /// uses.
    UsedItems,
    /// Collect items that are mentioned. The goal of this mode is that it is independent of
    /// optimizations: the set of "mentioned" items is computed before optimizations are run.
    ///
    /// The exact contents of this set are *not* a stable guarantee. (For instance, it is currently
    /// computed after drop-elaboration. If we ever do some optimizations even in debug builds, we
    /// might decide to run them before computing mentioned items.) The key property of this set is
    /// that it is optimization-independent.
    MentionedItems,
}
```
And the `mentioned_items` MIR body field docs:
```rust
    /// 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>>>,
```

Fixes rust-lang#107503
  • Loading branch information
bors committed Mar 21, 2024
2 parents 47dd709 + 8c01b85 commit df8ac8f
Show file tree
Hide file tree
Showing 52 changed files with 1,370 additions and 380 deletions.
9 changes: 6 additions & 3 deletions compiler/rustc_data_structures/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
//! | ----------------------- | ------------------- | ------------------------------- |
//! | `Lrc<T>` | `rc::Rc<T>` | `sync::Arc<T>` |
//! |` Weak<T>` | `rc::Weak<T>` | `sync::Weak<T>` |
//! | `LRef<'a, T>` [^2] | `&'a mut T` | `&'a T` |
//! | | | |
//! | `AtomicBool` | `Cell<bool>` | `atomic::AtomicBool` |
//! | `AtomicU32` | `Cell<u32>` | `atomic::AtomicU32` |
Expand All @@ -38,7 +39,7 @@
//! of a `RefCell`. This is appropriate when interior mutability is not
//! required.
//!
//! [^2] `MTLockRef` is a typedef.
//! [^2] `MTRef`, `MTLockRef` are type aliases.

pub use crate::marker::*;
use std::collections::HashMap;
Expand Down Expand Up @@ -208,7 +209,7 @@ cfg_match! {

use std::cell::RefCell as InnerRwLock;

pub type MTLockRef<'a, T> = &'a mut MTLock<T>;
pub type LRef<'a, T> = &'a mut T;

#[derive(Debug, Default)]
pub struct MTLock<T>(T);
Expand Down Expand Up @@ -274,7 +275,7 @@ cfg_match! {
pub use std::sync::Arc as Lrc;
pub use std::sync::Weak as Weak;

pub type MTLockRef<'a, T> = &'a MTLock<T>;
pub type LRef<'a, T> = &'a T;

#[derive(Debug, Default)]
pub struct MTLock<T>(Lock<T>);
Expand Down Expand Up @@ -314,6 +315,8 @@ cfg_match! {
}
}

pub type MTLockRef<'a, T> = LRef<'a, MTLock<T>>;

#[derive(Default)]
#[cfg_attr(parallel_compiler, repr(align(64)))]
pub struct CacheAligned<T>(pub T);
Expand Down
46 changes: 46 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
use rustc_hir::{self, CoroutineDesugaring, CoroutineKind, ImplicitSelfKind};
use rustc_hir::{self as hir, HirId};
use rustc_session::Session;
use rustc_span::source_map::Spanned;
use rustc_target::abi::{FieldIdx, VariantIdx};

use polonius_engine::Atom;
Expand All @@ -44,6 +45,7 @@ use std::ops::{Index, IndexMut};
use std::{iter, mem};

pub use self::query::*;
use self::visit::TyContext;
pub use basic_blocks::BasicBlocks;

mod basic_blocks;
Expand Down Expand Up @@ -304,6 +306,21 @@ 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 @@ -367,8 +384,24 @@ pub struct Body<'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>>>,

/// 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 @@ -445,6 +478,7 @@ impl<'tcx> Body<'tcx> {
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 @@ -474,6 +508,7 @@ impl<'tcx> Body<'tcx> {
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 Expand Up @@ -568,6 +603,17 @@ impl<'tcx> Body<'tcx> {
}
}

pub fn span_for_ty_context(&self, ty_context: TyContext) -> Span {
match ty_context {
TyContext::UserTy(span) => span,
TyContext::ReturnTy(source_info)
| TyContext::LocalDecl { source_info, .. }
| TyContext::YieldTy(source_info)
| TyContext::ResumeTy(source_info) => source_info.span,
TyContext::Location(loc) => self.source_info(loc).span,
}
}

/// Returns the return type; it always return first element from `local_decls` array.
#[inline]
pub fn return_ty(&self) -> Ty<'tcx> {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/custom/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub(super) fn build_custom_mir<'tcx>(
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
21 changes: 20 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ impl<'tcx> Inliner<'tcx> {
mut callee_body: Body<'tcx>,
) {
let terminator = caller_body[callsite.block].terminator.take().unwrap();
let TerminatorKind::Call { args, destination, unwind, target, .. } = terminator.kind else {
let TerminatorKind::Call { func, args, destination, unwind, target, .. } = terminator.kind
else {
bug!("unexpected terminator kind {:?}", terminator.kind);
};

Expand Down Expand Up @@ -717,6 +718,24 @@ impl<'tcx> Inliner<'tcx> {
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
5 changes: 5 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ mod lint;
mod lower_intrinsics;
mod lower_slice_len;
mod match_branches;
mod mentioned_items;
mod multiple_return_terminators;
mod normalize_array_len;
mod nrvo;
Expand Down Expand Up @@ -565,6 +566,10 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
tcx,
body,
&[
// Before doing anything, remember which items are being mentioned so that the set of items
// visited does not depend on the optimization level.
&mentioned_items::MentionedItems,
// Add some UB checks before any UB gets optimized away.
&check_alignment::CheckAlignment,
// Before inlining: trim down MIR with passes to reduce inlining work.

Expand Down
117 changes: 117 additions & 0 deletions compiler/rustc_mir_transform/src/mentioned_items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, Location, MentionedItem, MirPass};
use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt};
use rustc_session::Session;
use rustc_span::source_map::Spanned;

pub struct MentionedItems;

struct MentionedItemsVisitor<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
body: &'a mir::Body<'tcx>,
mentioned_items: &'a mut Vec<Spanned<MentionedItem<'tcx>>>,
}

impl<'tcx> MirPass<'tcx> for MentionedItems {
fn is_enabled(&self, _sess: &Session) -> bool {
// If this pass is skipped the collector assume that nothing got mentioned! We could
// potentially skip it in opt-level 0 if we are sure that opt-level will never *remove* uses
// of anything, but that still seems fragile. Furthermore, even debug builds use level 1, so
// special-casing level 0 is just not worth it.
true
}

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) {
debug_assert!(body.mentioned_items.is_empty());
let mut mentioned_items = Vec::new();
MentionedItemsVisitor { tcx, body, mentioned_items: &mut mentioned_items }.visit_body(body);
body.mentioned_items = mentioned_items;
}
}

// This visitor is carefully in sync with the one in `rustc_monomorphize::collector`. We are
// visiting the exact same places but then instead of monomorphizing and creating `MonoItems`, we
// have to remain generic and just recording the relevant information in `mentioned_items`, where it
// will then be monomorphized later during "mentioned items" collection.
impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
self.super_terminator(terminator, location);
let span = || self.body.source_info(location).span;
match &terminator.kind {
mir::TerminatorKind::Call { func, .. } => {
let callee_ty = func.ty(self.body, self.tcx);
self.mentioned_items
.push(Spanned { node: MentionedItem::Fn(callee_ty), span: span() });
}
mir::TerminatorKind::Drop { place, .. } => {
let ty = place.ty(self.body, self.tcx).ty;
self.mentioned_items.push(Spanned { node: MentionedItem::Drop(ty), span: span() });
}
mir::TerminatorKind::InlineAsm { ref operands, .. } => {
for op in operands {
match *op {
mir::InlineAsmOperand::SymFn { ref value } => {
self.mentioned_items.push(Spanned {
node: MentionedItem::Fn(value.const_.ty()),
span: span(),
});
}
_ => {}
}
}
}
_ => {}
}
}

fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
self.super_rvalue(rvalue, location);
let span = || self.body.source_info(location).span;
match *rvalue {
// We need to detect unsizing casts that required vtables.
mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::Unsize),
ref operand,
target_ty,
)
| mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => {
// This isn't monomorphized yet so we can't tell what the actual types are -- just
// add everything that may involve a vtable.
let source_ty = operand.ty(self.body, self.tcx);
let may_involve_vtable = match (
source_ty.builtin_deref(true).map(|t| t.ty.kind()),
target_ty.builtin_deref(true).map(|t| t.ty.kind()),
) {
(Some(ty::Array(..)), Some(ty::Str | ty::Slice(..))) => false, // &str/&[T] unsizing
_ => true,
};
if may_involve_vtable {
self.mentioned_items.push(Spanned {
node: MentionedItem::UnsizeCast { source_ty, target_ty },
span: span(),
});
}
}
// Similarly, record closures that are turned into function pointers.
mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_)),
ref operand,
_,
) => {
let source_ty = operand.ty(self.body, self.tcx);
self.mentioned_items
.push(Spanned { node: MentionedItem::Closure(source_ty), span: span() });
}
// And finally, function pointer reification casts.
mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer),
ref operand,
_,
) => {
let fn_ty = operand.ty(self.body, self.tcx);
self.mentioned_items.push(Spanned { node: MentionedItem::Fn(fn_ty), span: span() });
}
_ => {}
}
}
}
4 changes: 3 additions & 1 deletion compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::iter;

use crate::{
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator,
pass_manager as pm, remove_noop_landing_pads, simplify,
mentioned_items, pass_manager as pm, remove_noop_landing_pads, simplify,
};
use rustc_middle::mir::patch::MirPatch;
use rustc_mir_dataflow::elaborate_drops::{self, DropElaborator, DropFlagMode, DropStyle};
Expand Down Expand Up @@ -112,6 +112,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
tcx,
&mut body,
&[
&mentioned_items::MentionedItems,
&abort_unwinding_calls::AbortUnwindingCalls,
&add_call_guards::CriticalCallEdges,
],
Expand Down Expand Up @@ -143,6 +144,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
tcx,
&mut result,
&[
&mentioned_items::MentionedItems,
&add_moves_for_packed_drops::AddMovesForPackedDrops,
&deref_separator::Derefer,
&remove_noop_landing_pads::RemoveNoopLandingPads,
Expand Down
Loading

0 comments on commit df8ac8f

Please sign in to comment.