Rollup merge of #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```
This commit is contained in:
Matthias Krüger 2024-09-05 18:58:55 +02:00 committed by GitHub
commit 15e7a67b50
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -163,7 +163,8 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let def_id = body.source.def_id(); let def_id = body.source.def_id();
let mut allocations = Allocations::default(); let mut candidates = Candidates::default();
let mut write_info = WriteInfo::default();
trace!(func = ?tcx.def_path_str(def_id)); trace!(func = ?tcx.def_path_str(def_id));
let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body); let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body);
@ -191,12 +192,7 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
loop { loop {
// PERF: Can we do something smarter than recalculating the candidates and liveness // PERF: Can we do something smarter than recalculating the candidates and liveness
// results? // results?
let mut candidates = find_candidates( candidates.reset_and_find(body, &borrowed);
body,
&borrowed,
&mut allocations.candidates,
&mut allocations.candidates_reverse,
);
trace!(?candidates); trace!(?candidates);
dest_prop_mir_dump(tcx, body, &points, &live, round_count); dest_prop_mir_dump(tcx, body, &points, &live, round_count);
@ -204,7 +200,7 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
&mut candidates, &mut candidates,
&points, &points,
&live, &live,
&mut allocations.write_info, &mut write_info,
body, body,
); );
@ -253,20 +249,8 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
} }
} }
/// Container for the various allocations that we need. #[derive(Debug, Default)]
/// struct Candidates {
/// We store these here and hand out `&mut` access to them, instead of dropping and recreating them
/// frequently. Everything with a `&'alloc` lifetime points into here.
#[derive(Default)]
struct Allocations {
candidates: FxIndexMap<Local, Vec<Local>>,
candidates_reverse: FxIndexMap<Local, Vec<Local>>,
write_info: WriteInfo,
// PERF: Do this for `MaybeLiveLocals` allocations too.
}
#[derive(Debug)]
struct Candidates<'alloc> {
/// The set of candidates we are considering in this optimization. /// The set of candidates we are considering in this optimization.
/// ///
/// We will always merge the key into at most one of its values. /// We will always merge the key into at most one of its values.
@ -281,11 +265,12 @@ struct Candidates<'alloc> {
/// ///
/// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to /// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to
/// remove that assignment. /// remove that assignment.
c: &'alloc mut FxIndexMap<Local, Vec<Local>>, c: FxIndexMap<Local, Vec<Local>>,
/// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`, /// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`,
/// then this contains `b => a`. /// then this contains `b => a`.
// PERF: Possibly these should be `SmallVec`s? // PERF: Possibly these should be `SmallVec`s?
reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>, reverse: FxIndexMap<Local, Vec<Local>>,
} }
////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////
@ -358,19 +343,40 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> {
// //
// This section enforces bullet point 2 // This section enforces bullet point 2
struct FilterInformation<'a, 'body, 'alloc, 'tcx> { struct FilterInformation<'a, 'tcx> {
body: &'body Body<'tcx>, body: &'a Body<'tcx>,
points: &'a DenseLocationMap, points: &'a DenseLocationMap,
live: &'a SparseIntervalMatrix<Local, PointIndex>, live: &'a SparseIntervalMatrix<Local, PointIndex>,
candidates: &'a mut Candidates<'alloc>, candidates: &'a mut Candidates,
write_info: &'alloc mut WriteInfo, write_info: &'a mut WriteInfo,
at: Location, at: Location,
} }
// We first implement some utility functions which we will expose removing candidates according to // We first implement some utility functions which we will expose removing candidates according to
// different needs. Throughout the liveness filtering, the `candidates` are only ever accessed // different needs. Throughout the liveness filtering, the `candidates` are only ever accessed
// through these methods, and not directly. // through these methods, and not directly.
impl<'alloc> Candidates<'alloc> { impl Candidates {
/// Collects the candidates for merging.
///
/// This is responsible for enforcing the first and third bullet point.
fn reset_and_find<'tcx>(&mut self, body: &Body<'tcx>, borrowed: &BitSet<Local>) {
self.c.clear();
self.reverse.clear();
let mut visitor = FindAssignments { body, candidates: &mut self.c, borrowed };
visitor.visit_body(body);
// Deduplicate candidates.
for (_, cands) in self.c.iter_mut() {
cands.sort();
cands.dedup();
}
// Generate the reverse map.
for (src, cands) in self.c.iter() {
for dest in cands.iter().copied() {
self.reverse.entry(dest).or_default().push(*src);
}
}
}
/// Just `Vec::retain`, but the condition is inverted and we add debugging output /// Just `Vec::retain`, but the condition is inverted and we add debugging output
fn vec_filter_candidates( fn vec_filter_candidates(
src: Local, src: Local,
@ -445,7 +451,7 @@ enum CandidateFilter {
Remove, Remove,
} }
impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> { impl<'a, 'tcx> FilterInformation<'a, 'tcx> {
/// Filters the set of candidates to remove those that conflict. /// Filters the set of candidates to remove those that conflict.
/// ///
/// The steps we take are exactly those that are outlined at the top of the file. For each /// 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> {
/// before the statement/terminator will correctly report locals that are read in the /// before the statement/terminator will correctly report locals that are read in the
/// statement/terminator to be live. We are additionally conservative by treating all written to /// statement/terminator to be live. We are additionally conservative by treating all written to
/// locals as also being read from. /// locals as also being read from.
fn filter_liveness<'b>( fn filter_liveness(
candidates: &mut Candidates<'alloc>, candidates: &mut Candidates,
points: &DenseLocationMap, points: &DenseLocationMap,
live: &SparseIntervalMatrix<Local, PointIndex>, live: &SparseIntervalMatrix<Local, PointIndex>,
write_info_alloc: &'alloc mut WriteInfo, write_info: &mut WriteInfo,
body: &'b Body<'tcx>, body: &Body<'tcx>,
) { ) {
let mut this = FilterInformation { let mut this = FilterInformation {
body, body,
@ -477,7 +483,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
candidates, candidates,
// We don't actually store anything at this scope, we just keep things here to be able // We don't actually store anything at this scope, we just keep things here to be able
// to reuse the allocation. // to reuse the allocation.
write_info: write_info_alloc, write_info,
// Doesn't matter what we put here, will be overwritten before being used // Doesn't matter what we put here, will be overwritten before being used
at: Location::START, at: Location::START,
}; };
@ -734,40 +740,13 @@ fn places_to_candidate_pair<'tcx>(
Some((a, b)) Some((a, b))
} }
/// Collects the candidates for merging struct FindAssignments<'a, 'tcx> {
///
/// This is responsible for enforcing the first and third bullet point.
fn find_candidates<'alloc, 'tcx>(
body: &Body<'tcx>,
borrowed: &BitSet<Local>,
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
candidates_reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
) -> Candidates<'alloc> {
candidates.clear();
candidates_reverse.clear();
let mut visitor = FindAssignments { body, candidates, borrowed };
visitor.visit_body(body);
// Deduplicate candidates
for (_, cands) in candidates.iter_mut() {
cands.sort();
cands.dedup();
}
// Generate the reverse map
for (src, cands) in candidates.iter() {
for dest in cands.iter().copied() {
candidates_reverse.entry(dest).or_default().push(*src);
}
}
Candidates { c: candidates, reverse: candidates_reverse }
}
struct FindAssignments<'a, 'alloc, 'tcx> {
body: &'a Body<'tcx>, body: &'a Body<'tcx>,
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>, candidates: &'a mut FxIndexMap<Local, Vec<Local>>,
borrowed: &'a BitSet<Local>, borrowed: &'a BitSet<Local>,
} }
impl<'tcx> Visitor<'tcx> for FindAssignments<'_, '_, 'tcx> { impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) { fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) {
if let StatementKind::Assign(box ( if let StatementKind::Assign(box (
lhs, lhs,
@ -819,9 +798,9 @@ fn is_local_required(local: Local, body: &Body<'_>) -> bool {
///////////////////////////////////////////////////////// /////////////////////////////////////////////////////////
// MIR Dump // MIR Dump
fn dest_prop_mir_dump<'body, 'tcx>( fn dest_prop_mir_dump<'tcx>(
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
body: &'body Body<'tcx>, body: &Body<'tcx>,
points: &DenseLocationMap, points: &DenseLocationMap,
live: &SparseIntervalMatrix<Local, PointIndex>, live: &SparseIntervalMatrix<Local, PointIndex>,
round: usize, round: usize,