MIR required_consts, mentioned_items: ensure we do not forget to fill these lists

This commit is contained in:
Ralf Jung 2024-08-01 14:01:17 +02:00
parent 70591dc15d
commit 6d312d7bd1
11 changed files with 113 additions and 44 deletions

View File

@ -888,7 +888,7 @@ fn after_stack_frame_push(
body: &'tcx mir::Body<'tcx>, body: &'tcx mir::Body<'tcx>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
// Make sure all the constants required by this frame evaluate successfully (post-monomorphization check). // Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
for &const_ in &body.required_consts { for &const_ in body.required_consts() {
let c = let c =
self.instantiate_from_current_frame_and_normalize_erasing_regions(const_.const_)?; self.instantiate_from_current_frame_and_normalize_erasing_regions(const_.const_)?;
c.eval(*self.tcx, self.param_env, const_.span).map_err(|err| { c.eval(*self.tcx, self.param_env, const_.span).map_err(|err| {

View File

@ -383,15 +383,17 @@ pub struct Body<'tcx> {
/// Constants that are required to evaluate successfully for this MIR to be well-formed. /// 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. /// We hold in this field all the constants we are not able to evaluate yet.
/// `None` indicates that the list has not been computed yet.
/// ///
/// This is soundness-critical, we make a guarantee that all consts syntactically mentioned in a /// 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. /// function have successfully evaluated if the function ever gets executed at runtime.
pub required_consts: Vec<ConstOperand<'tcx>>, pub required_consts: Option<Vec<ConstOperand<'tcx>>>,
/// Further items that were mentioned in this function and hence *may* become monomorphized, /// 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 /// depending on optimizations. We use this to avoid optimization-dependent compile errors: the
/// collector recursively traverses all "mentioned" items and evaluates all their /// collector recursively traverses all "mentioned" items and evaluates all their
/// `required_consts`. /// `required_consts`.
/// `None` indicates that the list has not been computed yet.
/// ///
/// This is *not* soundness-critical and the contents of this list are *not* a stable guarantee. /// 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 /// All that's relevant is that this set is optimization-level-independent, and that it includes
@ -399,7 +401,7 @@ pub struct Body<'tcx> {
/// set after drop elaboration, so some drop calls that can never be reached are not considered /// set after drop elaboration, so some drop calls that can never be reached are not considered
/// "mentioned".) See the documentation of `CollectionMode` in /// "mentioned".) See the documentation of `CollectionMode` in
/// `compiler/rustc_monomorphize/src/collector.rs` for more context. /// `compiler/rustc_monomorphize/src/collector.rs` for more context.
pub mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>, pub mentioned_items: Option<Vec<Spanned<MentionedItem<'tcx>>>>,
/// Does this body use generic parameters. This is used for the `ConstEvaluatable` check. /// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
/// ///
@ -477,8 +479,8 @@ pub fn new(
spread_arg: None, spread_arg: None,
var_debug_info, var_debug_info,
span, span,
required_consts: Vec::new(), required_consts: None,
mentioned_items: Vec::new(), mentioned_items: None,
is_polymorphic: false, is_polymorphic: false,
injection_phase: None, injection_phase: None,
tainted_by_errors, tainted_by_errors,
@ -507,8 +509,8 @@ pub fn new_cfg_only(basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>) ->
arg_count: 0, arg_count: 0,
spread_arg: None, spread_arg: None,
span: DUMMY_SP, span: DUMMY_SP,
required_consts: Vec::new(), required_consts: None,
mentioned_items: Vec::new(), mentioned_items: None,
var_debug_info: Vec::new(), var_debug_info: Vec::new(),
is_polymorphic: false, is_polymorphic: false,
injection_phase: None, injection_phase: None,
@ -785,6 +787,40 @@ pub fn caller_location_span<T>(
// No inlined `SourceScope`s, or all of them were `#[track_caller]`. // No inlined `SourceScope`s, or all of them were `#[track_caller]`.
caller_location.unwrap_or_else(|| from_span(source_info.span)) caller_location.unwrap_or_else(|| from_span(source_info.span))
} }
#[track_caller]
pub fn set_required_consts(&mut self, required_consts: Vec<ConstOperand<'tcx>>) {
assert!(
self.required_consts.is_none(),
"required_consts for {:?} have already been set",
self.source.def_id()
);
self.required_consts = Some(required_consts);
}
#[track_caller]
pub fn required_consts(&self) -> &[ConstOperand<'tcx>] {
match &self.required_consts {
Some(l) => l,
None => panic!("required_consts for {:?} have not yet been set", self.source.def_id()),
}
}
#[track_caller]
pub fn set_mentioned_items(&mut self, mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>) {
assert!(
self.mentioned_items.is_none(),
"mentioned_items for {:?} have already been set",
self.source.def_id()
);
self.mentioned_items = Some(mentioned_items);
}
#[track_caller]
pub fn mentioned_items(&self) -> &[Spanned<MentionedItem<'tcx>>] {
match &self.mentioned_items {
Some(l) => l,
None => panic!("mentioned_items for {:?} have not yet been set", self.source.def_id()),
}
}
} }
impl<'tcx> Index<BasicBlock> for Body<'tcx> { impl<'tcx> Index<BasicBlock> for Body<'tcx> {

View File

@ -1066,12 +1066,14 @@ macro_rules! type_annotations {
$self.visit_span($(& $mutability)? $body.span); $self.visit_span($(& $mutability)? $body.span);
for const_ in &$($mutability)? $body.required_consts { if let Some(required_consts) = &$($mutability)? $body.required_consts {
for const_ in required_consts {
let location = Location::START; let location = Location::START;
$self.visit_const_operand(const_, location); $self.visit_const_operand(const_, location);
} }
} }
} }
}
macro_rules! visit_place_fns { macro_rules! visit_place_fns {
(mut) => { (mut) => {

View File

@ -54,8 +54,8 @@ pub(super) fn build_custom_mir<'tcx>(
spread_arg: None, spread_arg: None,
var_debug_info: Vec::new(), var_debug_info: Vec::new(),
span, span,
required_consts: Vec::new(), required_consts: None,
mentioned_items: Vec::new(), mentioned_items: None,
is_polymorphic: false, is_polymorphic: false,
tainted_by_errors: None, tainted_by_errors: None,
injection_phase: None, injection_phase: None,

View File

@ -744,8 +744,8 @@ fn dest_needs_borrow(place: Place<'_>) -> bool {
// Copy required constants from the callee_body into the caller_body. Although we are only // Copy required constants from the callee_body into the caller_body. Although we are only
// pushing unevaluated consts to `required_consts`, here they may have been evaluated // pushing unevaluated consts to `required_consts`, here they may have been evaluated
// because we are calling `instantiate_and_normalize_erasing_regions` -- so we filter again. // because we are calling `instantiate_and_normalize_erasing_regions` -- so we filter again.
caller_body.required_consts.extend( caller_body.required_consts.as_mut().unwrap().extend(
callee_body.required_consts.into_iter().filter(|ct| ct.const_.is_required_const()), callee_body.required_consts().into_iter().filter(|ct| ct.const_.is_required_const()),
); );
// Now that we incorporated the callee's `required_consts`, we can remove the callee from // 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 // `mentioned_items` -- but we have to take their `mentioned_items` in return. This does
@ -755,12 +755,11 @@ fn dest_needs_borrow(place: Place<'_>) -> bool {
// We need to reconstruct the `required_item` for the callee so that we can find and // We need to reconstruct the `required_item` for the callee so that we can find and
// remove it. // remove it.
let callee_item = MentionedItem::Fn(func.ty(caller_body, self.tcx)); let callee_item = MentionedItem::Fn(func.ty(caller_body, self.tcx));
if let Some(idx) = let caller_mentioned_items = caller_body.mentioned_items.as_mut().unwrap();
caller_body.mentioned_items.iter().position(|item| item.node == callee_item) if let Some(idx) = caller_mentioned_items.iter().position(|item| item.node == callee_item) {
{
// We found the callee, so remove it and add its items instead. // We found the callee, so remove it and add its items instead.
caller_body.mentioned_items.remove(idx); caller_mentioned_items.remove(idx);
caller_body.mentioned_items.extend(callee_body.mentioned_items); caller_mentioned_items.extend(callee_body.mentioned_items());
} else { } else {
// If we can't find the callee, there's no point in adding its items. Probably it // 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, so we already // already got removed by being inlined elsewhere in the same function, so we already

View File

@ -28,11 +28,10 @@
use rustc_hir::def_id::LocalDefId; use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::intravisit::{self, Visitor};
use rustc_index::IndexVec; use rustc_index::IndexVec;
use rustc_middle::mir::visit::Visitor as _;
use rustc_middle::mir::{ use rustc_middle::mir::{
traversal, AnalysisPhase, Body, CallSource, ClearCrossCrate, ConstOperand, ConstQualifs, AnalysisPhase, Body, CallSource, ClearCrossCrate, ConstOperand, ConstQualifs, LocalDecl,
LocalDecl, MirPass, MirPhase, Operand, Place, ProjectionElem, Promoted, RuntimePhase, Rvalue, MirPass, MirPhase, Operand, Place, ProjectionElem, Promoted, RuntimePhase, Rvalue, SourceInfo,
SourceInfo, Statement, StatementKind, TerminatorKind, START_BLOCK, Statement, StatementKind, TerminatorKind, START_BLOCK,
}; };
use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt}; use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
use rustc_middle::util::Providers; use rustc_middle::util::Providers;
@ -339,12 +338,15 @@ fn mir_promoted(
// Collect `required_consts` *before* promotion, so if there are any consts being promoted // Collect `required_consts` *before* promotion, so if there are any consts being promoted
// we still add them to the list in the outer MIR body. // we still add them to the list in the outer MIR body.
let mut required_consts = Vec::new(); RequiredConstsVisitor::compute_required_consts(&mut body);
let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts); // If this has an associated by-move async closure body, that doesn't get run through these
for (bb, bb_data) in traversal::reverse_postorder(&body) { // passes itself, it gets "tagged along" by the pass manager. `RequiredConstsVisitor` is not
required_consts_visitor.visit_basic_block_data(bb, bb_data); // a regular pass so we have to also apply it manually to the other body.
if let Some(coroutine) = body.coroutine.as_mut() {
if let Some(by_move_body) = coroutine.by_move_body.as_mut() {
RequiredConstsVisitor::compute_required_consts(by_move_body);
}
} }
body.required_consts = required_consts;
// What we need to run borrowck etc. // What we need to run borrowck etc.
let promote_pass = promote_consts::PromoteTemps::default(); let promote_pass = promote_consts::PromoteTemps::default();
@ -561,9 +563,6 @@ fn o1<T>(x: T) -> WithMinOptLevel<T> {
tcx, tcx,
body, 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. // Add some UB checks before any UB gets optimized away.
&check_alignment::CheckAlignment, &check_alignment::CheckAlignment,
// Before inlining: trim down MIR with passes to reduce inlining work. // Before inlining: trim down MIR with passes to reduce inlining work.
@ -657,6 +656,19 @@ fn inner_optimized_mir(tcx: TyCtxt<'_>, did: LocalDefId) -> Body<'_> {
return body; return body;
} }
// Before doing anything, remember which items are being mentioned so that the set of items
// visited does not depend on the optimization level.
// We do not use `run_passes` for this as that might skip the pass if `injection_phase` is set.
mentioned_items::MentionedItems.run_pass(tcx, &mut body);
// If this has an associated by-move async closure body, that doesn't get run through these
// passes itself, it gets "tagged along" by the pass manager. Since we're not using the pass
// manager we have to do this by hand.
if let Some(coroutine) = body.coroutine.as_mut() {
if let Some(by_move_body) = coroutine.by_move_body.as_mut() {
mentioned_items::MentionedItems.run_pass(tcx, by_move_body);
}
}
// If `mir_drops_elaborated_and_const_checked` found that the current body has unsatisfiable // If `mir_drops_elaborated_and_const_checked` found that the current body has unsatisfiable
// predicates, it will shrink the MIR to a single `unreachable` terminator. // predicates, it will shrink the MIR to a single `unreachable` terminator.
// More generally, if MIR is a lone `unreachable`, there is nothing to optimize. // More generally, if MIR is a lone `unreachable`, there is nothing to optimize.

View File

@ -23,10 +23,9 @@ fn is_enabled(&self, _sess: &Session) -> bool {
} }
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) { 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(); let mut mentioned_items = Vec::new();
MentionedItemsVisitor { tcx, body, mentioned_items: &mut mentioned_items }.visit_body(body); MentionedItemsVisitor { tcx, body, mentioned_items: &mut mentioned_items }.visit_body(body);
body.mentioned_items = mentioned_items; body.set_mentioned_items(mentioned_items);
} }
} }

View File

@ -702,6 +702,9 @@ struct Promoter<'a, 'tcx> {
temps: &'a mut IndexVec<Local, TempState>, temps: &'a mut IndexVec<Local, TempState>,
extra_statements: &'a mut Vec<(Location, Statement<'tcx>)>, extra_statements: &'a mut Vec<(Location, Statement<'tcx>)>,
/// Used to assemble the required_consts list while building the promoted.
required_consts: Vec<ConstOperand<'tcx>>,
/// If true, all nested temps are also kept in the /// If true, all nested temps are also kept in the
/// source MIR, not moved to the promoted MIR. /// source MIR, not moved to the promoted MIR.
keep_original: bool, keep_original: bool,
@ -924,11 +927,14 @@ fn promote_candidate(mut self, candidate: Candidate, next_promoted_id: usize) ->
let span = self.promoted.span; let span = self.promoted.span;
self.assign(RETURN_PLACE, rvalue, span); self.assign(RETURN_PLACE, rvalue, span);
// Now that we did promotion, we know whether we'll want to add this to `required_consts`. // Now that we did promotion, we know whether we'll want to add this to `required_consts` of
// the surrounding MIR body.
if self.add_to_required { if self.add_to_required {
self.source.required_consts.push(promoted_op); self.source.required_consts.as_mut().unwrap().push(promoted_op);
} }
self.promoted.set_required_consts(self.required_consts);
self.promoted self.promoted
} }
} }
@ -947,7 +953,7 @@ fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) {
fn visit_const_operand(&mut self, constant: &mut ConstOperand<'tcx>, _location: Location) { fn visit_const_operand(&mut self, constant: &mut ConstOperand<'tcx>, _location: Location) {
if constant.const_.is_required_const() { if constant.const_.is_required_const() {
self.promoted.required_consts.push(*constant); self.required_consts.push(*constant);
} }
// Skipping `super_constant` as the visitor is otherwise only looking for locals. // Skipping `super_constant` as the visitor is otherwise only looking for locals.
@ -1011,9 +1017,9 @@ fn promote_candidates<'tcx>(
extra_statements: &mut extra_statements, extra_statements: &mut extra_statements,
keep_original: false, keep_original: false,
add_to_required: false, add_to_required: false,
required_consts: Vec::new(),
}; };
// `required_consts` of the promoted itself gets filled while building the MIR body.
let mut promoted = promoter.promote_candidate(candidate, promotions.len()); let mut promoted = promoter.promote_candidate(candidate, promotions.len());
promoted.source.promoted = Some(promotions.next_index()); promoted.source.promoted = Some(promotions.next_index());
promotions.push(promoted); promotions.push(promoted);

View File

@ -1,14 +1,23 @@
use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{ConstOperand, Location}; use rustc_middle::mir::{traversal, Body, ConstOperand, Location};
pub struct RequiredConstsVisitor<'a, 'tcx> { pub struct RequiredConstsVisitor<'a, 'tcx> {
required_consts: &'a mut Vec<ConstOperand<'tcx>>, required_consts: &'a mut Vec<ConstOperand<'tcx>>,
} }
impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> { impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
pub fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> Self { fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> Self {
RequiredConstsVisitor { required_consts } RequiredConstsVisitor { required_consts }
} }
pub fn compute_required_consts(body: &mut Body<'tcx>) {
let mut required_consts = Vec::new();
let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
for (bb, bb_data) in traversal::reverse_postorder(&body) {
required_consts_visitor.visit_basic_block_data(bb, bb_data);
}
body.set_required_consts(required_consts);
}
} }
impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> { impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> {

View File

@ -305,7 +305,7 @@ fn new_body<'tcx>(
arg_count: usize, arg_count: usize,
span: Span, span: Span,
) -> Body<'tcx> { ) -> Body<'tcx> {
Body::new( let mut body = Body::new(
source, source,
basic_blocks, basic_blocks,
IndexVec::from_elem_n( IndexVec::from_elem_n(
@ -326,7 +326,10 @@ fn new_body<'tcx>(
None, None,
// FIXME(compiler-errors): is this correct? // FIXME(compiler-errors): is this correct?
None, None,
) );
// Shims do not directly mention any consts.
body.set_required_consts(Vec::new());
body
} }
pub struct DropShimElaborator<'a, 'tcx> { pub struct DropShimElaborator<'a, 'tcx> {
@ -969,13 +972,16 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> Body<'_> {
}; };
let source = MirSource::item(ctor_id); let source = MirSource::item(ctor_id);
let body = new_body( let mut body = new_body(
source, source,
IndexVec::from_elem_n(start_block, 1), IndexVec::from_elem_n(start_block, 1),
local_decls, local_decls,
sig.inputs().len(), sig.inputs().len(),
span, span,
); );
// A constructor doesn't mention any other items (and we don't run the usual optimization passes
// so this would otherwise not get filled).
body.set_mentioned_items(Vec::new());
crate::pass_manager::dump_mir_for_phase_change(tcx, &body); crate::pass_manager::dump_mir_for_phase_change(tcx, &body);

View File

@ -1229,7 +1229,7 @@ fn collect_items_of_instance<'tcx>(
// Always visit all `required_consts`, so that we evaluate them and abort compilation if any of // Always visit all `required_consts`, so that we evaluate them and abort compilation if any of
// them errors. // them errors.
for const_op in &body.required_consts { for const_op in body.required_consts() {
if let Some(val) = collector.eval_constant(const_op) { if let Some(val) = collector.eval_constant(const_op) {
collect_const_value(tcx, val, mentioned_items); collect_const_value(tcx, val, mentioned_items);
} }
@ -1237,7 +1237,7 @@ fn collect_items_of_instance<'tcx>(
// Always gather mentioned items. We try to avoid processing items that we have already added to // Always gather mentioned items. We try to avoid processing items that we have already added to
// `used_items` above. // `used_items` above.
for item in &body.mentioned_items { for item in body.mentioned_items() {
if !collector.used_mentioned_items.contains(&item.node) { if !collector.used_mentioned_items.contains(&item.node) {
let item_mono = collector.monomorphize(item.node); let item_mono = collector.monomorphize(item.node);
visit_mentioned_item(tcx, &item_mono, item.span, mentioned_items); visit_mentioned_item(tcx, &item_mono, item.span, mentioned_items);