Skip to content

Commit 82dc2eb

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #108322 - cjgillot:clean-const-prop, r=oli-obk
Clean ConstProp Small simplifications from the time when there that pass output lints.
2 parents c21b7f6 + f02d6c4 commit 82dc2eb

File tree

1 file changed

+105
-132
lines changed

1 file changed

+105
-132
lines changed

‎compiler/rustc_mir_transform/src/const_prop.rs

+105-132
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,7 @@ use rustc_index::vec::IndexVec;
1313
use rustc_middle::mir::visit::{
1414
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
1515
};
16-
use rustc_middle::mir::{
17-
BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, Location,
18-
Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind,
19-
RETURN_PLACE,
20-
};
16+
use rustc_middle::mir::*;
2117
use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
2218
use rustc_middle::ty::InternalSubsts;
2319
use rustc_middle::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeVisitable};
@@ -456,27 +452,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
456452
};
457453
}
458454

459-
fn use_ecx<F, T>(&mut self, f: F) -> Option<T>
460-
where
461-
F: FnOnce(&mut Self) -> InterpResult<'tcx, T>,
462-
{
463-
match f(self) {
464-
Ok(val) => Some(val),
465-
Err(error) => {
466-
trace!("InterpCx operation failed: {:?}", error);
467-
// Some errors shouldn't come up because creating them causes
468-
// an allocation, which we should avoid. When that happens,
469-
// dedicated error variants should be introduced instead.
470-
assert!(
471-
!error.kind().formatted_string(),
472-
"const-prop encountered formatting error: {}",
473-
error
474-
);
475-
None
476-
}
477-
}
478-
}
479-
480455
/// Returns the value, if any, of evaluating `c`.
481456
fn eval_constant(&mut self, c: &Constant<'tcx>) -> Option<OpTy<'tcx>> {
482457
// FIXME we need to revisit this for #67176
@@ -491,7 +466,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
491466
/// Returns the value, if any, of evaluating `place`.
492467
fn eval_place(&mut self, place: Place<'tcx>) -> Option<OpTy<'tcx>> {
493468
trace!("eval_place(place={:?})", place);
494-
self.use_ecx(|this| this.ecx.eval_place_to_op(place, None))
469+
self.ecx.eval_place_to_op(place, None).ok()
495470
}
496471

497472
/// Returns the value, if any, of evaluating `op`. Calls upon `eval_constant`
@@ -595,52 +570,54 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
595570
rvalue: &Rvalue<'tcx>,
596571
place: Place<'tcx>,
597572
) -> Option<()> {
598-
self.use_ecx(|this| match rvalue {
573+
match rvalue {
599574
Rvalue::BinaryOp(op, box (left, right))
600575
| Rvalue::CheckedBinaryOp(op, box (left, right)) => {
601-
let l = this.ecx.eval_operand(left, None).and_then(|x| this.ecx.read_immediate(&x));
576+
let l = self.ecx.eval_operand(left, None).and_then(|x| self.ecx.read_immediate(&x));
602577
let r =
603-
this.ecx.eval_operand(right, None).and_then(|x| this.ecx.read_immediate(&x));
578+
self.ecx.eval_operand(right, None).and_then(|x| self.ecx.read_immediate(&x));
604579

605580
let const_arg = match (l, r) {
606581
(Ok(x), Err(_)) | (Err(_), Ok(x)) => x, // exactly one side is known
607-
(Err(e), Err(_)) => return Err(e), // neither side is known
608-
(Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place), // both sides are known
582+
(Err(_), Err(_)) => return None, // neither side is known
583+
(Ok(_), Ok(_)) => return self.ecx.eval_rvalue_into_place(rvalue, place).ok(), // both sides are known
609584
};
610585

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

617-
let arg_value = const_arg.to_scalar().to_bits(const_arg.layout.size)?;
618-
let dest = this.ecx.eval_place(place)?;
592+
let arg_value = const_arg.to_scalar().to_bits(const_arg.layout.size).ok()?;
593+
let dest = self.ecx.eval_place(place).ok()?;
619594

620595
match op {
621-
BinOp::BitAnd if arg_value == 0 => this.ecx.write_immediate(*const_arg, &dest),
596+
BinOp::BitAnd if arg_value == 0 => {
597+
self.ecx.write_immediate(*const_arg, &dest).ok()
598+
}
622599
BinOp::BitOr
623600
if arg_value == const_arg.layout.size.truncate(u128::MAX)
624601
|| (const_arg.layout.ty.is_bool() && arg_value == 1) =>
625602
{
626-
this.ecx.write_immediate(*const_arg, &dest)
603+
self.ecx.write_immediate(*const_arg, &dest).ok()
627604
}
628605
BinOp::Mul if const_arg.layout.ty.is_integral() && arg_value == 0 => {
629606
if let Rvalue::CheckedBinaryOp(_, _) = rvalue {
630607
let val = Immediate::ScalarPair(
631608
const_arg.to_scalar(),
632609
Scalar::from_bool(false),
633610
);
634-
this.ecx.write_immediate(val, &dest)
611+
self.ecx.write_immediate(val, &dest).ok()
635612
} else {
636-
this.ecx.write_immediate(*const_arg, &dest)
613+
self.ecx.write_immediate(*const_arg, &dest).ok()
637614
}
638615
}
639-
_ => throw_machine_stop_str!("cannot optimize this"),
616+
_ => None,
640617
}
641618
}
642-
_ => this.ecx.eval_rvalue_into_place(rvalue, place),
643-
})
619+
_ => self.ecx.eval_rvalue_into_place(rvalue, place).ok(),
620+
}
644621
}
645622

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

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

687664
if let Some(Right(imm)) = imm {
688665
match *imm {
@@ -702,25 +679,23 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
702679
if let ty::Tuple(types) = ty.kind() {
703680
// Only do it if tuple is also a pair with two scalars
704681
if let [ty1, ty2] = types[..] {
705-
let alloc = self.use_ecx(|this| {
706-
let ty_is_scalar = |ty| {
707-
this.ecx.layout_of(ty).ok().map(|layout| layout.abi.is_scalar())
708-
== Some(true)
709-
};
710-
if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
711-
let alloc = this
712-
.ecx
713-
.intern_with_temp_alloc(value.layout, |ecx, dest| {
714-
ecx.write_immediate(*imm, dest)
715-
})
716-
.unwrap();
717-
Ok(Some(alloc))
718-
} else {
719-
Ok(None)
720-
}
721-
});
722-
723-
if let Some(Some(alloc)) = alloc {
682+
let ty_is_scalar = |ty| {
683+
self.ecx.layout_of(ty).ok().map(|layout| layout.abi.is_scalar())
684+
== Some(true)
685+
};
686+
let alloc = if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
687+
let alloc = self
688+
.ecx
689+
.intern_with_temp_alloc(value.layout, |ecx, dest| {
690+
ecx.write_immediate(*imm, dest)
691+
})
692+
.unwrap();
693+
Some(alloc)
694+
} else {
695+
None
696+
};
697+
698+
if let Some(alloc) = alloc {
724699
// Assign entire constant in a single statement.
725700
// We can't use aggregates, as we run after the aggregate-lowering `MirPhase`.
726701
let const_val = ConstValue::ByRef { alloc, offset: Size::ZERO };
@@ -921,84 +896,80 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
921896
trace!("visit_statement: {:?}", statement);
922897
let source_info = statement.source_info;
923898
self.source_info = Some(source_info);
924-
if let StatementKind::Assign(box (place, ref mut rval)) = statement.kind {
925-
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
926-
if let Some(()) = self.const_prop(rval, place) {
927-
// This will return None if the above `const_prop` invocation only "wrote" a
928-
// type whose creation requires no write. E.g. a generator whose initial state
929-
// consists solely of uninitialized memory (so it doesn't capture any locals).
930-
if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) {
931-
trace!("replacing {:?} with {:?}", rval, value);
932-
self.replace_with_const(rval, value, source_info);
933-
if can_const_prop == ConstPropMode::FullConstProp
934-
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
935-
{
936-
trace!("propagated into {:?}", place);
899+
match statement.kind {
900+
StatementKind::Assign(box (place, ref mut rval)) => {
901+
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
902+
if let Some(()) = self.const_prop(rval, place) {
903+
// This will return None if the above `const_prop` invocation only "wrote" a
904+
// type whose creation requires no write. E.g. a generator whose initial state
905+
// consists solely of uninitialized memory (so it doesn't capture any locals).
906+
if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) {
907+
trace!("replacing {:?} with {:?}", rval, value);
908+
self.replace_with_const(rval, value, source_info);
909+
if can_const_prop == ConstPropMode::FullConstProp
910+
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
911+
{
912+
trace!("propagated into {:?}", place);
913+
}
937914
}
938-
}
939-
match can_const_prop {
940-
ConstPropMode::OnlyInsideOwnBlock => {
941-
trace!(
942-
"found local restricted to its block. \
915+
match can_const_prop {
916+
ConstPropMode::OnlyInsideOwnBlock => {
917+
trace!(
918+
"found local restricted to its block. \
943919
Will remove it from const-prop after block is finished. Local: {:?}",
944-
place.local
945-
);
946-
}
947-
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
948-
trace!("can't propagate into {:?}", place);
949-
if place.local != RETURN_PLACE {
950-
Self::remove_const(&mut self.ecx, place.local);
920+
place.local
921+
);
951922
}
952-
}
953-
ConstPropMode::FullConstProp => {}
954-
}
955-
} else {
956-
// Const prop failed, so erase the destination, ensuring that whatever happens
957-
// from here on, does not know about the previous value.
958-
// This is important in case we have
959-
// ```rust
960-
// let mut x = 42;
961-
// x = SOME_MUTABLE_STATIC;
962-
// // x must now be uninit
963-
// ```
964-
// FIXME: we overzealously erase the entire local, because that's easier to
965-
// implement.
966-
trace!(
967-
"propagation into {:?} failed.
968-
Nuking the entire site from orbit, it's the only way to be sure",
969-
place,
970-
);
971-
Self::remove_const(&mut self.ecx, place.local);
972-
}
973-
} else {
974-
match statement.kind {
975-
StatementKind::SetDiscriminant { ref place, .. } => {
976-
match self.ecx.machine.can_const_prop[place.local] {
977-
ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
978-
if self.use_ecx(|this| this.ecx.statement(statement)).is_some() {
979-
trace!("propped discriminant into {:?}", place);
980-
} else {
923+
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
924+
trace!("can't propagate into {:?}", place);
925+
if place.local != RETURN_PLACE {
981926
Self::remove_const(&mut self.ecx, place.local);
982927
}
983928
}
984-
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
985-
Self::remove_const(&mut self.ecx, place.local);
986-
}
929+
ConstPropMode::FullConstProp => {}
987930
}
931+
} else {
932+
// Const prop failed, so erase the destination, ensuring that whatever happens
933+
// from here on, does not know about the previous value.
934+
// This is important in case we have
935+
// ```rust
936+
// let mut x = 42;
937+
// x = SOME_MUTABLE_STATIC;
938+
// // x must now be uninit
939+
// ```
940+
// FIXME: we overzealously erase the entire local, because that's easier to
941+
// implement.
942+
trace!(
943+
"propagation into {:?} failed.
944+
Nuking the entire site from orbit, it's the only way to be sure",
945+
place,
946+
);
947+
Self::remove_const(&mut self.ecx, place.local);
988948
}
989-
StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => {
990-
let frame = self.ecx.frame_mut();
991-
frame.locals[local].value =
992-
if let StatementKind::StorageLive(_) = statement.kind {
993-
LocalValue::Live(interpret::Operand::Immediate(
994-
interpret::Immediate::Uninit,
995-
))
949+
}
950+
StatementKind::SetDiscriminant { ref place, .. } => {
951+
match self.ecx.machine.can_const_prop[place.local] {
952+
ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
953+
if self.ecx.statement(statement).is_ok() {
954+
trace!("propped discriminant into {:?}", place);
996955
} else {
997-
LocalValue::Dead
998-
};
956+
Self::remove_const(&mut self.ecx, place.local);
957+
}
958+
}
959+
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
960+
Self::remove_const(&mut self.ecx, place.local);
961+
}
999962
}
1000-
_ => {}
1001963
}
964+
StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => {
965+
let frame = self.ecx.frame_mut();
966+
frame.locals[local].value = if let StatementKind::StorageLive(_) = statement.kind {
967+
LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit))
968+
} else {
969+
LocalValue::Dead
970+
};
971+
}
972+
_ => {}
1002973
}
1003974

1004975
self.super_statement(statement, location);
@@ -1008,12 +979,10 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
1008979
let source_info = terminator.source_info;
1009980
self.source_info = Some(source_info);
1010981
self.super_terminator(terminator, location);
1011-
// Do NOT early return in this function, it does some crucial fixup of the state at the end!
982+
1012983
match &mut terminator.kind {
1013984
TerminatorKind::Assert { expected, ref mut cond, .. } => {
1014985
if let Some(ref value) = self.eval_operand(&cond)
1015-
// FIXME should be used use_ecx rather than a local match... but we have
1016-
// quite a few of these read_scalar/read_immediate that need fixing.
1017986
&& let Ok(value_const) = self.ecx.read_scalar(&value)
1018987
&& self.should_const_prop(value)
1019988
{
@@ -1050,6 +1019,10 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
10501019
// gated on `mir_opt_level=3`.
10511020
TerminatorKind::Call { .. } => {}
10521021
}
1022+
}
1023+
1024+
fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) {
1025+
self.super_basic_block_data(block, data);
10531026

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

0 commit comments

Comments
 (0)