Do not recompute liveness for DestinationPropagation.

Author: cjgillot

compiler/rustc_mir_dataflow/src/points.rs

@@ -1,6 +1,9 @@
+use crate::framework::{visit_results, ResultsVisitable, ResultsVisitor};
+use rustc_index::bit_set::ChunkedBitSet;
+use rustc_index::interval::SparseIntervalMatrix;
 use rustc_index::Idx;
 use rustc_index::IndexVec;
-use rustc_middle::mir::{BasicBlock, Body, Location};
+use rustc_middle::mir::{self, BasicBlock, Body, Location};
 
 /// Maps between a `Location` and a `PointIndex` (and vice versa).
 pub struct DenseLocationMap {
@@ -92,3 +95,64 @@ rustc_index::newtype_index! {
     #[debug_format = "PointIndex({})"]
     pub struct PointIndex {}
 }
+
+/// Add points depending on the result of the given dataflow analysis.
+pub fn save_as_intervals<'tcx, N, R>(
+    elements: &DenseLocationMap,
+    body: &mir::Body<'tcx>,
+    mut results: R,
+) -> SparseIntervalMatrix<N, PointIndex>
+where
+    N: Idx,
+    R: ResultsVisitable<'tcx, FlowState = ChunkedBitSet<N>>,
+{
+    let values = SparseIntervalMatrix::new(elements.num_points());
+    let mut visitor = Visitor { elements, values };
+    visit_results(
+        body,
+        body.basic_blocks.reverse_postorder().iter().copied(),
+        &mut results,
+        &mut visitor,
+    );
+    visitor.values
+}
+
+struct Visitor<'a, N: Idx> {
+    elements: &'a DenseLocationMap,
+    values: SparseIntervalMatrix<N, PointIndex>,
+}
+
+impl<'mir, 'tcx, R, N> ResultsVisitor<'mir, 'tcx, R> for Visitor<'_, N>
+where
+    N: Idx,
+{
+    type FlowState = ChunkedBitSet<N>;
+
+    fn visit_statement_after_primary_effect(
+        &mut self,
+        _results: &mut R,
+        state: &Self::FlowState,
+        _statement: &'mir mir::Statement<'tcx>,
+        location: Location,
+    ) {
+        let point = self.elements.point_from_location(location);
+        // Use internal iterator manually as it is much more efficient.
+        state.iter().fold((), |(), node| {
+            self.values.insert(node, point);
+        });
+    }
+
+    fn visit_terminator_after_primary_effect(
+        &mut self,
+        _results: &mut R,
+        state: &Self::FlowState,
+        _terminator: &'mir mir::Terminator<'tcx>,
+        location: Location,
+    ) {
+        let point = self.elements.point_from_location(location);
+        // Use internal iterator manually as it is much more efficient.
+        state.iter().fold((), |(), node| {
+            self.values.insert(node, point);
+        });
+    }
+}

compiler/rustc_mir_transform/src/dest_prop.rs

@@ -137,6 +137,7 @@ use crate::simplify::remove_dead_blocks;
 use crate::MirPass;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_index::bit_set::BitSet;
+use rustc_index::interval::SparseIntervalMatrix;
 use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
 use rustc_middle::mir::HasLocalDecls;
 use rustc_middle::mir::{dump_mir, PassWhere};
@@ -146,7 +147,8 @@ use rustc_middle::mir::{
 };
 use rustc_middle::ty::TyCtxt;
 use rustc_mir_dataflow::impls::MaybeLiveLocals;
-use rustc_mir_dataflow::{Analysis, ResultsCursor};
+use rustc_mir_dataflow::points::{save_as_intervals, DenseLocationMap, PointIndex};
+use rustc_mir_dataflow::Analysis;
 
 pub struct DestinationPropagation;
 
@@ -170,6 +172,13 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
 
         let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body);
 
+        let live = MaybeLiveLocals
+            .into_engine(tcx, body)
+            .pass_name("MaybeLiveLocals-DestinationPropagation")
+            .iterate_to_fixpoint();
+        let points = DenseLocationMap::new(body);
+        let mut live = save_as_intervals(&points, body, live);
+
         // In order to avoid having to collect data for every single pair of locals in the body, we
         // do not allow doing more than one merge for places that are derived from the same local at
         // once. To avoid missed opportunities, we instead iterate to a fixed point - we'll refer to
@@ -193,22 +202,19 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
                 &mut allocations.candidates_reverse,
             );
             trace!(?candidates);
-            let mut live = MaybeLiveLocals
-                .into_engine(tcx, body)
-                .iterate_to_fixpoint()
-                .into_results_cursor(body);
-            dest_prop_mir_dump(tcx, body, &mut live, round_count);
+            dest_prop_mir_dump(tcx, body, &points, &live, round_count);
 
             FilterInformation::filter_liveness(
                 &mut candidates,
-                &mut live,
+                &points,
+                &live,
                 &mut allocations.write_info,
                 body,
             );
 
-            // Because we do not update liveness information, it is unsound to use a local for more
-            // than one merge operation within a single round of optimizations. We store here which
-            // ones we have already used.
+            // Because we only filter once per round, it is unsound to use a local for more than
+            // one merge operation within a single round of optimizations. We store here which ones
+            // we have already used.
             let mut merged_locals: BitSet<Local> = BitSet::new_empty(body.local_decls.len());
 
             // This is the set of merges we will apply this round. It is a subset of the candidates.
@@ -227,9 +233,15 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
                 }) {
                     break;
                 }
+
+                // Replace `src` by `dest` everywhere.
                 merges.insert(*src, *dest);
                 merged_locals.insert(*src);
                 merged_locals.insert(*dest);
+
+                // Update liveness information based on the merge we just performed.
+                // Every location where `src` was live, `dest` will be live.
+                live.union_rows(*src, *dest);
             }
             trace!(merging = ?merges);
 
@@ -358,7 +370,8 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> {
 
 struct FilterInformation<'a, 'body, 'alloc, 'tcx> {
     body: &'body Body<'tcx>,
-    live: &'a mut ResultsCursor<'body, 'tcx, MaybeLiveLocals>,
+    points: &'a DenseLocationMap,
+    live: &'a SparseIntervalMatrix<Local, PointIndex>,
     candidates: &'a mut Candidates<'alloc>,
     write_info: &'alloc mut WriteInfo,
     at: Location,
@@ -461,12 +474,14 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
     /// locals as also being read from.
     fn filter_liveness<'b>(
         candidates: &mut Candidates<'alloc>,
-        live: &mut ResultsCursor<'b, 'tcx, MaybeLiveLocals>,
+        points: &DenseLocationMap,
+        live: &SparseIntervalMatrix<Local, PointIndex>,
         write_info_alloc: &'alloc mut WriteInfo,
         body: &'b Body<'tcx>,
     ) {
         let mut this = FilterInformation {
             body,
+            points,
             live,
             candidates,
             // We don't actually store anything at this scope, we just keep things here to be able
@@ -481,13 +496,11 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
     fn internal_filter_liveness(&mut self) {
         for (block, data) in traversal::preorder(self.body) {
             self.at = Location { block, statement_index: data.statements.len() };
-            self.live.seek_after_primary_effect(self.at);
             self.write_info.for_terminator(&data.terminator().kind);
             self.apply_conflicts();
 
             for (i, statement) in data.statements.iter().enumerate().rev() {
                 self.at = Location { block, statement_index: i };
-                self.live.seek_after_primary_effect(self.at);
                 self.write_info.for_statement(&statement.kind, self.body);
                 self.apply_conflicts();
             }
@@ -506,6 +519,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
                     None
                 }
             });
+            let at = self.points.point_from_location(self.at);
             self.candidates.filter_candidates_by(
                 *p,
                 |q| {
@@ -517,7 +531,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
                     // calls or inline asm. Because of this, we also mark locals as
                     // conflicting when both of them are written to in the same
                     // statement.
-                    if self.live.contains(q) || writes.contains(&q) {
+                    if self.live.contains(q, at) || writes.contains(&q) {
                         CandidateFilter::Remove
                     } else {
                         CandidateFilter::Keep
@@ -810,38 +824,17 @@ fn is_local_required(local: Local, body: &Body<'_>) -> bool {
 fn dest_prop_mir_dump<'body, 'tcx>(
     tcx: TyCtxt<'tcx>,
     body: &'body Body<'tcx>,
-    live: &mut ResultsCursor<'body, 'tcx, MaybeLiveLocals>,
+    points: &DenseLocationMap,
+    live: &SparseIntervalMatrix<Local, PointIndex>,
     round: usize,
 ) {
-    let mut reachable = None;
+    let locals_live_at = |location| {
+        let location = points.point_from_location(location);
+        live.rows().filter(|&r| live.contains(r, location)).collect::<Vec<_>>()
+    };
     dump_mir(tcx, false, "DestinationPropagation-dataflow", &round, body, |pass_where, w| {
-        let reachable = reachable.get_or_insert_with(|| traversal::reachable_as_bitset(body));
-
-        match pass_where {
-            PassWhere::BeforeLocation(loc) if reachable.contains(loc.block) => {
-                live.seek_after_primary_effect(loc);
-                writeln!(w, "        // live: {:?}", live.get())?;
-            }
-            PassWhere::AfterTerminator(bb) if reachable.contains(bb) => {
-                let loc = body.terminator_loc(bb);
-                live.seek_before_primary_effect(loc);
-                writeln!(w, "        // live: {:?}", live.get())?;
-            }
-
-            PassWhere::BeforeBlock(bb) if reachable.contains(bb) => {
-                live.seek_to_block_start(bb);
-                writeln!(w, "    // live: {:?}", live.get())?;
-            }
-
-            PassWhere::BeforeCFG | PassWhere::AfterCFG | PassWhere::AfterLocation(_) => {}
-
-            PassWhere::BeforeLocation(_) | PassWhere::AfterTerminator(_) => {
-                writeln!(w, "        // live: <unreachable>")?;
-            }
-
-            PassWhere::BeforeBlock(_) => {
-                writeln!(w, "    // live: <unreachable>")?;
-            }
+        if let PassWhere::BeforeLocation(loc) = pass_where {
+            writeln!(w, "        // live: {:?}", locals_live_at(loc))?;
         }
 
         Ok(())