Auto merge of #61787 - ecstatic-morse:dataflow-split-block-sets, r=pnkfelix

rustc_mir: Hide initial block state when defining transfer functions

This PR addresses [this FIXME](https://github.com/rust-lang/rust/blob/2887008e0ce0824be4e0e9562c22ea397b165c97/src/librustc_mir/dataflow/mod.rs#L594-L596).

This makes `sets.on_entry` inaccessible in `{before_,}{statement,terminator}_effect`. This field was meant to allow implementors of `BitDenotation` to access the initial state for each block (optionally with the effect of all previous statements applied via `accumulates_intrablock_state`) while defining transfer functions.  However, the ability to set the initial value for the entry set of each basic block (except for START_BLOCK) no longer exists. As a result, this functionality is mostly useless, and when it *was* used it was used erroneously (see #62007).

Since `on_entry` is now useless, we can also remove `BlockSets`, which held the `gen`, `kill`, and `on_entry` bitvectors and replace it with a `GenKill` struct. Variables of this type are called `trans` since they represent a transfer function. `GenKill`s are stored contiguously in `AllSets`, which reduces the number of bounds checks and may improve cache performance: one is almost never accessed without the other.

Replacing `BlockSets` with `GenKill` allows us to define some new helper functions which streamline dataflow iteration and the dataflow-at-location APIs. Notably, `state_for_location` used a subtle side-effect of the `kill`/`kill_all` setters to apply the transfer function, and could be incorrect if a transfer function depended on effects of previous statements in the block on `gen_set`.

Additionally, this PR merges `BitSetOperator` and `InitialFlow` into one trait. Since the value of `InitialFlow` defines the semantics of the `join` operation, there's no reason to have seperate traits for each. We can add a default impl of `join` which branches based on `BOTTOM_VALUE`.  This should get optimized away.

Author: bors

src/librustc_data_structures/bit_set.rs

@@ -316,11 +316,6 @@ impl<'a, T: Idx> Iterator for BitIter<'a, T> {
     }
 }
 
-pub trait BitSetOperator {
-    /// Combine one bitset into another.
-    fn join<T: Idx>(&self, inout_set: &mut BitSet<T>, in_set: &BitSet<T>) -> bool;
-}
-
 #[inline]
 fn bitwise<Op>(out_vec: &mut [Word], in_vec: &[Word], op: Op) -> bool
     where Op: Fn(Word, Word) -> Word

src/librustc_mir/dataflow/at_location.rs

@@ -4,7 +4,7 @@
 use rustc::mir::{BasicBlock, Location};
 use rustc_data_structures::bit_set::{BitIter, BitSet, HybridBitSet};
 
-use crate::dataflow::{BitDenotation, BlockSets, DataflowResults};
+use crate::dataflow::{BitDenotation, DataflowResults, GenKillSet};
 use crate::dataflow::move_paths::{HasMoveData, MovePathIndex};
 
 use std::iter;
@@ -66,8 +66,7 @@ where
 {
     base_results: DataflowResults<'tcx, BD>,
     curr_state: BitSet<BD::Idx>,
-    stmt_gen: HybridBitSet<BD::Idx>,
-    stmt_kill: HybridBitSet<BD::Idx>,
+    stmt_trans: GenKillSet<BD::Idx>,
 }
 
 impl<'tcx, BD> FlowAtLocation<'tcx, BD>
@@ -89,19 +88,17 @@ where
     where
         F: FnMut(BD::Idx),
     {
-        self.stmt_gen.iter().for_each(f)
+        self.stmt_trans.gen_set.iter().for_each(f)
     }
 
     pub fn new(results: DataflowResults<'tcx, BD>) -> Self {
         let bits_per_block = results.sets().bits_per_block();
         let curr_state = BitSet::new_empty(bits_per_block);
-        let stmt_gen = HybridBitSet::new_empty(bits_per_block);
-        let stmt_kill = HybridBitSet::new_empty(bits_per_block);
+        let stmt_trans = GenKillSet::from_elem(HybridBitSet::new_empty(bits_per_block));
         FlowAtLocation {
             base_results: results,
-            curr_state: curr_state,
-            stmt_gen: stmt_gen,
-            stmt_kill: stmt_kill,
+            curr_state,
+            stmt_trans,
         }
     }
 
@@ -127,8 +124,7 @@ where
         F: FnOnce(BitIter<'_, BD::Idx>),
     {
         let mut curr_state = self.curr_state.clone();
-        curr_state.union(&self.stmt_gen);
-        curr_state.subtract(&self.stmt_kill);
+        self.stmt_trans.apply(&mut curr_state);
         f(curr_state.iter());
     }
 
@@ -142,68 +138,41 @@ impl<'tcx, BD> FlowsAtLocation for FlowAtLocation<'tcx, BD>
     where BD: BitDenotation<'tcx>
 {
     fn reset_to_entry_of(&mut self, bb: BasicBlock) {
-        self.curr_state.overwrite(self.base_results.sets().on_entry_set_for(bb.index()));
+        self.curr_state.overwrite(self.base_results.sets().entry_set_for(bb.index()));
     }
 
     fn reset_to_exit_of(&mut self, bb: BasicBlock) {
         self.reset_to_entry_of(bb);
-        self.curr_state.union(self.base_results.sets().gen_set_for(bb.index()));
-        self.curr_state.subtract(self.base_results.sets().kill_set_for(bb.index()));
+        let trans = self.base_results.sets().trans_for(bb.index());
+        trans.apply(&mut self.curr_state)
     }
 
     fn reconstruct_statement_effect(&mut self, loc: Location) {
-        self.stmt_gen.clear();
-        self.stmt_kill.clear();
-        {
-            let mut sets = BlockSets {
-                on_entry: &mut self.curr_state,
-                gen_set: &mut self.stmt_gen,
-                kill_set: &mut self.stmt_kill,
-            };
-            self.base_results
-                .operator()
-                .before_statement_effect(&mut sets, loc);
-        }
-        self.apply_local_effect(loc);
+        self.stmt_trans.clear();
+        self.base_results
+            .operator()
+            .before_statement_effect(&mut self.stmt_trans, loc);
+        self.stmt_trans.apply(&mut self.curr_state);
 
-        let mut sets = BlockSets {
-            on_entry: &mut self.curr_state,
-            gen_set: &mut self.stmt_gen,
-            kill_set: &mut self.stmt_kill,
-        };
         self.base_results
             .operator()
-            .statement_effect(&mut sets, loc);
+            .statement_effect(&mut self.stmt_trans, loc);
     }
 
     fn reconstruct_terminator_effect(&mut self, loc: Location) {
-        self.stmt_gen.clear();
-        self.stmt_kill.clear();
-        {
-            let mut sets = BlockSets {
-                on_entry: &mut self.curr_state,
-                gen_set: &mut self.stmt_gen,
-                kill_set: &mut self.stmt_kill,
-            };
-            self.base_results
-                .operator()
-                .before_terminator_effect(&mut sets, loc);
-        }
-        self.apply_local_effect(loc);
+        self.stmt_trans.clear();
+        self.base_results
+            .operator()
+            .before_terminator_effect(&mut self.stmt_trans, loc);
+        self.stmt_trans.apply(&mut self.curr_state);
 
-        let mut sets = BlockSets {
-            on_entry: &mut self.curr_state,
-            gen_set: &mut self.stmt_gen,
-            kill_set: &mut self.stmt_kill,
-        };
         self.base_results
             .operator()
-            .terminator_effect(&mut sets, loc);
+            .terminator_effect(&mut self.stmt_trans, loc);
     }
 
     fn apply_local_effect(&mut self, _loc: Location) {
-        self.curr_state.union(&self.stmt_gen);
-        self.curr_state.subtract(&self.stmt_kill);
+        self.stmt_trans.apply(&mut self.curr_state)
     }
 }
 

src/librustc_mir/dataflow/graphviz.rs

@@ -170,7 +170,7 @@ where MWF: MirWithFlowState<'tcx>,
 
         write!(w, "<tr>")?;
         // Entry
-        dump_set_for!(on_entry_set_for, interpret_set);
+        dump_set_for!(entry_set_for, interpret_set);
 
         // MIR statements
         write!(w, "<td>")?;
@@ -208,7 +208,7 @@ where MWF: MirWithFlowState<'tcx>,
         write!(w, "<tr>")?;
 
         // Entry
-        let set = flow.sets.on_entry_set_for(i);
+        let set = flow.sets.entry_set_for(i);
         write!(w, "<td>{:?}</td>", dot::escape_html(&set.to_string()))?;
 
         // Terminator
@@ -221,13 +221,10 @@ where MWF: MirWithFlowState<'tcx>,
         }
         write!(w, "</td>")?;
 
-        // Gen
-        let set = flow.sets.gen_set_for(i);
-        write!(w, "<td>{:?}</td>", dot::escape_html(&format!("{:?}", set)))?;
-
-        // Kill
-        let set = flow.sets.kill_set_for(i);
-        write!(w, "<td>{:?}</td>", dot::escape_html(&format!("{:?}", set)))?;
+        // Gen/Kill
+        let trans = flow.sets.trans_for(i);
+        write!(w, "<td>{:?}</td>", dot::escape_html(&format!("{:?}", trans.gen_set)))?;
+        write!(w, "<td>{:?}</td>", dot::escape_html(&format!("{:?}", trans.kill_set)))?;
 
         write!(w, "</tr>")?;
 

src/librustc_mir/dataflow/impls/borrowed_locals.rs

@@ -2,7 +2,7 @@ pub use super::*;
 
 use rustc::mir::*;
 use rustc::mir::visit::Visitor;
-use crate::dataflow::BitDenotation;
+use crate::dataflow::{BitDenotation, GenKillSet};
 
 /// This calculates if any part of a MIR local could have previously been borrowed.
 /// This means that once a local has been borrowed, its bit will be set
@@ -33,39 +33,39 @@ impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> {
         self.body.local_decls.len()
     }
 
-    fn start_block_effect(&self, _sets: &mut BitSet<Local>) {
+    fn start_block_effect(&self, _on_entry: &mut BitSet<Local>) {
         // Nothing is borrowed on function entry
     }
 
     fn statement_effect(&self,
-                        sets: &mut BlockSets<'_, Local>,
+                        trans: &mut GenKillSet<Local>,
                         loc: Location) {
         let stmt = &self.body[loc.block].statements[loc.statement_index];
 
         BorrowedLocalsVisitor {
-            sets,
+            trans,
         }.visit_statement(stmt, loc);
 
         // StorageDead invalidates all borrows and raw pointers to a local
         match stmt.kind {
-            StatementKind::StorageDead(l) => sets.kill(l),
+            StatementKind::StorageDead(l) => trans.kill(l),
             _ => (),
         }
     }
 
     fn terminator_effect(&self,
-                         sets: &mut BlockSets<'_, Local>,
+                         trans: &mut GenKillSet<Local>,
                          loc: Location) {
         let terminator = self.body[loc.block].terminator();
         BorrowedLocalsVisitor {
-            sets,
+            trans,
         }.visit_terminator(terminator, loc);
         match &terminator.kind {
             // Drop terminators borrows the location
             TerminatorKind::Drop { location, .. } |
             TerminatorKind::DropAndReplace { location, .. } => {
                 if let Some(local) = find_local(location) {
-                    sets.gen(local);
+                    trans.gen(local);
                 }
             }
             _ => (),
@@ -83,22 +83,13 @@ impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> BitSetOperator for HaveBeenBorrowedLocals<'a, 'tcx> {
-    #[inline]
-    fn join<T: Idx>(&self, inout_set: &mut BitSet<T>, in_set: &BitSet<T>) -> bool {
-        inout_set.union(in_set) // "maybe" means we union effects of both preds
-    }
-}
-
-impl<'a, 'tcx> InitialFlow for HaveBeenBorrowedLocals<'a, 'tcx> {
-    #[inline]
-    fn bottom_value() -> bool {
-        false // bottom = unborrowed
-    }
+impl<'a, 'tcx> BottomValue for HaveBeenBorrowedLocals<'a, 'tcx> {
+    // bottom = unborrowed
+    const BOTTOM_VALUE: bool = false;
 }
 
-struct BorrowedLocalsVisitor<'b, 'c> {
-    sets: &'b mut BlockSets<'c, Local>,
+struct BorrowedLocalsVisitor<'gk> {
+    trans: &'gk mut GenKillSet<Local>,
 }
 
 fn find_local<'tcx>(place: &Place<'tcx>) -> Option<Local> {
@@ -117,13 +108,13 @@ fn find_local<'tcx>(place: &Place<'tcx>) -> Option<Local> {
     })
 }
 
-impl<'tcx, 'b, 'c> Visitor<'tcx> for BorrowedLocalsVisitor<'b, 'c> {
+impl<'tcx> Visitor<'tcx> for BorrowedLocalsVisitor<'_> {
     fn visit_rvalue(&mut self,
                     rvalue: &Rvalue<'tcx>,
                     location: Location) {
         if let Rvalue::Ref(_, _, ref place) = *rvalue {
             if let Some(local) = find_local(place) {
-                self.sets.gen(local);
+                self.trans.gen(local);
             }
         }