Skip to content

Commit

Permalink
Rollup merge of #74169 - ecstatic-morse:dataflow-unreachable, r=pnkfelix
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Manishearth authored Jul 17, 2020
2 parents 9c84c6b + 698e870 commit 10b7eec
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 12 deletions.
17 changes: 17 additions & 0 deletions src/librustc_middle/mir/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions src/librustc_mir/dataflow/framework/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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;
Expand Down
18 changes: 9 additions & 9 deletions src/librustc_mir/dataflow/framework/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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];
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/dataflow/framework/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_mir/transform/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 10b7eec

Please sign in to comment.