Skip to content

Commit 15e7a67

Browse files
authored
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```
2 parents 11085aa + 1be2204 commit 15e7a67

1 file changed

Lines changed: 46 additions & 67 deletions

File tree

‎compiler/rustc_mir_transform/src/dest_prop.rs‎

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

164164
fnrun_pass(&self,tcx:TyCtxt<'tcx>,body:&mutBody<'tcx>){
165165
let def_id = body.source.def_id();
166-
letmut allocations = Allocations::default();
166+
letmut candidates = Candidates::default();
167+
letmut 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-
letmut 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-
&mutallocations.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-
structAllocations{
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-
structCandidates<'alloc>{
252+
#[derive(Debug,Default)]
253+
structCandidates{
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:&'allocmutFxIndexMap<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:&'allocmutFxIndexMap<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-
structFilterInformation<'a,'body,'alloc,'tcx>{
362-
body:&'bodyBody<'tcx>,
346+
structFilterInformation<'a,'tcx>{
347+
body:&'aBody<'tcx>,
363348
points:&'aDenseLocationMap,
364349
live:&'aSparseIntervalMatrix<Local,PointIndex>,
365-
candidates:&'amutCandidates<'alloc>,
366-
write_info:&'allocmutWriteInfo,
350+
candidates:&'amutCandidates,
351+
write_info:&'amutWriteInfo,
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+
implCandidates{
359+
/// Collects the candidates for merging.
360+
///
361+
/// This is responsible for enforcing the first and third bullet point.
362+
fnreset_and_find<'tcx>(&mutself,body:&Body<'tcx>,borrowed:&BitSet<Local>){
363+
self.c.clear();
364+
self.reverse.clear();
365+
letmut visitor = FindAssignments{ body,candidates:&mutself.c, borrowed };
366+
visitor.visit_body(body);
367+
// Deduplicate candidates.
368+
for(_, cands)inself.c.iter_mut(){
369+
cands.sort();
370+
cands.dedup();
371+
}
372+
// Generate the reverse map.
373+
for(src, cands)inself.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
fnvec_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-
fnfilter_liveness<'b>(
467-
candidates:&mutCandidates<'alloc>,
472+
fnfilter_liveness(
473+
candidates:&mutCandidates,
468474
points:&DenseLocationMap,
469475
live:&SparseIntervalMatrix<Local,PointIndex>,
470-
write_info_alloc:&'allocmutWriteInfo,
471-
body:&'bBody<'tcx>,
476+
write_info:&mutWriteInfo,
477+
body:&Body<'tcx>,
472478
){
473479
letmut 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-
fnfind_candidates<'alloc,'tcx>(
741-
body:&Body<'tcx>,
742-
borrowed:&BitSet<Local>,
743-
candidates:&'allocmutFxIndexMap<Local,Vec<Local>>,
744-
candidates_reverse:&'allocmutFxIndexMap<Local,Vec<Local>>,
745-
) -> Candidates<'alloc>{
746-
candidates.clear();
747-
candidates_reverse.clear();
748-
letmut 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-
structFindAssignments<'a,'alloc,'tcx>{
743+
structFindAssignments<'a,'tcx>{
765744
body:&'aBody<'tcx>,
766-
candidates:&'allocmutFxIndexMap<Local,Vec<Local>>,
745+
candidates:&'amutFxIndexMap<Local,Vec<Local>>,
767746
borrowed:&'aBitSet<Local>,
768747
}
769748

770-
impl<'tcx>Visitor<'tcx>forFindAssignments<'_,'_,'tcx>{
749+
impl<'tcx>Visitor<'tcx>forFindAssignments<'_,'tcx>{
771750
fnvisit_statement(&mutself,statement:&Statement<'tcx>, _:Location){
772751
ifletStatementKind::Assign(box (
773752
lhs,
@@ -819,9 +798,9 @@ fn is_local_required(local: Local, body: &Body<'_>) -> bool {
819798
/////////////////////////////////////////////////////////
820799
// MIR Dump
821800

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

0 commit comments

Comments
 (0)