Skip to content

Commit b55c4f8

Browse files
cjgillotcjgillot
committed
Separate checking rvalue from evaluation.
1 parent f00be8b commit b55c4f8

File tree

2 files changed

+104
-113
lines changed

2 files changed

+104
-113
lines changed

‎compiler/rustc_mir_transform/src/const_prop.rs‎

Lines changed: 57 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
470470
}
471471
}
472472

473-
fn const_prop(&mut self, rvalue: &Rvalue<'tcx>, place: Place<'tcx>) -> Option<()> {
473+
fn check_rvalue(&mut self, rvalue: &Rvalue<'tcx>) -> Option<()> {
474474
// Perform any special handling for specific Rvalue types.
475475
// Generally, checks here fall into one of two categories:
476476
// 1. Additional checking to provide useful lints to the user
@@ -527,7 +527,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
527527
return None;
528528
}
529529

530-
self.eval_rvalue_with_identities(rvalue, place)
530+
Some(())
531531
}
532532

533533
// Attempt to use algebraic identities to eliminate constant expressions
@@ -595,7 +595,16 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
595595
}))
596596
}
597597

598-
fn replace_with_const(&mut self, rval: &mut Rvalue<'tcx>, value: &OpTy<'tcx>) {
598+
fn replace_with_const(&mut self, place: Place<'tcx>, rval: &mut Rvalue<'tcx>) {
599+
// This will return None if the above `const_prop` invocation only "wrote" a
600+
// type whose creation requires no write. E.g. a generator whose initial state
601+
// consists solely of uninitialized memory (so it doesn't capture any locals).
602+
let Some(ref value) = self.get_const(place) else { return };
603+
if !self.should_const_prop(value) {
604+
return;
605+
}
606+
trace!("replacing {:?}={:?} with {:?}", place, rval, value);
607+
599608
if let Rvalue::Use(Operand::Constant(c)) = rval {
600609
match c.literal {
601610
ConstantKind::Ty(c) if matches!(c.kind(), ConstKind::Unevaluated(..)) => {}
@@ -688,6 +697,19 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
688697
_ => false,
689698
}
690699
}
700+
701+
fn ensure_not_propagated(&mut self, local: Local) {
702+
if cfg!(debug_assertions) {
703+
assert!(
704+
self.get_const(local.into()).is_none()
705+
|| self
706+
.layout_of(self.local_decls[local].ty)
707+
.map_or(true, |layout| layout.is_zst()),
708+
"failed to remove values for `{local:?}`, value={:?}",
709+
self.get_const(local.into()),
710+
)
711+
}
712+
}
691713
}
692714

693715
/// The mode that `ConstProp` is allowed to run in for a given `Local`.
@@ -827,44 +849,23 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
827849
self.eval_constant(constant);
828850
}
829851

830-
fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) {
831-
trace!("visit_statement: {:?}", statement);
832-
833-
// Recurse into statement before applying the assignment.
834-
self.super_statement(statement, location);
835-
836-
match statement.kind {
837-
StatementKind::Assign(box (place, ref mut rval)) => {
838-
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
839-
if let Some(()) = self.const_prop(rval, place) {
840-
// This will return None if the above `const_prop` invocation only "wrote" a
841-
// type whose creation requires no write. E.g. a generator whose initial state
842-
// consists solely of uninitialized memory (so it doesn't capture any locals).
843-
if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) {
844-
trace!("replacing {:?} with {:?}", rval, value);
845-
self.replace_with_const(rval, value);
846-
if can_const_prop == ConstPropMode::FullConstProp
847-
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
848-
{
849-
trace!("propagated into {:?}", place);
850-
}
851-
}
852-
match can_const_prop {
853-
ConstPropMode::OnlyInsideOwnBlock => {
854-
trace!(
855-
"found local restricted to its block. \
856-
Will remove it from const-prop after block is finished. Local: {:?}",
857-
place.local
858-
);
859-
}
860-
ConstPropMode::NoPropagation => {
861-
trace!("can't propagate into {:?}", place);
862-
if place.local != RETURN_PLACE {
863-
Self::remove_const(&mut self.ecx, place.local);
864-
}
865-
}
866-
ConstPropMode::FullConstProp => {}
867-
}
852+
fn visit_assign(
853+
&mut self,
854+
place: &mut Place<'tcx>,
855+
rvalue: &mut Rvalue<'tcx>,
856+
location: Location,
857+
) {
858+
self.super_assign(place, rvalue, location);
859+
860+
let Some(()) = self.check_rvalue(rvalue) else { return };
861+
862+
match self.ecx.machine.can_const_prop[place.local] {
863+
// Do nothing if the place is indirect.
864+
_ if place.is_indirect() => {}
865+
ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local),
866+
ConstPropMode::OnlyInsideOwnBlock | ConstPropMode::FullConstProp => {
867+
if let Some(()) = self.eval_rvalue_with_identities(rvalue, *place) {
868+
self.replace_with_const(*place, rvalue);
868869
} else {
869870
// Const prop failed, so erase the destination, ensuring that whatever happens
870871
// from here on, does not know about the previous value.
@@ -884,18 +885,28 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
884885
Self::remove_const(&mut self.ecx, place.local);
885886
}
886887
}
888+
}
889+
}
890+
891+
fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) {
892+
trace!("visit_statement: {:?}", statement);
893+
894+
// Recurse into statement before applying the assignment.
895+
self.super_statement(statement, location);
896+
897+
match statement.kind {
887898
StatementKind::SetDiscriminant { ref place, .. } => {
888899
match self.ecx.machine.can_const_prop[place.local] {
900+
// Do nothing if the place is indirect.
901+
_ if place.is_indirect() => {}
902+
ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local),
889903
ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
890904
if self.ecx.statement(statement).is_ok() {
891905
trace!("propped discriminant into {:?}", place);
892906
} else {
893907
Self::remove_const(&mut self.ecx, place.local);
894908
}
895909
}
896-
ConstPropMode::NoPropagation => {
897-
Self::remove_const(&mut self.ecx, place.local);
898-
}
899910
}
900911
}
901912
StatementKind::StorageLive(local) => {
@@ -958,30 +969,17 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
958969
fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) {
959970
self.super_basic_block_data(block, data);
960971

961-
let ensure_not_propagated = |this: &mut Self, local: Local| {
962-
if cfg!(debug_assertions) {
963-
assert!(
964-
this.get_const(local.into()).is_none()
965-
|| this
966-
.layout_of(this.local_decls[local].ty)
967-
.map_or(true, |layout| layout.is_zst()),
968-
"failed to remove values for `{local:?}`, value={:?}",
969-
this.get_const(local.into()),
970-
)
971-
}
972-
};
973-
974972
// We remove all Locals which are restricted in propagation to their containing blocks and
975973
// which were modified in the current block.
976974
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`.
977975
let can_const_prop = std::mem::take(&mut self.ecx.machine.can_const_prop);
978976
for (local, &mode) in can_const_prop.iter_enumerated() {
979977
match mode {
980978
ConstPropMode::FullConstProp => {}
981-
ConstPropMode::NoPropagation => ensure_not_propagated(self, local),
979+
ConstPropMode::NoPropagation => self.ensure_not_propagated(local),
982980
ConstPropMode::OnlyInsideOwnBlock => {
983981
Self::remove_const(&mut self.ecx, local);
984-
ensure_not_propagated(self, local);
982+
self.ensure_not_propagated(local);
985983
}
986984
}
987985
}

‎compiler/rustc_mir_transform/src/const_prop_lint.rs‎

Lines changed: 47 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -405,12 +405,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
405405
Some(())
406406
}
407407

408-
fn const_prop(
409-
&mut self,
410-
rvalue: &Rvalue<'tcx>,
411-
source_info: SourceInfo,
412-
place: Place<'tcx>,
413-
) -> Option<()> {
408+
fn check_rvalue(&mut self, rvalue: &Rvalue<'tcx>, source_info: SourceInfo) -> Option<()> {
414409
// Perform any special handling for specific Rvalue types.
415410
// Generally, checks here fall into one of two categories:
416411
// 1. Additional checking to provide useful lints to the user
@@ -486,7 +481,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
486481
return None;
487482
}
488483

489-
self.use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, place))
484+
Some(())
485+
}
486+
487+
fn ensure_not_propagated(&mut self, local: Local) {
488+
if cfg!(debug_assertions) {
489+
assert!(
490+
self.get_const(local.into()).is_none()
491+
|| self
492+
.layout_of(self.local_decls[local].ty)
493+
.map_or(true, |layout| layout.is_zst()),
494+
"failed to remove values for `{local:?}`, value={:?}",
495+
self.get_const(local.into()),
496+
)
497+
}
490498
}
491499
}
492500

@@ -507,35 +515,21 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
507515
self.eval_constant(constant, self.source_info.unwrap());
508516
}
509517

510-
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
511-
trace!("visit_statement: {:?}", statement);
512-
let source_info = statement.source_info;
513-
self.source_info = Some(source_info);
514-
515-
// Recurse into statement before applying the assignment.
516-
self.super_statement(statement, location);
517-
518-
match statement.kind {
519-
StatementKind::Assign(box (place, ref rval)) => {
520-
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
521-
if let Some(()) = self.const_prop(rval, source_info, place) {
522-
match can_const_prop {
523-
ConstPropMode::OnlyInsideOwnBlock => {
524-
trace!(
525-
"found local restricted to its block. \
526-
Will remove it from const-prop after block is finished. Local: {:?}",
527-
place.local
528-
);
529-
}
530-
ConstPropMode::NoPropagation => {
531-
trace!("can't propagate into {:?}", place);
532-
if place.local != RETURN_PLACE {
533-
Self::remove_const(&mut self.ecx, place.local);
534-
}
535-
}
536-
ConstPropMode::FullConstProp => {}
537-
}
538-
} else {
518+
fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) {
519+
self.super_assign(place, rvalue, location);
520+
521+
let source_info = self.source_info.unwrap();
522+
let Some(()) = self.check_rvalue(rvalue, source_info) else { return };
523+
524+
match self.ecx.machine.can_const_prop[place.local] {
525+
// Do nothing if the place is indirect.
526+
_ if place.is_indirect() => {}
527+
ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local),
528+
ConstPropMode::OnlyInsideOwnBlock | ConstPropMode::FullConstProp => {
529+
if self
530+
.use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, *place))
531+
.is_none()
532+
{
539533
// Const prop failed, so erase the destination, ensuring that whatever happens
540534
// from here on, does not know about the previous value.
541535
// This is important in case we have
@@ -554,8 +548,23 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
554548
Self::remove_const(&mut self.ecx, place.local);
555549
}
556550
}
551+
}
552+
}
553+
554+
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
555+
trace!("visit_statement: {:?}", statement);
556+
let source_info = statement.source_info;
557+
self.source_info = Some(source_info);
558+
559+
// Recurse into statement before applying the assignment.
560+
self.super_statement(statement, location);
561+
562+
match statement.kind {
557563
StatementKind::SetDiscriminant { ref place, .. } => {
558564
match self.ecx.machine.can_const_prop[place.local] {
565+
// Do nothing if the place is indirect.
566+
_ if place.is_indirect() => {}
567+
ConstPropMode::NoPropagation => self.ensure_not_propagated(place.local),
559568
ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
560569
if self.use_ecx(source_info, |this| this.ecx.statement(statement)).is_some()
561570
{
@@ -564,9 +573,6 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
564573
Self::remove_const(&mut self.ecx, place.local);
565574
}
566575
}
567-
ConstPropMode::NoPropagation => {
568-
Self::remove_const(&mut self.ecx, place.local);
569-
}
570576
}
571577
}
572578
StatementKind::StorageLive(local) => {
@@ -682,30 +688,17 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
682688
fn visit_basic_block_data(&mut self, block: BasicBlock, data: &BasicBlockData<'tcx>) {
683689
self.super_basic_block_data(block, data);
684690

685-
let ensure_not_propagated = |this: &mut Self, local: Local| {
686-
if cfg!(debug_assertions) {
687-
assert!(
688-
this.get_const(local.into()).is_none()
689-
|| this
690-
.layout_of(this.local_decls[local].ty)
691-
.map_or(true, |layout| layout.is_zst()),
692-
"failed to remove values for `{local:?}`, value={:?}",
693-
this.get_const(local.into()),
694-
)
695-
}
696-
};
697-
698691
// We remove all Locals which are restricted in propagation to their containing blocks and
699692
// which were modified in the current block.
700693
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`.
701694
let can_const_prop = std::mem::take(&mut self.ecx.machine.can_const_prop);
702695
for (local, &mode) in can_const_prop.iter_enumerated() {
703696
match mode {
704697
ConstPropMode::FullConstProp => {}
705-
ConstPropMode::NoPropagation => ensure_not_propagated(self, local),
698+
ConstPropMode::NoPropagation => self.ensure_not_propagated(local),
706699
ConstPropMode::OnlyInsideOwnBlock => {
707700
Self::remove_const(&mut self.ecx, local);
708-
ensure_not_propagated(self, local);
701+
self.ensure_not_propagated(local);
709702
}
710703
}
711704
}

0 commit comments

Comments
 (0)