Skip to content

Commit

Permalink
Rollup merge of #121260 - gurry:constprop-lint-cleanup, r=oli-obk
Browse files Browse the repository at this point in the history
Remove const_prop.rs

Removed const_prop.rs and moved its contents to const_prop_lint.rs and dataflow_const_prop.rs where they are used.

const_prop.rs does not actually do any const propagation any more. After #116012 all it contains is code that is used by const_prop_lint.rs and one macro that is used by dataflow_const_prop.rs. So it made sense to just move it to those two places and remove this file.
  • Loading branch information
matthiaskrgr authored Feb 18, 2024
2 parents 717d602 + 50e316b commit d35c4f9
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 168 deletions.
161 changes: 0 additions & 161 deletions compiler/rustc_mir_transform/src/const_prop.rs

This file was deleted.

132 changes: 127 additions & 5 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,14 @@ use rustc_const_eval::interpret::{
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::DefKind;
use rustc_hir::HirId;
use rustc_index::bit_set::BitSet;
use rustc_index::{Idx, IndexVec};
use rustc_middle::mir::visit::Visitor;
use rustc_index::{bit_set::BitSet, Idx, IndexVec};
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
use rustc_middle::ty::{self, ConstInt, ParamEnv, ScalarInt, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::Span;
use rustc_target::abi::{Abi, FieldIdx, HasDataLayout, Size, TargetDataLayout, VariantIdx};

use crate::const_prop::CanConstProp;
use crate::const_prop::ConstPropMode;
use crate::dataflow_const_prop::DummyMachine;
use crate::errors::{AssertLint, AssertLintKind};
use crate::MirLint;
Expand Down Expand Up @@ -849,3 +846,128 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
}
}
}

/// The maximum number of bytes that we'll allocate space for a local or the return value.
/// Needed for #66397, because otherwise we eval into large places and that can cause OOM or just
/// Severely regress performance.
const MAX_ALLOC_LIMIT: u64 = 1024;

/// The mode that `ConstProp` is allowed to run in for a given `Local`.
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum ConstPropMode {
/// The `Local` can be propagated into and reads of this `Local` can also be propagated.
FullConstProp,
/// The `Local` can only be propagated into and from its own block.
OnlyInsideOwnBlock,
/// The `Local` cannot be part of propagation at all. Any statement
/// referencing it either for reading or writing will not get propagated.
NoPropagation,
}

pub struct CanConstProp {
can_const_prop: IndexVec<Local, ConstPropMode>,
// False at the beginning. Once set, no more assignments are allowed to that local.
found_assignment: BitSet<Local>,
}

impl CanConstProp {
/// Returns true if `local` can be propagated
pub fn check<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
body: &Body<'tcx>,
) -> IndexVec<Local, ConstPropMode> {
let mut cpv = CanConstProp {
can_const_prop: IndexVec::from_elem(ConstPropMode::FullConstProp, &body.local_decls),
found_assignment: BitSet::new_empty(body.local_decls.len()),
};
for (local, val) in cpv.can_const_prop.iter_enumerated_mut() {
let ty = body.local_decls[local].ty;
match tcx.layout_of(param_env.and(ty)) {
Ok(layout) if layout.size < Size::from_bytes(MAX_ALLOC_LIMIT) => {}
// Either the layout fails to compute, then we can't use this local anyway
// or the local is too large, then we don't want to.
_ => {
*val = ConstPropMode::NoPropagation;
continue;
}
}
}
// Consider that arguments are assigned on entry.
for arg in body.args_iter() {
cpv.found_assignment.insert(arg);
}
cpv.visit_body(body);
cpv.can_const_prop
}
}

impl<'tcx> Visitor<'tcx> for CanConstProp {
fn visit_place(&mut self, place: &Place<'tcx>, mut context: PlaceContext, loc: Location) {
use rustc_middle::mir::visit::PlaceContext::*;

// Dereferencing just read the addess of `place.local`.
if place.projection.first() == Some(&PlaceElem::Deref) {
context = NonMutatingUse(NonMutatingUseContext::Copy);
}

self.visit_local(place.local, context, loc);
self.visit_projection(place.as_ref(), context, loc);
}

fn visit_local(&mut self, local: Local, context: PlaceContext, _: Location) {
use rustc_middle::mir::visit::PlaceContext::*;
match context {
// These are just stores, where the storing is not propagatable, but there may be later
// mutations of the same local via `Store`
| MutatingUse(MutatingUseContext::Call)
| MutatingUse(MutatingUseContext::AsmOutput)
| MutatingUse(MutatingUseContext::Deinit)
// Actual store that can possibly even propagate a value
| MutatingUse(MutatingUseContext::Store)
| MutatingUse(MutatingUseContext::SetDiscriminant) => {
if !self.found_assignment.insert(local) {
match &mut self.can_const_prop[local] {
// If the local can only get propagated in its own block, then we don't have
// to worry about multiple assignments, as we'll nuke the const state at the
// end of the block anyway, and inside the block we overwrite previous
// states as applicable.
ConstPropMode::OnlyInsideOwnBlock => {}
ConstPropMode::NoPropagation => {}
other @ ConstPropMode::FullConstProp => {
trace!(
"local {:?} can't be propagated because of multiple assignments. Previous state: {:?}",
local, other,
);
*other = ConstPropMode::OnlyInsideOwnBlock;
}
}
}
}
// Reading constants is allowed an arbitrary number of times
NonMutatingUse(NonMutatingUseContext::Copy)
| NonMutatingUse(NonMutatingUseContext::Move)
| NonMutatingUse(NonMutatingUseContext::Inspect)
| NonMutatingUse(NonMutatingUseContext::PlaceMention)
| NonUse(_) => {}

// These could be propagated with a smarter analysis or just some careful thinking about
// whether they'd be fine right now.
MutatingUse(MutatingUseContext::Yield)
| MutatingUse(MutatingUseContext::Drop)
| MutatingUse(MutatingUseContext::Retag)
// These can't ever be propagated under any scheme, as we can't reason about indirect
// mutation.
| NonMutatingUse(NonMutatingUseContext::SharedBorrow)
| NonMutatingUse(NonMutatingUseContext::FakeBorrow)
| NonMutatingUse(NonMutatingUseContext::AddressOf)
| MutatingUse(MutatingUseContext::Borrow)
| MutatingUse(MutatingUseContext::AddressOf) => {
trace!("local {:?} can't be propagated because it's used: {:?}", local, context);
self.can_const_prop[local] = ConstPropMode::NoPropagation;
}
MutatingUse(MutatingUseContext::Projection)
| NonMutatingUse(NonMutatingUseContext::Projection) => bug!("visit_place should not pass {context:?} for {local:?}"),
}
}
}
27 changes: 26 additions & 1 deletion compiler/rustc_mir_transform/src/dataflow_const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,32 @@ use rustc_span::def_id::DefId;
use rustc_span::DUMMY_SP;
use rustc_target::abi::{Abi, FieldIdx, Size, VariantIdx, FIRST_VARIANT};

use crate::const_prop::throw_machine_stop_str;
/// Macro for machine-specific `InterpError` without allocation.
/// (These will never be shown to the user, but they help diagnose ICEs.)
pub(crate) macro throw_machine_stop_str($($tt:tt)*) {{
// We make a new local type for it. The type itself does not carry any information,
// but its vtable (for the `MachineStopType` trait) does.
#[derive(Debug)]
struct Zst;
// Printing this type shows the desired string.
impl std::fmt::Display for Zst {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, $($tt)*)
}
}

impl rustc_middle::mir::interpret::MachineStopType for Zst {
fn diagnostic_message(&self) -> rustc_errors::DiagnosticMessage {
self.to_string().into()
}

fn add_args(
self: Box<Self>,
_: &mut dyn FnMut(rustc_errors::DiagnosticArgName, rustc_errors::DiagnosticArgValue),
) {}
}
throw_machine_stop!(Zst)
}}

// These constants are somewhat random guesses and have not been optimized.
// If `tcx.sess.mir_opt_level() >= 4`, we ignore the limits (this can become very expensive).
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ mod remove_place_mention;
mod add_subtyping_projections;
pub mod cleanup_post_borrowck;
mod const_debuginfo;
mod const_prop;
mod const_prop_lint;
mod copy_prop;
mod coroutine;
Expand Down

0 comments on commit d35c4f9

Please sign in to comment.