Skip to content

Commit

Permalink
Auto merge of #71006 - ecstatic-morse:dataflow-bidi, r=ecstatic-morse
Browse files Browse the repository at this point in the history
Use existing framework for backward dataflow analyses

This PR adds support for backward analyses to the dataflow framework and adds a new live variable analysis (based on the existing one in `librustc_mir/util/liveness.rs`). By adding these to the framework instead of having a separate API, all newly implemented backward dataflow analyses get cursors/visitors, `rustc_peek` tests, and graphviz visualizations for free. In the near-term, this makes it much easier to implement global dead-store elimination, and I believe that this will enable even more MIR optimizations in the future.

This PR makes many changes to the dataflow API, since some concepts and terminology only make sense in forward dataflow. Below is a list of the important changes.
- ~~`entry_set` -> `fixpoint` (the fixpoint for backward dataflow problems is after the block's terminator)~~
- `seek_{before,after}` -> `seek_{before,after}_primary_effect` (the unprefixed dataflow effect is now referred to as the "primary" effect instead of the "after" effect. The "before" effect remains the same, although I considered changing it to the "antecedent" effect. In both backward and forward dataflow, the "before" effect is applied prior to the "primary" effect. I feel very strongly that this is the correct choice, as it means consumers don't have to switch between `seek_before` and `seek_after` based on the direction of their analysis.
- `seek_after_assume_call_returns` is now gone. Users can use `ResultsCursor::apply_custom_effect` to emulate it.
- `visit_{statement,terminator}_exit` -> `visit_{statement,terminator}_after_primary_effect`
- `visit_{statement,terminator}` -> `visit_{statement,terminator}_before_primary_effect`

Implementing this also required refactoring the dataflow cursor implementation so it could work in both directions. This is a large percentage of the diff, since the cursor code is rather complex. The fact that the cursor is exhaustively tested in both directions should reassure whomever is unlucky enough to review this 🤣.

In order to avoid computing the reverse CFG for forward dataflow analyses, I've added some hacks to the existing `mir::BodyAndCache` interface. I've requested changes to this interface that would let me implement this more efficiently.

r? @eddyb (feel free to reassign)
cc @rust-lang/wg-mir-opt
  • Loading branch information
bors committed May 3, 2020
2 parents ea733c3 + 21c72b6 commit 65b4482
Show file tree
Hide file tree
Showing 25 changed files with 1,369 additions and 709 deletions.
3 changes: 2 additions & 1 deletion src/librustc_codegen_ssa/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>

fn visit_local(&mut self, &local: &mir::Local, context: PlaceContext, location: Location) {
match context {
PlaceContext::MutatingUse(MutatingUseContext::Call) => {
PlaceContext::MutatingUse(MutatingUseContext::Call)
| PlaceContext::MutatingUse(MutatingUseContext::Yield) => {
self.assign(local, location);
}

Expand Down
4 changes: 3 additions & 1 deletion src/librustc_middle/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ macro_rules! make_mir_visitor {
self.visit_operand(value, source_location);
self.visit_place(
resume_arg,
PlaceContext::MutatingUse(MutatingUseContext::Store),
PlaceContext::MutatingUse(MutatingUseContext::Yield),
source_location,
);
}
Expand Down Expand Up @@ -1052,6 +1052,8 @@ pub enum MutatingUseContext {
AsmOutput,
/// Destination of a call.
Call,
/// Destination of a yield.
Yield,
/// Being dropped.
Drop,
/// Mutable borrow.
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> {
impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> {
type FlowState = Flows<'cx, 'tcx>;

fn visit_statement(
fn visit_statement_before_primary_effect(
&mut self,
flow_state: &Flows<'cx, 'tcx>,
stmt: &'cx Statement<'tcx>,
Expand Down Expand Up @@ -607,7 +607,7 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc
}
}

fn visit_terminator(
fn visit_terminator_before_primary_effect(
&mut self,
flow_state: &Flows<'cx, 'tcx>,
term: &'cx Terminator<'tcx>,
Expand Down Expand Up @@ -701,7 +701,7 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc
}
}

fn visit_terminator_exit(
fn visit_terminator_after_primary_effect(
&mut self,
flow_state: &Flows<'cx, 'tcx>,
term: &'cx Terminator<'tcx>,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/type_check/liveness/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ impl LivenessContext<'_, '_, '_, 'tcx> {
/// DROP of some local variable will have an effect -- note that
/// drops, as they may unwind, are always terminators.
fn initialized_at_terminator(&mut self, block: BasicBlock, mpi: MovePathIndex) -> bool {
self.flow_inits.seek_before(self.body.terminator_loc(block));
self.flow_inits.seek_before_primary_effect(self.body.terminator_loc(block));
self.initialized_at_curr_loc(mpi)
}

Expand All @@ -418,7 +418,7 @@ impl LivenessContext<'_, '_, '_, 'tcx> {
/// **Warning:** Does not account for the result of `Call`
/// instructions.
fn initialized_at_exit(&mut self, block: BasicBlock, mpi: MovePathIndex) -> bool {
self.flow_inits.seek_after(self.body.terminator_loc(block));
self.flow_inits.seek_after_primary_effect(self.body.terminator_loc(block));
self.initialized_at_curr_loc(mpi)
}

Expand Down
Loading

0 comments on commit 65b4482

Please sign in to comment.