Do not recompute liveness for DestinationPropagation.

Author: cjgillot

Diff for: compiler/rustc_mir_dataflow/src/points.rs

@@ -1,8 +1,10 @@
+use crate::framework::{visit_results, ResultsVisitable, ResultsVisitor};
+use rustc_index::bit_set::ChunkedBitSet;
 use rustc_index::interval::IntervalSet;
 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};
 use std::rc::Rc;
 
 /// Maps between a `Location` and a `PointIndex` (and vice versa).
@@ -162,4 +164,65 @@ impl<N: Idx> LivenessValues<N> {
     pub fn get_intervals(&self, region: N) -> Option<&IntervalSet<PointIndex>> {
         self.points.row(region)
     }
+
+    /// Insert all the elements from the `src` region into the `dest` region.
+    pub fn union_regions(&mut self, src: N, dest: N) {
+        self.points.union_rows(src, dest);
+    }
+
+    /// Add points depending on the result of the given dataflow analysis.
+    pub fn fill_from_dataflow<'tcx, R>(body: &mir::Body<'tcx>, mut results: R) -> Self
+    where
+        R: ResultsVisitable<'tcx, FlowState = ChunkedBitSet<N>>,
+    {
+        let elements = Rc::new(DenseLocationMap::new(body));
+        let values = Self::new(elements);
+        let mut visitor = Visitor { values };
+        visit_results(
+            body,
+            body.basic_blocks.reverse_postorder().iter().copied(),
+            &mut results,
+            &mut visitor,
+        );
+        visitor.values
+    }
+}
+
+struct Visitor<N: Idx> {
+    values: LivenessValues<N>,
+}
+
+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.values.elements.point_from_location(location);
+        // Use internal iterator manually as it is much more efficient.
+        state.iter().fold((), |(), node| {
+            self.values.points.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.values.elements.point_from_location(location);
+        // Use internal iterator manually as it is much more efficient.
+        state.iter().fold((), |(), node| {
+            self.values.points.insert(node, point);
+        });
+    }
 }

Diff for: compiler/rustc_mir_transform/src/dest_prop.rs

@@ -146,7 +146,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::LivenessValues;
+use rustc_mir_dataflow::Analysis;
 
 pub struct DestinationPropagation;
 
@@ -170,6 +171,12 @@ 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 mut live = LivenessValues::fill_from_dataflow(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,11 +200,7 @@ 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, &live, round_count);
 
             FilterInformation::filter_liveness(
                 &mut candidates,
@@ -206,9 +209,9 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
                 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 +230,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_regions(*src, *dest);
             }
             trace!(merging = ?merges);
 
@@ -358,7 +367,7 @@ 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>,
+    live: &'a LivenessValues<Local>,
     candidates: &'a mut Candidates<'alloc>,
     write_info: &'alloc mut WriteInfo,
     at: Location,
@@ -461,7 +470,7 @@ 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>,
+        live: &LivenessValues<Local>,
         write_info_alloc: &'alloc mut WriteInfo,
         body: &'b Body<'tcx>,
     ) {
@@ -481,13 +490,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();
             }
@@ -517,7 +524,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, self.at) || writes.contains(&q) {
                         CandidateFilter::Remove
                     } else {
                         CandidateFilter::Keep
@@ -810,38 +817,14 @@ 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>,
+    live: &LivenessValues<Local>,
     round: usize,
 ) {
-    let mut reachable = None;
+    let locals_live_at =
+        |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(())