Skip to content

Commit 5fa73a7

Browse files
committed
Auto merge of #109087 - cjgillot:sparse-bb-clear, r=davidtwco
Only clear written-to locals in ConstProp This aims to revert the regression in #108872 Clearing all locals at the end of each bb is very costly. This PR goes back to the original implementation of recording which locals are modified.
2 parents 6dc3999 + e5a55dc commit 5fa73a7

File tree

2 files changed

+61
-22
lines changed

2 files changed

+61
-22
lines changed

compiler/rustc_mir_transform/src/const_prop.rs

+36-12
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use either::Right;
55

66
use rustc_const_eval::const_eval::CheckAlignment;
7+
use rustc_data_structures::fx::FxHashSet;
78
use rustc_hir::def::DefKind;
89
use rustc_index::bit_set::BitSet;
910
use rustc_index::vec::IndexVec;
@@ -151,12 +152,17 @@ impl<'tcx> MirPass<'tcx> for ConstProp {
151152
pub struct ConstPropMachine<'mir, 'tcx> {
152153
/// The virtual call stack.
153154
stack: Vec<Frame<'mir, 'tcx>>,
155+
pub written_only_inside_own_block_locals: FxHashSet<Local>,
154156
pub can_const_prop: IndexVec<Local, ConstPropMode>,
155157
}
156158

157159
impl ConstPropMachine<'_, '_> {
158160
pub fn new(can_const_prop: IndexVec<Local, ConstPropMode>) -> Self {
159-
Self { stack: Vec::new(), can_const_prop }
161+
Self {
162+
stack: Vec::new(),
163+
written_only_inside_own_block_locals: Default::default(),
164+
can_const_prop,
165+
}
160166
}
161167
}
162168

@@ -249,7 +255,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
249255
"tried to write to a local that is marked as not propagatable"
250256
)
251257
}
252-
ConstPropMode::OnlyInsideOwnBlock | ConstPropMode::FullConstProp => {}
258+
ConstPropMode::OnlyInsideOwnBlock => {
259+
ecx.machine.written_only_inside_own_block_locals.insert(local);
260+
}
261+
ConstPropMode::FullConstProp => {}
253262
}
254263
ecx.machine.stack[frame].locals[local].access_mut()
255264
}
@@ -416,6 +425,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
416425
fn remove_const(ecx: &mut InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, local: Local) {
417426
ecx.frame_mut().locals[local].value =
418427
LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit));
428+
ecx.machine.written_only_inside_own_block_locals.remove(&local);
419429
}
420430

421431
/// Returns the value, if any, of evaluating `c`.
@@ -693,7 +703,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
693703
}
694704
}
695705

696-
fn ensure_not_propagated(&mut self, local: Local) {
706+
fn ensure_not_propagated(&self, local: Local) {
697707
if cfg!(debug_assertions) {
698708
assert!(
699709
self.get_const(local.into()).is_none()
@@ -963,17 +973,31 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
963973
// We remove all Locals which are restricted in propagation to their containing blocks and
964974
// which were modified in the current block.
965975
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`.
966-
let can_const_prop = std::mem::take(&mut self.ecx.machine.can_const_prop);
967-
for (local, &mode) in can_const_prop.iter_enumerated() {
968-
match mode {
969-
ConstPropMode::FullConstProp => {}
970-
ConstPropMode::NoPropagation => self.ensure_not_propagated(local),
971-
ConstPropMode::OnlyInsideOwnBlock => {
972-
Self::remove_const(&mut self.ecx, local);
973-
self.ensure_not_propagated(local);
976+
let mut written_only_inside_own_block_locals =
977+
std::mem::take(&mut self.ecx.machine.written_only_inside_own_block_locals);
978+
979+
// This loop can get very hot for some bodies: it check each local in each bb.
980+
// To avoid this quadratic behaviour, we only clear the locals that were modified inside
981+
// the current block.
982+
for local in written_only_inside_own_block_locals.drain() {
983+
debug_assert_eq!(
984+
self.ecx.machine.can_const_prop[local],
985+
ConstPropMode::OnlyInsideOwnBlock
986+
);
987+
Self::remove_const(&mut self.ecx, local);
988+
}
989+
self.ecx.machine.written_only_inside_own_block_locals =
990+
written_only_inside_own_block_locals;
991+
992+
if cfg!(debug_assertions) {
993+
for (local, &mode) in self.ecx.machine.can_const_prop.iter_enumerated() {
994+
match mode {
995+
ConstPropMode::FullConstProp => {}
996+
ConstPropMode::NoPropagation | ConstPropMode::OnlyInsideOwnBlock => {
997+
self.ensure_not_propagated(local);
998+
}
974999
}
9751000
}
9761001
}
977-
self.ecx.machine.can_const_prop = can_const_prop;
9781002
}
9791003
}

compiler/rustc_mir_transform/src/const_prop_lint.rs

+25-10
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
247247
fn remove_const(ecx: &mut InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, local: Local) {
248248
ecx.frame_mut().locals[local].value =
249249
LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit));
250+
ecx.machine.written_only_inside_own_block_locals.remove(&local);
250251
}
251252

252253
fn lint_root(&self, source_info: SourceInfo) -> Option<HirId> {
@@ -484,7 +485,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
484485
Some(())
485486
}
486487

487-
fn ensure_not_propagated(&mut self, local: Local) {
488+
fn ensure_not_propagated(&self, local: Local) {
488489
if cfg!(debug_assertions) {
489490
assert!(
490491
self.get_const(local.into()).is_none()
@@ -691,17 +692,31 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
691692
// We remove all Locals which are restricted in propagation to their containing blocks and
692693
// which were modified in the current block.
693694
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`.
694-
let can_const_prop = std::mem::take(&mut self.ecx.machine.can_const_prop);
695-
for (local, &mode) in can_const_prop.iter_enumerated() {
696-
match mode {
697-
ConstPropMode::FullConstProp => {}
698-
ConstPropMode::NoPropagation => self.ensure_not_propagated(local),
699-
ConstPropMode::OnlyInsideOwnBlock => {
700-
Self::remove_const(&mut self.ecx, local);
701-
self.ensure_not_propagated(local);
695+
let mut written_only_inside_own_block_locals =
696+
std::mem::take(&mut self.ecx.machine.written_only_inside_own_block_locals);
697+
698+
// This loop can get very hot for some bodies: it check each local in each bb.
699+
// To avoid this quadratic behaviour, we only clear the locals that were modified inside
700+
// the current block.
701+
for local in written_only_inside_own_block_locals.drain() {
702+
debug_assert_eq!(
703+
self.ecx.machine.can_const_prop[local],
704+
ConstPropMode::OnlyInsideOwnBlock
705+
);
706+
Self::remove_const(&mut self.ecx, local);
707+
}
708+
self.ecx.machine.written_only_inside_own_block_locals =
709+
written_only_inside_own_block_locals;
710+
711+
if cfg!(debug_assertions) {
712+
for (local, &mode) in self.ecx.machine.can_const_prop.iter_enumerated() {
713+
match mode {
714+
ConstPropMode::FullConstProp => {}
715+
ConstPropMode::NoPropagation | ConstPropMode::OnlyInsideOwnBlock => {
716+
self.ensure_not_propagated(local);
717+
}
702718
}
703719
}
704720
}
705-
self.ecx.machine.can_const_prop = can_const_prop;
706721
}
707722
}

0 commit comments

Comments
 (0)