From efe634f14c20661216c57322b319c1cac1f9704f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 8 Jul 2020 09:47:14 -0700 Subject: [PATCH 1/2] Add `reachable` and friends to `mir::traversal` module --- src/librustc_middle/mir/traversal.rs | 17 +++++++++++++++++ src/librustc_mir/dataflow/framework/engine.rs | 9 +++++++++ src/librustc_mir/transform/generator.rs | 4 +--- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/librustc_middle/mir/traversal.rs b/src/librustc_middle/mir/traversal.rs index ed8129b1e09a5..36e277d1a88f3 100644 --- a/src/librustc_middle/mir/traversal.rs +++ b/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)> { + preorder(body) +} + +/// Returns a `BitSet` containing all basic blocks reachable from the `START_BLOCK`. +pub fn reachable_as_bitset(body: &Body<'tcx>) -> BitSet { + let mut iter = preorder(body); + (&mut iter).for_each(drop); + iter.visited +} diff --git a/src/librustc_mir/dataflow/framework/engine.rs b/src/librustc_mir/dataflow/framework/engine.rs index 243b3679f2984..eb38811791168 100644 --- a/src/librustc_mir/dataflow/framework/engine.rs +++ b/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>, + ) { + 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>, diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 523d3c9af3f68..8618cc126c563 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/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; From 698e870f75710152309158342fd18461531cc0fb Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 8 Jul 2020 09:17:02 -0700 Subject: [PATCH 2/2] Stop adding unreachable basic blocks to dataflow work queue Also adds some debug assertions to prevent API consumers from visiting those basic blocks by accident. --- src/librustc_mir/dataflow/framework/cursor.rs | 9 +++++++++ src/librustc_mir/dataflow/framework/engine.rs | 9 --------- src/librustc_mir/dataflow/framework/visitor.rs | 6 ++++++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/dataflow/framework/cursor.rs b/src/librustc_mir/dataflow/framework/cursor.rs index 2ae353adfc7f3..4f5930dc3f5a2 100644 --- a/src/librustc_mir/dataflow/framework/cursor.rs +++ b/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, } 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 --git a/src/librustc_mir/dataflow/framework/engine.rs b/src/librustc_mir/dataflow/framework/engine.rs index eb38811791168..003c40f290b8d 100644 --- a/src/librustc_mir/dataflow/framework/engine.rs +++ b/src/librustc_mir/dataflow/framework/engine.rs @@ -213,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 --git a/src/librustc_mir/dataflow/framework/visitor.rs b/src/librustc_mir/dataflow/framework/visitor.rs index 0df9322e7fe08..257f3cb9a6dd0 100644 --- a/src/librustc_mir/dataflow/framework/visitor.rs +++ b/src/librustc_mir/dataflow/framework/visitor.rs @@ -16,7 +16,13 @@ pub fn visit_results( { 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); }