Skip to content

Commit efdbbee

Browse files
authored
Rollup merge of rust-lang#129720 - nnethercote:simplify-dest_prop-mm, r=cjgillot
Simplify DestProp memory management The DestProp MIR pass has some convoluted memory management. This PR simplifies it. r? `@davidtwco`
2 parents fe5e59a + 1be2204 commit efdbbee

File tree

1 file changed

+46
-67
lines changed

1 file changed

+46
-67
lines changed

compiler/rustc_mir_transform/src/dest_prop.rs

+46-67
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
163163

164164
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
165165
let def_id = body.source.def_id();
166-
let mut allocations = Allocations::default();
166+
let mut candidates = Candidates::default();
167+
let mut write_info = WriteInfo::default();
167168
trace!(func = ?tcx.def_path_str(def_id));
168169

169170
let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body);
@@ -191,20 +192,15 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
191192
loop {
192193
// PERF: Can we do something smarter than recalculating the candidates and liveness
193194
// results?
194-
let mut candidates = find_candidates(
195-
body,
196-
&borrowed,
197-
&mut allocations.candidates,
198-
&mut allocations.candidates_reverse,
199-
);
195+
candidates.reset_and_find(body, &borrowed);
200196
trace!(?candidates);
201197
dest_prop_mir_dump(tcx, body, &points, &live, round_count);
202198

203199
FilterInformation::filter_liveness(
204200
&mut candidates,
205201
&points,
206202
&live,
207-
&mut allocations.write_info,
203+
&mut write_info,
208204
body,
209205
);
210206

@@ -253,20 +249,8 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
253249
}
254250
}
255251

256-
/// Container for the various allocations that we need.
257-
///
258-
/// We store these here and hand out `&mut` access to them, instead of dropping and recreating them
259-
/// frequently. Everything with a `&'alloc` lifetime points into here.
260-
#[derive(Default)]
261-
struct Allocations {
262-
candidates: FxIndexMap<Local, Vec<Local>>,
263-
candidates_reverse: FxIndexMap<Local, Vec<Local>>,
264-
write_info: WriteInfo,
265-
// PERF: Do this for `MaybeLiveLocals` allocations too.
266-
}
267-
268-
#[derive(Debug)]
269-
struct Candidates<'alloc> {
252+
#[derive(Debug, Default)]
253+
struct Candidates {
270254
/// The set of candidates we are considering in this optimization.
271255
///
272256
/// We will always merge the key into at most one of its values.
@@ -281,11 +265,12 @@ struct Candidates<'alloc> {
281265
///
282266
/// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to
283267
/// remove that assignment.
284-
c: &'alloc mut FxIndexMap<Local, Vec<Local>>,
268+
c: FxIndexMap<Local, Vec<Local>>,
269+
285270
/// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`,
286271
/// then this contains `b => a`.
287272
// PERF: Possibly these should be `SmallVec`s?
288-
reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
273+
reverse: FxIndexMap<Local, Vec<Local>>,
289274
}
290275

291276
//////////////////////////////////////////////////////////
@@ -358,19 +343,40 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> {
358343
//
359344
// This section enforces bullet point 2
360345

361-
struct FilterInformation<'a, 'body, 'alloc, 'tcx> {
362-
body: &'body Body<'tcx>,
346+
struct FilterInformation<'a, 'tcx> {
347+
body: &'a Body<'tcx>,
363348
points: &'a DenseLocationMap,
364349
live: &'a SparseIntervalMatrix<Local, PointIndex>,
365-
candidates: &'a mut Candidates<'alloc>,
366-
write_info: &'alloc mut WriteInfo,
350+
candidates: &'a mut Candidates,
351+
write_info: &'a mut WriteInfo,
367352
at: Location,
368353
}
369354

370355
// We first implement some utility functions which we will expose removing candidates according to
371356
// different needs. Throughout the liveness filtering, the `candidates` are only ever accessed
372357
// through these methods, and not directly.
373-
impl<'alloc> Candidates<'alloc> {
358+
impl Candidates {
359+
/// Collects the candidates for merging.
360+
///
361+
/// This is responsible for enforcing the first and third bullet point.
362+
fn reset_and_find<'tcx>(&mut self, body: &Body<'tcx>, borrowed: &BitSet<Local>) {
363+
self.c.clear();
364+
self.reverse.clear();
365+
let mut visitor = FindAssignments { body, candidates: &mut self.c, borrowed };
366+
visitor.visit_body(body);
367+
// Deduplicate candidates.
368+
for (_, cands) in self.c.iter_mut() {
369+
cands.sort();
370+
cands.dedup();
371+
}
372+
// Generate the reverse map.
373+
for (src, cands) in self.c.iter() {
374+
for dest in cands.iter().copied() {
375+
self.reverse.entry(dest).or_default().push(*src);
376+
}
377+
}
378+
}
379+
374380
/// Just `Vec::retain`, but the condition is inverted and we add debugging output
375381
fn vec_filter_candidates(
376382
src: Local,
@@ -445,7 +451,7 @@ enum CandidateFilter {
445451
Remove,
446452
}
447453

448-
impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
454+
impl<'a, 'tcx> FilterInformation<'a, 'tcx> {
449455
/// Filters the set of candidates to remove those that conflict.
450456
///
451457
/// The steps we take are exactly those that are outlined at the top of the file. For each
@@ -463,12 +469,12 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
463469
/// before the statement/terminator will correctly report locals that are read in the
464470
/// statement/terminator to be live. We are additionally conservative by treating all written to
465471
/// locals as also being read from.
466-
fn filter_liveness<'b>(
467-
candidates: &mut Candidates<'alloc>,
472+
fn filter_liveness(
473+
candidates: &mut Candidates,
468474
points: &DenseLocationMap,
469475
live: &SparseIntervalMatrix<Local, PointIndex>,
470-
write_info_alloc: &'alloc mut WriteInfo,
471-
body: &'b Body<'tcx>,
476+
write_info: &mut WriteInfo,
477+
body: &Body<'tcx>,
472478
) {
473479
let mut this = FilterInformation {
474480
body,
@@ -477,7 +483,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
477483
candidates,
478484
// We don't actually store anything at this scope, we just keep things here to be able
479485
// to reuse the allocation.
480-
write_info: write_info_alloc,
486+
write_info,
481487
// Doesn't matter what we put here, will be overwritten before being used
482488
at: Location::START,
483489
};
@@ -734,40 +740,13 @@ fn places_to_candidate_pair<'tcx>(
734740
Some((a, b))
735741
}
736742

737-
/// Collects the candidates for merging
738-
///
739-
/// This is responsible for enforcing the first and third bullet point.
740-
fn find_candidates<'alloc, 'tcx>(
741-
body: &Body<'tcx>,
742-
borrowed: &BitSet<Local>,
743-
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
744-
candidates_reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
745-
) -> Candidates<'alloc> {
746-
candidates.clear();
747-
candidates_reverse.clear();
748-
let mut visitor = FindAssignments { body, candidates, borrowed };
749-
visitor.visit_body(body);
750-
// Deduplicate candidates
751-
for (_, cands) in candidates.iter_mut() {
752-
cands.sort();
753-
cands.dedup();
754-
}
755-
// Generate the reverse map
756-
for (src, cands) in candidates.iter() {
757-
for dest in cands.iter().copied() {
758-
candidates_reverse.entry(dest).or_default().push(*src);
759-
}
760-
}
761-
Candidates { c: candidates, reverse: candidates_reverse }
762-
}
763-
764-
struct FindAssignments<'a, 'alloc, 'tcx> {
743+
struct FindAssignments<'a, 'tcx> {
765744
body: &'a Body<'tcx>,
766-
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
745+
candidates: &'a mut FxIndexMap<Local, Vec<Local>>,
767746
borrowed: &'a BitSet<Local>,
768747
}
769748

770-
impl<'tcx> Visitor<'tcx> for FindAssignments<'_, '_, 'tcx> {
749+
impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
771750
fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) {
772751
if let StatementKind::Assign(box (
773752
lhs,
@@ -819,9 +798,9 @@ fn is_local_required(local: Local, body: &Body<'_>) -> bool {
819798
/////////////////////////////////////////////////////////
820799
// MIR Dump
821800

822-
fn dest_prop_mir_dump<'body, 'tcx>(
801+
fn dest_prop_mir_dump<'tcx>(
823802
tcx: TyCtxt<'tcx>,
824-
body: &'body Body<'tcx>,
803+
body: &Body<'tcx>,
825804
points: &DenseLocationMap,
826805
live: &SparseIntervalMatrix<Local, PointIndex>,
827806
round: usize,

0 commit comments

Comments
 (0)