Rollup merge of #74169 - ecstatic-morse:dataflow-unreachable, r=pnkfelix

Stop processing unreachable blocks when solving dataflow

...instead we `debug_assert` that the user is not checking the dataflow state for an unreachable block. This resolves a FIXME in the dataflow engine. The old behavior was an artifact of the previous dataflow framework. Things should run a tiny bit faster now, but I suspect not enough to show up in benchmarks. AFAIK, only the generator transform runs dataflow on MIR with unreachable basic blocks.

This PR also adds some utility methods to `mir::traversal`.

r? @pnkfelix

Author: Manishearth

Diff for: src/librustc_middle/mir/traversal.rs

@@ -292,3 +292,20 @@ impl<'a, 'tcx> Iterator for ReversePostorder<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> ExactSizeIterator for ReversePostorder<'a, 'tcx> {}
+
+/// Returns an iterator over all basic blocks reachable from the `START_BLOCK` in no particular
+/// order.
+///
+/// This is clearer than writing `preorder` in cases where the order doesn't matter.
+pub fn reachable<'a, 'tcx>(
+    body: &'a Body<'tcx>,
+) -> impl 'a + Iterator<Item = (BasicBlock, &'a BasicBlockData<'tcx>)> {
+    preorder(body)
+}
+
+/// Returns a `BitSet` containing all basic blocks reachable from the `START_BLOCK`.
+pub fn reachable_as_bitset(body: &Body<'tcx>) -> BitSet<BasicBlock> {
+    let mut iter = preorder(body);
+    (&mut iter).for_each(drop);
+    iter.visited
+}

Diff for: src/librustc_mir/dataflow/framework/cursor.rs

@@ -34,6 +34,9 @@ where
     ///
     /// When this flag is set, we need to reset to an entry set before doing a seek.
     state_needs_reset: bool,
+
+    #[cfg(debug_assertions)]
+    reachable_blocks: BitSet<BasicBlock>,
 }
 
 impl<'mir, 'tcx, A, R> ResultsCursor<'mir, 'tcx, A, R>
@@ -55,6 +58,9 @@ where
             state_needs_reset: true,
             state: BitSet::new_empty(bits_per_block),
             pos: CursorPosition::block_entry(mir::START_BLOCK),
+
+            #[cfg(debug_assertions)]
+            reachable_blocks: mir::traversal::reachable_as_bitset(body),
         }
     }
 
@@ -85,6 +91,9 @@ where
     ///
     /// For backward dataflow analyses, this is the dataflow state after the terminator.
     pub(super) fn seek_to_block_entry(&mut self, block: BasicBlock) {
+        #[cfg(debug_assertions)]
+        assert!(self.reachable_blocks.contains(block));
+
         self.state.overwrite(&self.results.borrow().entry_set_for_block(block));
         self.pos = CursorPosition::block_entry(block);
         self.state_needs_reset = false;

Diff for: src/librustc_mir/dataflow/framework/engine.rs

@@ -52,6 +52,15 @@ where
         visit_results(body, blocks, self, vis)
     }
 
+    pub fn visit_reachable_with(
+        &self,
+        body: &'mir mir::Body<'tcx>,
+        vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = BitSet<A::Idx>>,
+    ) {
+        let blocks = mir::traversal::reachable(body);
+        visit_results(body, blocks.map(|(bb, _)| bb), self, vis)
+    }
+
     pub fn visit_in_rpo_with(
         &self,
         body: &'mir mir::Body<'tcx>,
@@ -204,15 +213,6 @@ where
             }
         }
 
-        // Add blocks that are not reachable from START_BLOCK to the work queue. These blocks will
-        // be processed after the ones added above.
-        //
-        // FIXME(ecstaticmorse): Is this actually necessary? In principle, we shouldn't need to
-        // know the dataflow state in unreachable basic blocks.
-        for bb in body.basic_blocks().indices() {
-            dirty_queue.insert(bb);
-        }
-
         let mut state = BitSet::new_empty(bits_per_block);
         while let Some(bb) = dirty_queue.pop() {
             let bb_data = &body[bb];

Diff for: src/librustc_mir/dataflow/framework/visitor.rs

@@ -16,7 +16,13 @@ pub fn visit_results<F, V>(
 {
     let mut state = results.new_flow_state(body);
 
+    #[cfg(debug_assertions)]
+    let reachable_blocks = mir::traversal::reachable_as_bitset(body);
+
     for block in blocks {
+        #[cfg(debug_assertions)]
+        assert!(reachable_blocks.contains(block));
+
         let block_data = &body[block];
         V::Direction::visit_results_in_block(&mut state, block, block_data, results, vis);
     }

Diff for: src/librustc_mir/transform/generator.rs

@@ -624,9 +624,7 @@ fn compute_storage_conflicts(
         local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len()),
     };
 
-    // Visit only reachable basic blocks. The exact order is not important.
-    let reachable_blocks = traversal::preorder(body).map(|(bb, _)| bb);
-    requires_storage.visit_with(body, reachable_blocks, &mut visitor);
+    requires_storage.visit_reachable_with(body, &mut visitor);
 
     let local_conflicts = visitor.local_conflicts;