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.

Sign up for GitHub

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

Clean ConstProp #108322

Merged
merged 3 commits into from
Feb 22, 2023
Merged

Clean ConstProp #108322

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a75995
Move state fixup into a different method.
cjgillot Feb 8, 2023
d0934f1
Merge if-let and match.
cjgillot Feb 8, 2023
f02d6c4
Remove use_ecx.
cjgillot Feb 10, 2023
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
237 changes: 105 additions & 132 deletions compiler/rustc_mir_transform/src/const_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ use rustc_index::vec::IndexVec;
use rustc_middle::mir::visit::{
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
};
use rustc_middle::mir::{
BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, Location,
Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind,
RETURN_PLACE,
};
use rustc_middle::mir::*;
use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
use rustc_middle::ty::InternalSubsts;
use rustc_middle::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeVisitable};
Expand Down Expand Up @@ -456,27 +452,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
};
}

fn use_ecx<F, T>(&mut self, f: F) -> Option<T>
where
F: FnOnce(&mut Self) -> InterpResult<'tcx, T>,
{
match f(self) {
Ok(val) => Some(val),
Err(error) => {
trace!("InterpCx operation failed: {:?}", error);
// Some errors shouldn't come up because creating them causes
// an allocation, which we should avoid. When that happens,
// dedicated error variants should be introduced instead.
assert!(
!error.kind().formatted_string(),
"const-prop encountered formatting error: {}",
error
);
None
}
}
}

/// Returns the value, if any, of evaluating `c`.
fn eval_constant(&mut self, c: &Constant<'tcx>) -> Option<OpTy<'tcx>> {
// FIXME we need to revisit this for #67176
Expand All @@ -491,7 +466,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
/// Returns the value, if any, of evaluating `place`.
fn eval_place(&mut self, place: Place<'tcx>) -> Option<OpTy<'tcx>> {
trace!("eval_place(place={:?})", place);
self.use_ecx(|this| this.ecx.eval_place_to_op(place, None))
self.ecx.eval_place_to_op(place, None).ok()
}

/// Returns the value, if any, of evaluating `op`. Calls upon `eval_constant`
Expand Down Expand Up @@ -595,52 +570,54 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
rvalue: &Rvalue<'tcx>,
place: Place<'tcx>,
) -> Option<()> {
self.use_ecx(|this| match rvalue {
match rvalue {
Rvalue::BinaryOp(op, box (left, right))
| Rvalue::CheckedBinaryOp(op, box (left, right)) => {
let l = this.ecx.eval_operand(left, None).and_then(|x| this.ecx.read_immediate(&x));
let l = self.ecx.eval_operand(left, None).and_then(|x| self.ecx.read_immediate(&x));
let r =
this.ecx.eval_operand(right, None).and_then(|x| this.ecx.read_immediate(&x));
self.ecx.eval_operand(right, None).and_then(|x| self.ecx.read_immediate(&x));

let const_arg = match (l, r) {
(Ok(x), Err(_)) | (Err(_), Ok(x)) => x, // exactly one side is known
(Err(e), Err(_)) => return Err(e), // neither side is known
(Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place), // both sides are known
(Err(_), Err(_)) => return None, // neither side is known
(Ok(_), Ok(_)) => return self.ecx.eval_rvalue_into_place(rvalue, place).ok(), // both sides are known
};

if !matches!(const_arg.layout.abi, abi::Abi::Scalar(..)) {
// We cannot handle Scalar Pair stuff.
// No point in calling `eval_rvalue_into_place`, since only one side is known
throw_machine_stop_str!("cannot optimize this")
return None;
}

let arg_value = const_arg.to_scalar().to_bits(const_arg.layout.size)?;
let dest = this.ecx.eval_place(place)?;
let arg_value = const_arg.to_scalar().to_bits(const_arg.layout.size).ok()?;
let dest = self.ecx.eval_place(place).ok()?;

match op {
BinOp::BitAnd if arg_value == 0 => this.ecx.write_immediate(*const_arg, &dest),
BinOp::BitAnd if arg_value == 0 => {
self.ecx.write_immediate(*const_arg, &dest).ok()
}
BinOp::BitOr
if arg_value == const_arg.layout.size.truncate(u128::MAX)
|| (const_arg.layout.ty.is_bool() && arg_value == 1) =>
{
this.ecx.write_immediate(*const_arg, &dest)
self.ecx.write_immediate(*const_arg, &dest).ok()
}
BinOp::Mul if const_arg.layout.ty.is_integral() && arg_value == 0 => {
if let Rvalue::CheckedBinaryOp(_, _) = rvalue {
let val = Immediate::ScalarPair(
const_arg.to_scalar(),
Scalar::from_bool(false),
);
this.ecx.write_immediate(val, &dest)
self.ecx.write_immediate(val, &dest).ok()
} else {
this.ecx.write_immediate(*const_arg, &dest)
self.ecx.write_immediate(*const_arg, &dest).ok()
}
}
_ => throw_machine_stop_str!("cannot optimize this"),
_ => None,
}
}
_ => this.ecx.eval_rvalue_into_place(rvalue, place),
})
_ => self.ecx.eval_rvalue_into_place(rvalue, place).ok(),
}
}

/// Creates a new `Operand::Constant` from a `Scalar` value
Expand Down Expand Up @@ -682,7 +659,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

// FIXME> figure out what to do when read_immediate_raw fails
let imm = self.use_ecx(|this| this.ecx.read_immediate_raw(value));
let imm = self.ecx.read_immediate_raw(value).ok();

if let Some(Right(imm)) = imm {
match *imm {
Expand All @@ -702,25 +679,23 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
if let ty::Tuple(types) = ty.kind() {
// Only do it if tuple is also a pair with two scalars
if let [ty1, ty2] = types[..] {
let alloc = self.use_ecx(|this| {
let ty_is_scalar = |ty| {
this.ecx.layout_of(ty).ok().map(|layout| layout.abi.is_scalar())
== Some(true)
};
if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
let alloc = this
.ecx
.intern_with_temp_alloc(value.layout, |ecx, dest| {
ecx.write_immediate(*imm, dest)
})
.unwrap();
Ok(Some(alloc))
} else {
Ok(None)
}
});

if let Some(Some(alloc)) = alloc {
let ty_is_scalar = |ty| {
self.ecx.layout_of(ty).ok().map(|layout| layout.abi.is_scalar())
== Some(true)
};
let alloc = if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
let alloc = self
.ecx
.intern_with_temp_alloc(value.layout, |ecx, dest| {
ecx.write_immediate(*imm, dest)
})
.unwrap();
Some(alloc)
} else {
None
};

if let Some(alloc) = alloc {
// Assign entire constant in a single statement.
// We can't use aggregates, as we run after the aggregate-lowering `MirPhase`.
let const_val = ConstValue::ByRef { alloc, offset: Size::ZERO };
Expand Down Expand Up @@ -921,84 +896,80 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
trace!("visit_statement: {:?}", statement);
let source_info = statement.source_info;
self.source_info = Some(source_info);
if let StatementKind::Assign(box (place, ref mut rval)) = statement.kind {
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
if let Some(()) = self.const_prop(rval, place) {
// This will return None if the above `const_prop` invocation only "wrote" a
// type whose creation requires no write. E.g. a generator whose initial state
// consists solely of uninitialized memory (so it doesn't capture any locals).
if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) {
trace!("replacing {:?} with {:?}", rval, value);
self.replace_with_const(rval, value, source_info);
if can_const_prop == ConstPropMode::FullConstProp
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
{
trace!("propagated into {:?}", place);
match statement.kind {
StatementKind::Assign(box (place, ref mut rval)) => {
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
if let Some(()) = self.const_prop(rval, place) {
// This will return None if the above `const_prop` invocation only "wrote" a
// type whose creation requires no write. E.g. a generator whose initial state
// consists solely of uninitialized memory (so it doesn't capture any locals).
if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) {
trace!("replacing {:?} with {:?}", rval, value);
self.replace_with_const(rval, value, source_info);
if can_const_prop == ConstPropMode::FullConstProp
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
{
trace!("propagated into {:?}", place);
}
}
}
match can_const_prop {
ConstPropMode::OnlyInsideOwnBlock => {
trace!(
"found local restricted to its block. \
match can_const_prop {
ConstPropMode::OnlyInsideOwnBlock => {
trace!(
"found local restricted to its block. \
Will remove it from const-prop after block is finished. Local: {:?}",
place.local
);
}
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
trace!("can't propagate into {:?}", place);
if place.local != RETURN_PLACE {
Self::remove_const(&mut self.ecx, place.local);
place.local
);
}
}
ConstPropMode::FullConstProp => {}
}
} else {
// Const prop failed, so erase the destination, ensuring that whatever happens
// from here on, does not know about the previous value.
// This is important in case we have
// ```rust
// let mut x = 42;
// x = SOME_MUTABLE_STATIC;
// // x must now be uninit
// ```
// FIXME: we overzealously erase the entire local, because that's easier to
// implement.
trace!(
"propagation into {:?} failed.
Nuking the entire site from orbit, it's the only way to be sure",
place,
);
Self::remove_const(&mut self.ecx, place.local);
}
} else {
match statement.kind {
StatementKind::SetDiscriminant { ref place, .. } => {
match self.ecx.machine.can_const_prop[place.local] {
ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
if self.use_ecx(|this| this.ecx.statement(statement)).is_some() {
trace!("propped discriminant into {:?}", place);
} else {
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
trace!("can't propagate into {:?}", place);
if place.local != RETURN_PLACE {
Self::remove_const(&mut self.ecx, place.local);
}
}
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
Self::remove_const(&mut self.ecx, place.local);
}
ConstPropMode::FullConstProp => {}
}
} else {
// Const prop failed, so erase the destination, ensuring that whatever happens
// from here on, does not know about the previous value.
// This is important in case we have
// ```rust
// let mut x = 42;
// x = SOME_MUTABLE_STATIC;
// // x must now be uninit
// ```
// FIXME: we overzealously erase the entire local, because that's easier to
// implement.
trace!(
"propagation into {:?} failed.
Nuking the entire site from orbit, it's the only way to be sure",
place,
);
Self::remove_const(&mut self.ecx, place.local);
}
StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => {
let frame = self.ecx.frame_mut();
frame.locals[local].value =
if let StatementKind::StorageLive(_) = statement.kind {
LocalValue::Live(interpret::Operand::Immediate(
interpret::Immediate::Uninit,
))
}
StatementKind::SetDiscriminant { ref place, .. } => {
match self.ecx.machine.can_const_prop[place.local] {
ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
if self.ecx.statement(statement).is_ok() {
trace!("propped discriminant into {:?}", place);
} else {
LocalValue::Dead
};
Self::remove_const(&mut self.ecx, place.local);
}
}
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
Self::remove_const(&mut self.ecx, place.local);
}
}
_ => {}
}
StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => {
let frame = self.ecx.frame_mut();
frame.locals[local].value = if let StatementKind::StorageLive(_) = statement.kind {
LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit))
} else {
LocalValue::Dead
};
}
_ => {}
}

self.super_statement(statement, location);
Expand All @@ -1008,12 +979,10 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
let source_info = terminator.source_info;
self.source_info = Some(source_info);
self.super_terminator(terminator, location);
// Do NOT early return in this function, it does some crucial fixup of the state at the end!

match &mut terminator.kind {
TerminatorKind::Assert { expected, ref mut cond, .. } => {
if let Some(ref value) = self.eval_operand(&cond)
// FIXME should be used use_ecx rather than a local match... but we have
// quite a few of these read_scalar/read_immediate that need fixing.
&& let Ok(value_const) = self.ecx.read_scalar(&value)
&& self.should_const_prop(value)
{
Expand Down Expand Up @@ -1050,6 +1019,10 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
// gated on `mir_opt_level=3`.
TerminatorKind::Call { .. } => {}
}
}

fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) {
self.super_basic_block_data(block, data);

// We remove all Locals which are restricted in propagation to their containing blocks and
// which were modified in the current block.
Expand Down