From e7f3a8a8fe9a997f69bdb3a12514542fc49a191f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 11:48:47 -0800 Subject: [PATCH 01/12] Improve graphviz visualization for new framework --- src/librustc_mir/dataflow/generic/graphviz.rs | 553 ++++++++++++++---- src/libsyntax_pos/symbol.rs | 3 + 2 files changed, 438 insertions(+), 118 deletions(-) diff --git a/src/librustc_mir/dataflow/generic/graphviz.rs b/src/librustc_mir/dataflow/generic/graphviz.rs index 47ace8f33ecac..da64094a7bd3c 100644 --- a/src/librustc_mir/dataflow/generic/graphviz.rs +++ b/src/librustc_mir/dataflow/generic/graphviz.rs @@ -1,14 +1,15 @@ +//! A helpful diagram for debugging dataflow problems. + use std::cell::RefCell; -use std::io::{self, Write}; -use std::{ops, str}; +use std::{io, ops, str}; use rustc::hir::def_id::DefId; use rustc::mir::{self, BasicBlock, Body, Location}; use rustc_index::bit_set::{BitSet, HybridBitSet}; -use rustc_index::vec::Idx; +use rustc_index::vec::{Idx, IndexVec}; use crate::util::graphviz_safe_def_name; -use super::{Analysis, Results, ResultsRefCursor}; +use super::{Analysis, GenKillSet, Results, ResultsRefCursor}; pub struct Formatter<'a, 'tcx, A> where @@ -29,11 +30,12 @@ where body: &'a Body<'tcx>, def_id: DefId, results: &'a Results<'tcx, A>, + state_formatter: &'a mut dyn StateFormatter<'tcx, A>, ) -> Self { let block_formatter = BlockFormatter { bg: Background::Light, - prev_state: BitSet::new_empty(results.analysis.bits_per_block(body)), results: ResultsRefCursor::new(body, results), + state_formatter, }; Formatter { @@ -143,15 +145,21 @@ struct BlockFormatter<'a, 'tcx, A> where A: Analysis<'tcx>, { - prev_state: BitSet, results: ResultsRefCursor<'a, 'a, 'tcx, A>, bg: Background, + state_formatter: &'a mut dyn StateFormatter<'tcx, A>, } impl BlockFormatter<'a, 'tcx, A> where A: Analysis<'tcx>, { + const HEADER_COLOR: &'static str = "#a0a0a0"; + + fn num_state_columns(&self) -> usize { + std::cmp::max(1, self.state_formatter.column_names().len()) + } + fn toggle_background(&mut self) -> Background { let bg = self.bg; self.bg = !bg; @@ -190,199 +198,508 @@ where r#""#, )?; - // A: Block info - write!( - w, - r#" - - "#, - num_headers = 3, - block_id = block.index(), - )?; - - // B: Column headings - write!( - w, - r#" - - - "#, - fmt = r##"bgcolor="#a0a0a0" sides="tl""##, - )?; + // A + B: Block header + if self.state_formatter.column_names().is_empty() { + self.write_block_header_simple(w, block)?; + } else { + self.write_block_header_with_state_columns(w, block)?; + } // C: Entry state self.bg = Background::Light; self.results.seek_to_block_start(block); - self.write_row_with_curr_state(w, "", "(on entry)")?; - self.prev_state.overwrite(self.results.get()); + self.write_row_with_full_state(w, "", "(on_entry)")?; // D: Statement transfer functions for (i, statement) in body[block].statements.iter().enumerate() { let location = Location { block, statement_index: i }; - - let mir_col = format!("{:?}", statement); - let i_col = i.to_string(); - - self.results.seek_after(location); - self.write_row_with_curr_diff(w, &i_col, &mir_col)?; - self.prev_state.overwrite(self.results.get()); + let statement_str = format!("{:?}", statement); + self.write_row_for_location(w, &i.to_string(), &statement_str, location)?; } // E: Terminator transfer function let terminator = body[block].terminator(); - let location = body.terminator_loc(block); - - let mut mir_col = String::new(); - terminator.kind.fmt_head(&mut mir_col).unwrap(); + let terminator_loc = body.terminator_loc(block); + let mut terminator_str = String::new(); + terminator.kind.fmt_head(&mut terminator_str).unwrap(); - self.results.seek_after(location); - self.write_row_with_curr_diff(w, "T", &mir_col)?; - self.prev_state.overwrite(self.results.get()); + self.write_row_for_location(w, "T", &terminator_str, terminator_loc)?; // F: Exit state + self.results.seek_after(terminator_loc); if let mir::TerminatorKind::Call { destination: Some(_), .. } = &terminator.kind { - self.write_row_with_curr_state(w, "", "(on unwind)")?; - - self.results.seek_after_assume_call_returns(location); - self.write_row_with_curr_diff(w, "", "(on successful return)")?; + self.write_row_with_full_state(w, "", "(on unwind)")?; + + let num_state_columns = self.num_state_columns(); + self.write_row(w, "", "(on successful return)", |this, w, fmt| { + write!( + w, + r#"") + })?; } else { - self.write_row_with_curr_state(w, "", "(on exit)")?; + self.write_row_with_full_state(w, "", "(on exit)")?; } write!(w, "
bb{block_id}
MIRSTATE
"#, + colspan = num_state_columns, + fmt = fmt, + )?; + + let state_on_unwind = this.results.get().clone(); + this.results.seek_after_assume_call_returns(terminator_loc); + write_diff(w, this.results.analysis(), &state_on_unwind, this.results.get())?; + + write!(w, "
") } - fn write_row_with_curr_state( + fn write_block_header_simple( &mut self, w: &mut impl io::Write, - i: &str, - mir: &str, + block: BasicBlock, ) -> io::Result<()> { - let bg = self.toggle_background(); + // +-------------------------------------------------+ + // A | bb4 | + // +-----------------------------------+-------------+ + // B | MIR | STATE | + // +-+---------------------------------+-------------+ + // | | ... | | - let mut out = Vec::new(); - write!(&mut out, "{{")?; - pretty_print_state_elems(&mut out, self.results.analysis(), self.results.get().iter())?; - write!(&mut out, "}}")?; + // A + write!( + w, + concat!( + "", + r#"bb{block_id}"#, + "", + ), + block_id = block.index(), + )?; + // B write!( w, - r#" - {i} - {mir} - {state} - "#, - fmt = &["sides=\"tl\"", bg.attr()].join(" "), - i = i, - mir = dot::escape_html(mir), - state = dot::escape_html(str::from_utf8(&out).unwrap()), + concat!( + "", + r#"MIR"#, + r#"STATE"#, + "", + ), + fmt = format!("bgcolor=\"{}\" sides=\"tl\"", Self::HEADER_COLOR), ) } - fn write_row_with_curr_diff( + fn write_block_header_with_state_columns( &mut self, w: &mut impl io::Write, - i: &str, - mir: &str, + block: BasicBlock, ) -> io::Result<()> { - let bg = self.toggle_background(); - let analysis = self.results.analysis(); + // +------------------------------------+-------------+ + // A | bb4 | STATE | + // +------------------------------------+------+------+ + // B | MIR | GEN | KILL | + // +-+----------------------------------+------+------+ + // | | ... | | | + + let state_column_names = self.state_formatter.column_names(); + + // A + write!( + w, + concat!( + "", + r#"bb{block_id}"#, + r#"STATE"#, + "", + ), + fmt = "sides=\"tl\"", + num_state_cols = state_column_names.len(), + block_id = block.index(), + )?; - let diff = BitSetDiff::compute(&self.prev_state, self.results.get()); + // B + let fmt = format!("bgcolor=\"{}\" sides=\"tl\"", Self::HEADER_COLOR); + write!( + w, + concat!( + "", + r#"MIR"#, + ), + fmt = fmt, + )?; + + for name in state_column_names { + write!(w, "{name}", fmt = fmt, name = name)?; + } - let mut set = Vec::new(); - pretty_print_state_elems(&mut set, analysis, diff.set.iter())?; + write!(w, "") + } - let mut clear = Vec::new(); - pretty_print_state_elems(&mut clear, analysis, diff.clear.iter())?; + /// Write a row with the given index and MIR, using the function argument to fill in the + /// "STATE" column(s). + fn write_row( + &mut self, + w: &mut W, + i: &str, + mir: &str, + f: impl FnOnce(&mut Self, &mut W, &str) -> io::Result<()>, + ) -> io::Result<()> { + let bg = self.toggle_background(); + let fmt = format!("sides=\"tl\" {}", bg.attr()); write!( w, - r#" - {i} - {mir} - "#, + concat!( + "", + r#"{i}"#, + r#"{mir}"#, + ), i = i, - fmt = &["sides=\"tl\"", bg.attr()].join(" "), + fmt = fmt, mir = dot::escape_html(mir), )?; - if !set.is_empty() { + f(self, w, &fmt)?; + write!(w, "") + } + + fn write_row_with_full_state( + &mut self, + w: &mut impl io::Write, + i: &str, + mir: &str, + ) -> io::Result<()> { + self.write_row(w, i, mir, |this, w, fmt| { + let state = this.results.get(); + let analysis = this.results.analysis(); + write!( w, - r#"+{}"#, - dot::escape_html(str::from_utf8(&set).unwrap()), + r#"{{"#, + colspan = this.num_state_columns(), + fmt = fmt, )?; + pretty_print_state_elems(w, analysis, state.iter(), ",", LIMIT_40_ALIGN_1)?; + write!(w, "}}") + }) + } + + fn write_row_for_location( + &mut self, + w: &mut impl io::Write, + i: &str, + mir: &str, + location: Location, + ) -> io::Result<()> { + self.write_row(w, i, mir, |this, w, fmt| { + this.state_formatter.write_state_for_location(w, fmt, &mut this.results, location) + }) + } +} + +/// Controls what gets printed under the `STATE` header. +pub trait StateFormatter<'tcx, A> where A: Analysis<'tcx> { + /// The columns that will get printed under `STATE`. + fn column_names(&self) -> &[&str]; + + fn write_state_for_location( + &mut self, + w: &mut dyn io::Write, + fmt: &str, + results: &mut ResultsRefCursor<'_, '_, 'tcx, A>, + location: Location, + ) -> io::Result<()>; +} + +/// Prints a single column containing the state vector immediately *after* each statement. +pub struct SimpleDiff { + prev_state: BitSet, + prev_loc: Location, +} + +impl SimpleDiff { + #![allow(unused)] + pub fn new(bits_per_block: usize) -> Self { + SimpleDiff { + prev_state: BitSet::new_empty(bits_per_block), + prev_loc: Location::START, + } + } +} + +impl StateFormatter<'tcx, A> for SimpleDiff +where + A: Analysis<'tcx>, +{ + fn column_names(&self) -> &[&str] { + &[] + } + + fn write_state_for_location( + &mut self, + mut w: &mut dyn io::Write, + fmt: &str, + results: &mut ResultsRefCursor<'_, '_, 'tcx, A>, + location: Location, + ) -> io::Result<()> { + if location.statement_index == 0 { + results.seek_to_block_start(location.block); + self.prev_state.overwrite(results.get()); + } else { + // Ensure that we are visiting statements in order, so `prev_state` is correct. + assert_eq!(self.prev_loc.successor_within_block(), location); } - if !set.is_empty() && !clear.is_empty() { - write!(w, " ")?; + self.prev_loc = location; + write!(w, r#""#, fmt = fmt)?; + results.seek_before(location); + let curr_state = results.get(); + write_diff(&mut w, results.analysis(), &self.prev_state, curr_state)?; + self.prev_state.overwrite(curr_state); + write!(w, "") + } +} + +/// Prints two state columns, one containing only the "before" effect of each statement and one +/// containing the full effect. +pub struct TwoPhaseDiff { + prev_state: BitSet, + prev_loc: Location, +} + +impl TwoPhaseDiff { + #![allow(unused)] + pub fn new(bits_per_block: usize) -> Self { + TwoPhaseDiff { + prev_state: BitSet::new_empty(bits_per_block), + prev_loc: Location::START, } + } +} - if !clear.is_empty() { - write!( - w, - r#"-{}"#, - dot::escape_html(str::from_utf8(&clear).unwrap()), - )?; +impl StateFormatter<'tcx, A> for TwoPhaseDiff +where + A: Analysis<'tcx>, +{ + fn column_names(&self) -> &[&str] { + &["ENTRY", " EXIT"] + } + + fn write_state_for_location( + &mut self, + mut w: &mut dyn io::Write, + fmt: &str, + results: &mut ResultsRefCursor<'_, '_, 'tcx, A>, + location: Location, + ) -> io::Result<()> { + if location.statement_index == 0 { + results.seek_to_block_start(location.block); + self.prev_state.overwrite(results.get()); + } else { + // Ensure that we are visiting statements in order, so `prev_state` is correct. + assert_eq!(self.prev_loc.successor_within_block(), location); } - write!(w, "") + self.prev_loc = location; + + // Entry + + write!(w, r#""#, fmt = fmt)?; + results.seek_before(location); + let curr_state = results.get(); + write_diff(&mut w, results.analysis(), &self.prev_state, curr_state)?; + self.prev_state.overwrite(curr_state); + write!(w, "")?; + + // Exit + + write!(w, r#""#, fmt = fmt)?; + results.seek_after(location); + let curr_state = results.get(); + write_diff(&mut w, results.analysis(), &self.prev_state, curr_state)?; + self.prev_state.overwrite(curr_state); + write!(w, "") } } -/// The operations required to transform one `BitSet` into another. -struct BitSetDiff { - set: HybridBitSet, - clear: HybridBitSet, +/// Prints the gen/kill set for the entire block. +pub struct BlockTransferFunc<'a, 'tcx, T: Idx> { + body: &'a mir::Body<'tcx>, + trans_for_block: IndexVec>, } -impl BitSetDiff { - fn compute(from: &BitSet, to: &BitSet) -> Self { - assert_eq!(from.domain_size(), to.domain_size()); - let len = from.domain_size(); - - let mut set = HybridBitSet::new_empty(len); - let mut clear = HybridBitSet::new_empty(len); - - // FIXME: This could be made faster if `BitSet::xor` were implemented. - for i in (0..len).map(|i| T::new(i)) { - match (from.contains(i), to.contains(i)) { - (false, true) => set.insert(i), - (true, false) => clear.insert(i), - _ => continue, - }; +impl BlockTransferFunc<'mir, 'tcx, T> { + #![allow(unused)] + pub fn new( + body: &'mir mir::Body<'tcx>, + trans_for_block: IndexVec> + ) -> Self { + BlockTransferFunc { + body, + trans_for_block, + } + } +} + +impl StateFormatter<'tcx, A> for BlockTransferFunc<'mir, 'tcx, A::Idx> +where + A: Analysis<'tcx>, +{ + fn column_names(&self) -> &[&str] { + &["GEN", "KILL"] + } + + fn write_state_for_location( + &mut self, + mut w: &mut dyn io::Write, + fmt: &str, + results: &mut ResultsRefCursor<'_, '_, 'tcx, A>, + location: Location, + ) -> io::Result<()> { + // Only print a single row. + if location.statement_index != 0 { + return Ok(()); } - BitSetDiff { - set, - clear, + let block_trans = &self.trans_for_block[location.block]; + let rowspan = self.body.basic_blocks()[location.block].statements.len(); + + for set in &[&block_trans.gen, &block_trans.kill] { + write!( + w, + r#""#, + fmt = fmt, + rowspan = rowspan + )?; + + pretty_print_state_elems( + &mut w, + results.analysis(), + set.iter(), + "\n", + None, + )?; + write!(w, "")?; } + + Ok(()) } } -/// Formats each `elem` using the pretty printer provided by `analysis` into a comma-separated -/// list. +/// Writes two lines, one containing the added bits and one the removed bits. +fn write_diff>( + w: &mut impl io::Write, + analysis: &A, + from: &BitSet, + to: &BitSet, +) -> io::Result<()> { + assert_eq!(from.domain_size(), to.domain_size()); + let len = from.domain_size(); + + let mut set = HybridBitSet::new_empty(len); + let mut clear = HybridBitSet::new_empty(len); + + // FIXME: Implement a lazy iterator over the symmetric difference of two bitsets. + for i in (0..len).map(|i| A::Idx::new(i)) { + match (from.contains(i), to.contains(i)) { + (false, true) => set.insert(i), + (true, false) => clear.insert(i), + _ => continue, + }; + } + + if !set.is_empty() { + write!(w, r#"+"#)?; + pretty_print_state_elems( + w, + analysis, + set.iter(), + ",", + LIMIT_40_ALIGN_1, + )?; + write!(w, r#""#)?; + } + + if !set.is_empty() && !clear.is_empty() { + write!(w, "
")?; + } + + if !clear.is_empty() { + write!(w, r#"-"#)?; + pretty_print_state_elems( + w, + analysis, + clear.iter(), + ",", + LIMIT_40_ALIGN_1, + )?; + write!(w, r#""#)?; + } + + Ok(()) +} + +/// Line break policy that breaks at 40 characters and starts the next line with a single space. +const LIMIT_40_ALIGN_1: Option = Some(LineBreak { + sequence: "
", + limit: 40, +}); + +struct LineBreak { + sequence: &'static str, + limit: usize, +} + +/// Formats each `elem` using the pretty printer provided by `analysis` into a list with the given +/// separator (`sep`). +/// +/// Optionally, it will break lines using the given character sequence (usually `
`) and +/// character limit. fn pretty_print_state_elems( w: &mut impl io::Write, analysis: &A, elems: impl Iterator, -) -> io::Result<()> + sep: &str, + line_break: Option, +) -> io::Result where A: Analysis<'tcx>, { + let sep_width = sep.chars().count(); + + let mut buf = Vec::new(); + let mut first = true; + let mut curr_line_width = 0; + let mut line_break_inserted = false; + for idx in elems { if first { first = false; } else { - write!(w, ",")?; + write!(w, "{}", sep)?; + curr_line_width += sep_width; + } + + buf.clear(); + analysis.pretty_print_idx(&mut buf, idx)?; + let idx_str = str::from_utf8(&buf) + .expect("Output of `pretty_print_idx` must be valid UTF-8"); + let escaped = dot::escape_html(idx_str); + let escaped_width = escaped.chars().count(); + + if let Some(line_break) = &line_break { + if curr_line_width + sep_width + escaped_width > line_break.limit { + write!(w, "{}", line_break.sequence)?; + line_break_inserted = true; + curr_line_width = 0; + } } - analysis.pretty_print_idx(w, idx)?; + write!(w, "{}", escaped)?; + curr_line_width += escaped_width; } - Ok(()) + Ok(line_break_inserted) } /// The background color used for zebra-striping the table. diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index e8f7a125739ac..21434100f556e 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -167,6 +167,7 @@ symbols! { bind_by_move_pattern_guards, block, bool, + borrowck_graphviz_format, borrowck_graphviz_postflow, borrowck_graphviz_preflow, box_patterns, @@ -334,6 +335,7 @@ symbols! { FxHashSet, FxHashMap, gen_future, + gen_kill, generators, generic_associated_types, generic_param_attrs, @@ -728,6 +730,7 @@ symbols! { try_trait, tt, tuple_indexing, + two_phase, Ty, ty, type_alias_impl_trait, From 0ea06a7de9d67224d0f9ca7e9a5a3417c5f60d25 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 11:48:17 -0800 Subject: [PATCH 02/12] Implement new dataflow framework and cursor --- src/librustc_mir/dataflow/generic/cursor.rs | 267 ++++++++++++ src/librustc_mir/dataflow/generic/engine.rs | 436 ++++++++++++++++++++ src/librustc_mir/dataflow/generic/mod.rs | 305 ++++++++++++++ 3 files changed, 1008 insertions(+) create mode 100644 src/librustc_mir/dataflow/generic/cursor.rs create mode 100644 src/librustc_mir/dataflow/generic/engine.rs create mode 100644 src/librustc_mir/dataflow/generic/mod.rs diff --git a/src/librustc_mir/dataflow/generic/cursor.rs b/src/librustc_mir/dataflow/generic/cursor.rs new file mode 100644 index 0000000000000..b9be85ce74a7a --- /dev/null +++ b/src/librustc_mir/dataflow/generic/cursor.rs @@ -0,0 +1,267 @@ +//! Random access inspection of the results of a dataflow analysis. + +use std::borrow::Borrow; + +use rustc::mir::{self, BasicBlock, Location}; +use rustc_index::bit_set::BitSet; + +use super::{Analysis, Results}; + +/// A `ResultsCursor` that borrows the underlying `Results`. +pub type ResultsRefCursor<'a, 'mir, 'tcx, A> = + ResultsCursor<'mir, 'tcx, A, &'a Results<'tcx, A>>; + +/// Allows random access inspection of the results of a dataflow analysis. +/// +/// This cursor only has linear performance within a basic block when its statements are visited in +/// order. In the worst case—when statements are visited in *reverse* order—performance will be +/// quadratic in the number of statements in the block. The order in which basic blocks are +/// inspected has no impact on performance. +/// +/// A `ResultsCursor` can either own (the default) or borrow the dataflow results it inspects. The +/// type of ownership is determined by `R` (see `ResultsRefCursor` above). +pub struct ResultsCursor<'mir, 'tcx, A, R = Results<'tcx, A>> +where + A: Analysis<'tcx>, +{ + body: &'mir mir::Body<'tcx>, + results: R, + state: BitSet, + + pos: CursorPosition, + + /// When this flag is set, the cursor is pointing at a `Call` terminator whose call return + /// effect has been applied to `state`. + /// + /// This flag helps to ensure that multiple calls to `seek_after_assume_call_returns` with the + /// same target will result in exactly one invocation of `apply_call_return_effect`. It is + /// sufficient to clear this only in `seek_to_block_start`, since seeking away from a + /// terminator will always require a cursor reset. + call_return_effect_applied: bool, +} + +impl<'mir, 'tcx, A, R> ResultsCursor<'mir, 'tcx, A, R> +where + A: Analysis<'tcx>, + R: Borrow>, +{ + /// Returns a new cursor for `results` that points to the start of the `START_BLOCK`. + pub fn new(body: &'mir mir::Body<'tcx>, results: R) -> Self { + ResultsCursor { + body, + pos: CursorPosition::BlockStart(mir::START_BLOCK), + state: results.borrow().entry_sets[mir::START_BLOCK].clone(), + call_return_effect_applied: false, + results, + } + } + + /// Returns the `Analysis` used to generate the underlying results. + pub fn analysis(&self) -> &A { + &self.results.borrow().analysis + } + + /// Returns the dataflow state at the current location. + pub fn get(&self) -> &BitSet { + &self.state + } + + /// Resets the cursor to the start of the given basic block. + pub fn seek_to_block_start(&mut self, block: BasicBlock) { + self.state.overwrite(&self.results.borrow().entry_sets[block]); + self.pos = CursorPosition::BlockStart(block); + self.call_return_effect_applied = false; + } + + /// Advances the cursor to hold all effects up to and including to the "before" effect of the + /// statement (or terminator) at the given location. + /// + /// If you wish to observe the full effect of a statement or terminator, not just the "before" + /// effect, use `seek_after` or `seek_after_assume_call_returns`. + pub fn seek_before(&mut self, target: Location) { + assert!(target <= self.body.terminator_loc(target.block)); + self._seek(target, false); + } + + /// Advances the cursor to hold the full effect of all statements (and possibly closing + /// terminators) up to and including the `target`. + /// + /// If the `target` is a `Call` terminator, any call return effect for that terminator will + /// **not** be observed. Use `seek_after_assume_call_returns` if you wish to observe the call + /// return effect. + pub fn seek_after(&mut self, target: Location) { + assert!(target <= self.body.terminator_loc(target.block)); + + // If we have already applied the call return effect, we are currently pointing at a `Call` + // terminator. Unconditionally reset the dataflow cursor, since there is no way to "undo" + // the call return effect. + if self.call_return_effect_applied { + self.seek_to_block_start(target.block); + } + + self._seek(target, true); + } + + /// Advances the cursor to hold all effects up to and including of the statement (or + /// terminator) at the given location. + /// + /// If the `target` is a `Call` terminator, any call return effect for that terminator will + /// be observed. Use `seek_after` if you do **not** wish to observe the call return effect. + pub fn seek_after_assume_call_returns(&mut self, target: Location) { + let terminator_loc = self.body.terminator_loc(target.block); + assert!(target.statement_index <= terminator_loc.statement_index); + + self._seek(target, true); + + if target != terminator_loc { + return; + } + + let terminator = self.body.basic_blocks()[target.block].terminator(); + if let mir::TerminatorKind::Call { + destination: Some((return_place, _)), + func, + args, + .. + } = &terminator.kind { + if !self.call_return_effect_applied { + self.call_return_effect_applied = true; + self.results.borrow().analysis.apply_call_return_effect( + &mut self.state, + target.block, + func, + args, + return_place, + ); + } + } + } + + fn _seek(&mut self, target: Location, apply_after_effect_at_target: bool) { + use CursorPosition::*; + + match self.pos { + // Return early if we are already at the target location. + Before(curr) if curr == target && !apply_after_effect_at_target => return, + After(curr) if curr == target && apply_after_effect_at_target => return, + + // Otherwise, we must reset to the start of the target block if... + + // we are in a different block entirely. + | BlockStart(block) + | Before(Location { block, .. }) + | After(Location { block, .. }) + if block != target.block + => self.seek_to_block_start(target.block), + + // we are in the same block but have advanced past the target statement. + | Before(curr) + | After(curr) + if curr.statement_index > target.statement_index + => self.seek_to_block_start(target.block), + + // we have already applied the entire effect of a statement but only wish to observe + // its "before" effect. + | After(curr) + if curr.statement_index == target.statement_index && !apply_after_effect_at_target + => self.seek_to_block_start(target.block), + + // N.B., `call_return_effect_applied` is checked in `seek_after`, not here. + + _ => (), + } + + let analysis = &self.results.borrow().analysis; + let block_data = &self.body.basic_blocks()[target.block]; + + // At this point, the cursor is in the same block as the target location at an earlier + // statement. + debug_assert_eq!(target.block, self.pos.block()); + + // Find the first statement whose transfer function has not yet been applied. + let first_unapplied_statement = match self.pos { + BlockStart(_) => 0, + After(Location { statement_index, .. }) => statement_index + 1, + + // If we have only applied the "before" effect for the current statement, apply the + // remainder before continuing. + Before(curr) => { + if curr.statement_index == block_data.statements.len() { + let terminator = block_data.terminator(); + analysis.apply_terminator_effect(&mut self.state, terminator, curr); + } else { + let statement = &block_data.statements[curr.statement_index]; + analysis.apply_statement_effect(&mut self.state, statement, curr); + } + + // If all we needed to do was go from `Before` to `After` in the same statement, + // we are now done. + if curr.statement_index == target.statement_index { + debug_assert!(apply_after_effect_at_target); + self.pos = After(target); + return; + } + + curr.statement_index + 1 + } + }; + + // We have now applied all effects prior to `first_unapplied_statement`. + + // Apply the effects of all statements before `target`. + let mut location = Location { block: target.block, statement_index: 0 }; + for statement_index in first_unapplied_statement..target.statement_index { + location.statement_index = statement_index; + let statement = &block_data.statements[statement_index]; + analysis.apply_before_statement_effect(&mut self.state, statement, location); + analysis.apply_statement_effect(&mut self.state, statement, location); + } + + // Apply the effect of the statement (or terminator) at `target`. + location.statement_index = target.statement_index; + if target.statement_index == block_data.statements.len() { + let terminator = &block_data.terminator(); + analysis.apply_before_terminator_effect(&mut self.state, terminator, location); + + if apply_after_effect_at_target{ + analysis.apply_terminator_effect(&mut self.state, terminator, location); + self.pos = After(target); + } else { + self.pos = Before(target); + } + } else { + let statement = &block_data.statements[target.statement_index]; + analysis.apply_before_statement_effect(&mut self.state, statement, location); + + if apply_after_effect_at_target { + analysis.apply_statement_effect(&mut self.state, statement, location); + self.pos = After(target) + } else { + self.pos = Before(target); + } + } + } +} + +#[derive(Clone, Copy, Debug)] +enum CursorPosition { + /// No effects within this block have been applied. + BlockStart(BasicBlock), + + /// Only the "before" effect of the statement (or terminator) at this location has been + /// applied (along with the effects of all previous statements). + Before(Location), + + /// The effects of all statements up to and including the one at this location have been + /// applied. + After(Location), +} + +impl CursorPosition { + fn block(&self) -> BasicBlock { + match *self { + Self::BlockStart(block) => block, + Self::Before(loc) | Self::After(loc) => loc.block, + } + } +} diff --git a/src/librustc_mir/dataflow/generic/engine.rs b/src/librustc_mir/dataflow/generic/engine.rs new file mode 100644 index 0000000000000..ba182869b847e --- /dev/null +++ b/src/librustc_mir/dataflow/generic/engine.rs @@ -0,0 +1,436 @@ +//! A solver for dataflow problems. + +use std::ffi::OsString; +use std::path::PathBuf; +use std::fs; + +use rustc::hir::def_id::DefId; +use rustc::mir::{self, traversal, BasicBlock, Location}; +use rustc::ty::TyCtxt; +use rustc_data_structures::work_queue::WorkQueue; +use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; +use syntax::symbol::{sym, Symbol}; +use syntax::ast; + +use super::graphviz; +use super::{Analysis, GenKillAnalysis, GenKillSet, Results}; + +/// A solver for dataflow problems. +pub struct Engine<'a, 'tcx, A> +where + A: Analysis<'tcx>, +{ + bits_per_block: usize, + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + def_id: DefId, + dead_unwinds: &'a BitSet, + entry_sets: IndexVec>, + analysis: A, + + /// Cached, cumulative transfer functions for each block. + trans_for_block: Option>>, +} + +impl Engine<'a, 'tcx, A> +where + A: GenKillAnalysis<'tcx>, +{ + /// Creates a new `Engine` to solve a gen-kill dataflow problem. + pub fn new_gen_kill( + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + def_id: DefId, + dead_unwinds: &'a BitSet, + analysis: A, + ) -> Self { + let bits_per_block = analysis.bits_per_block(body); + let mut trans_for_block = + IndexVec::from_elem(GenKillSet::identity(bits_per_block), body.basic_blocks()); + + // Compute cumulative block transfer functions. + // + // FIXME: we may want to skip this if the MIR is acyclic, since we will never access a + // block transfer function more than once. + + for (block, block_data) in body.basic_blocks().iter_enumerated() { + let trans = &mut trans_for_block[block]; + + for (i, statement) in block_data.statements.iter().enumerate() { + let loc = Location { block, statement_index: i }; + analysis.before_statement_effect(trans, statement, loc); + analysis.statement_effect(trans, statement, loc); + } + + if let Some(terminator) = &block_data.terminator { + let loc = Location { block, statement_index: block_data.statements.len() }; + analysis.before_terminator_effect(trans, terminator, loc); + analysis.terminator_effect(trans, terminator, loc); + } + } + + Self::new(tcx, body, def_id, dead_unwinds, analysis, Some(trans_for_block)) + } +} + +impl Engine<'a, 'tcx, A> +where + A: Analysis<'tcx>, +{ + /// Creates a new `Engine` to solve a dataflow problem with an arbitrary transfer + /// function. + /// + /// Gen-kill problems should use `new_gen_kill`, which will coalesce transfer functions for + /// better performance. + pub fn new_generic( + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + def_id: DefId, + dead_unwinds: &'a BitSet, + analysis: A, + ) -> Self { + Self::new(tcx, body, def_id, dead_unwinds, analysis, None) + } + + fn new( + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + def_id: DefId, + dead_unwinds: &'a BitSet, + analysis: A, + trans_for_block: Option>>, + ) -> Self { + let bits_per_block = analysis.bits_per_block(body); + + let bottom_value_set = if A::BOTTOM_VALUE == true { + BitSet::new_filled(bits_per_block) + } else { + BitSet::new_empty(bits_per_block) + }; + + let mut entry_sets = IndexVec::from_elem(bottom_value_set, body.basic_blocks()); + analysis.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); + + Engine { + analysis, + bits_per_block, + tcx, + body, + def_id, + dead_unwinds, + entry_sets, + trans_for_block, + } + } + + pub fn iterate_to_fixpoint(mut self) -> Results<'tcx, A> { + let mut temp_state = BitSet::new_empty(self.bits_per_block); + + let mut dirty_queue: WorkQueue = + WorkQueue::with_none(self.body.basic_blocks().len()); + + for (bb, _) in traversal::reverse_postorder(self.body) { + dirty_queue.insert(bb); + } + + // Add blocks that are not reachable from START_BLOCK to the work queue. These blocks will + // be processed after the ones added above. + for bb in self.body.basic_blocks().indices() { + dirty_queue.insert(bb); + } + + while let Some(bb) = dirty_queue.pop() { + let bb_data = &self.body[bb]; + let on_entry = &self.entry_sets[bb]; + + temp_state.overwrite(on_entry); + self.apply_whole_block_effect(&mut temp_state, bb, bb_data); + + self.propagate_bits_into_graph_successors_of( + &mut temp_state, + (bb, bb_data), + &mut dirty_queue, + ); + } + + let Engine { tcx, body, def_id, trans_for_block, entry_sets, analysis, .. } = self; + let results = Results { analysis, entry_sets }; + + let res = write_graphviz_results(tcx, def_id, body, &results, trans_for_block); + if let Err(e) = res { + warn!("Failed to write graphviz dataflow results: {}", e); + } + + results + } + + /// Applies the cumulative effect of an entire block, excluding the call return effect if one + /// exists. + fn apply_whole_block_effect( + &self, + state: &mut BitSet, + block: BasicBlock, + block_data: &mir::BasicBlockData<'tcx>, + ) { + // Use the cached block transfer function if available. + if let Some(trans_for_block) = &self.trans_for_block { + trans_for_block[block].apply(state); + return; + } + + // Otherwise apply effects one-by-one. + + for (statement_index, statement) in block_data.statements.iter().enumerate() { + let location = Location { block, statement_index }; + self.analysis.apply_before_statement_effect(state, statement, location); + self.analysis.apply_statement_effect(state, statement, location); + } + + let terminator = block_data.terminator(); + let location = Location { block, statement_index: block_data.statements.len() }; + self.analysis.apply_before_terminator_effect(state, terminator, location); + self.analysis.apply_terminator_effect(state, terminator, location); + } + + fn propagate_bits_into_graph_successors_of( + &mut self, + in_out: &mut BitSet, + (bb, bb_data): (BasicBlock, &'a mir::BasicBlockData<'tcx>), + dirty_list: &mut WorkQueue, + ) { + use mir::TerminatorKind::*; + + // FIXME: This should be implemented using a `for_each_successor` method on + // `TerminatorKind`. + + match bb_data.terminator().kind { + | Return + | Resume + | Abort + | GeneratorDrop + | Unreachable + => {} + + | Goto { target } + | Assert { target, cleanup: None, .. } + | Yield { resume: target, drop: None, .. } + | Drop { target, location: _, unwind: None } + | DropAndReplace { target, value: _, location: _, unwind: None } + => self.propagate_bits_into_entry_set_for(in_out, target, dirty_list), + + Yield { resume: target, drop: Some(drop), .. } => { + self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); + self.propagate_bits_into_entry_set_for(in_out, drop, dirty_list); + } + + | Assert { target, cleanup: Some(unwind), .. } + | Drop { target, location: _, unwind: Some(unwind) } + | DropAndReplace { target, value: _, location: _, unwind: Some(unwind) } + => { + self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); + if !self.dead_unwinds.contains(bb) { + self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); + } + } + + SwitchInt { ref targets, .. } => { + for target in targets { + self.propagate_bits_into_entry_set_for(in_out, *target, dirty_list); + } + } + + Call { cleanup, ref destination, ref func, ref args, .. } => { + if let Some(unwind) = cleanup { + if !self.dead_unwinds.contains(bb) { + self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); + } + } + + if let Some((ref dest_place, dest_bb)) = *destination { + // N.B.: This must be done *last*, otherwise the unwind path will see the call + // return effect. + self.analysis.apply_call_return_effect(in_out, bb, func, args, dest_place); + self.propagate_bits_into_entry_set_for(in_out, dest_bb, dirty_list); + } + } + + FalseEdges { real_target, imaginary_target } => { + self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); + self.propagate_bits_into_entry_set_for(in_out, imaginary_target, dirty_list); + } + + FalseUnwind { real_target, unwind } => { + self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); + if let Some(unwind) = unwind { + if !self.dead_unwinds.contains(bb) { + self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); + } + } + } + } + } + + fn propagate_bits_into_entry_set_for( + &mut self, + in_out: &BitSet, + bb: BasicBlock, + dirty_queue: &mut WorkQueue, + ) { + let entry_set = &mut self.entry_sets[bb]; + let set_changed = self.analysis.join(entry_set, &in_out); + if set_changed { + dirty_queue.insert(bb); + } + } +} + +// Graphviz + +/// Writes a DOT file containing the results of a dataflow analysis if the user requested it via +/// `rustc_mir` attributes. +fn write_graphviz_results( + tcx: TyCtxt<'tcx>, + def_id: DefId, + body: &mir::Body<'tcx>, + results: &Results<'tcx, A>, + block_transfer_functions: Option>>, +) -> std::io::Result<()> +where + A: Analysis<'tcx>, +{ + let attrs = match RustcMirAttrs::parse(tcx, def_id) { + Ok(attrs) => attrs, + + // Invalid `rustc_mir` attrs will be reported using `span_err`. + Err(()) => return Ok(()), + }; + + let path = match attrs.output_path(A::NAME) { + Some(path) => path, + None => return Ok(()), + }; + + let bits_per_block = results.analysis.bits_per_block(body); + + let mut formatter: Box> = match attrs.formatter { + Some(sym::two_phase) => Box::new(graphviz::TwoPhaseDiff::new(bits_per_block)), + Some(sym::gen_kill) => { + if let Some(trans_for_block) = block_transfer_functions { + Box::new(graphviz::BlockTransferFunc::new(body, trans_for_block)) + } else { + Box::new(graphviz::SimpleDiff::new(bits_per_block)) + } + } + + // Default to the `SimpleDiff` output style. + _ => Box::new(graphviz::SimpleDiff::new(bits_per_block)), + }; + + debug!("printing dataflow results for {:?} to {}", def_id, path.display()); + let mut buf = Vec::new(); + + let graphviz = graphviz::Formatter::new(body, def_id, results, &mut *formatter); + dot::render(&graphviz, &mut buf)?; + fs::write(&path, buf)?; + Ok(()) +} + +#[derive(Default)] +struct RustcMirAttrs { + basename_and_suffix: Option, + formatter: Option, +} + +impl RustcMirAttrs { + fn parse( + tcx: TyCtxt<'tcx>, + def_id: DefId, + ) -> Result { + let attrs = tcx.get_attrs(def_id); + + let mut result = Ok(()); + let mut ret = RustcMirAttrs::default(); + + let rustc_mir_attrs = attrs + .into_iter() + .filter(|attr| attr.check_name(sym::rustc_mir)) + .flat_map(|attr| attr.meta_item_list().into_iter().flat_map(|v| v.into_iter())); + + for attr in rustc_mir_attrs { + let attr_result = if attr.check_name(sym::borrowck_graphviz_postflow) { + Self::set_field(&mut ret.basename_and_suffix, tcx, &attr, |s| { + let path = PathBuf::from(s.to_string()); + match path.file_name() { + Some(_) => Ok(path), + None => { + tcx.sess.span_err(attr.span(), "path must end in a filename"); + Err(()) + } + } + }) + } else if attr.check_name(sym::borrowck_graphviz_format) { + Self::set_field(&mut ret.formatter, tcx, &attr, |s| { + match s { + sym::gen_kill | sym::two_phase => Ok(s), + _ => { + tcx.sess.span_err(attr.span(), "unknown formatter"); + Err(()) + } + } + }) + } else { + Ok(()) + }; + + result = result.and(attr_result); + } + + result.map(|()| ret) + } + + fn set_field( + field: &mut Option, + tcx: TyCtxt<'tcx>, + attr: &ast::NestedMetaItem, + mapper: impl FnOnce(Symbol) -> Result, + ) -> Result<(), ()> { + if field.is_some() { + tcx.sess.span_err( + attr.span(), + &format!("duplicate values for `{}`", attr.name_or_empty()), + ); + + return Err(()); + } + + if let Some(s) = attr.value_str() { + *field = Some(mapper(s)?); + Ok(()) + } else { + tcx.sess.span_err( + attr.span(), + &format!("`{}` requires an argument", attr.name_or_empty()), + ); + Err(()) + } + } + + /// Returns the path where dataflow results should be written, or `None` + /// `borrowck_graphviz_postflow` was not specified. + /// + /// This performs the following transformation to the argument of `borrowck_graphviz_postflow`: + /// + /// "path/suffix.dot" -> "path/analysis_name_suffix.dot" + fn output_path(&self, analysis_name: &str) -> Option { + let mut ret = self.basename_and_suffix.as_ref().cloned()?; + let suffix = ret.file_name().unwrap(); // Checked when parsing attrs + + let mut file_name: OsString = analysis_name.into(); + file_name.push("_"); + file_name.push(suffix); + ret.set_file_name(file_name); + + Some(ret) + } +} diff --git a/src/librustc_mir/dataflow/generic/mod.rs b/src/librustc_mir/dataflow/generic/mod.rs new file mode 100644 index 0000000000000..6271c37de3558 --- /dev/null +++ b/src/librustc_mir/dataflow/generic/mod.rs @@ -0,0 +1,305 @@ +//! A framework for expressing dataflow problems. + +use std::io; + +use rustc::mir::{self, Location, BasicBlock}; +use rustc_index::bit_set::{BitSet, HybridBitSet}; +use rustc_index::vec::{Idx, IndexVec}; + +use crate::dataflow::BottomValue; + +mod cursor; +mod engine; +mod graphviz; + +pub use self::cursor::{ResultsCursor, ResultsRefCursor}; +pub use self::engine::Engine; + +/// A dataflow analysis that has converged to fixpoint. +pub struct Results<'tcx, A> +where + A: Analysis<'tcx>, +{ + pub analysis: A, + entry_sets: IndexVec>, +} + +impl Results<'tcx, A> +where + A: Analysis<'tcx>, +{ + pub fn into_cursor(self, body: &'mir mir::Body<'tcx>) -> ResultsCursor<'mir, 'tcx, A> { + ResultsCursor::new(body, self) + } + + pub fn on_block_entry(&self, block: BasicBlock) -> &BitSet { + &self.entry_sets[block] + } +} + +/// Define the domain of a dataflow problem. +/// +/// This trait specifies the lattice on which this analysis operates. For now, this must be a +/// powerset of values of type `Idx`. The elements of this lattice are represented with a `BitSet` +/// and referred to as the state vector. +/// +/// This trait also defines the initial value for the dataflow state upon entry to the +/// `START_BLOCK`, as well as some names used to refer to this analysis when debugging. +pub trait AnalysisDomain<'tcx>: BottomValue { + /// The type of the elements in the state vector. + type Idx: Idx; + + /// A descriptive name for this analysis. Used only for debugging. + /// + /// This name should be brief and contain no spaces, periods or other characters that are not + /// suitable as part of a filename. + const NAME: &'static str; + + /// The size of the state vector. + fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize; + + /// Mutates the entry set of the `START_BLOCK` to contain the initial state for dataflow + /// analysis. + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet); + + /// Prints an element in the state vector for debugging. + fn pretty_print_idx(&self, w: &mut impl io::Write, idx: Self::Idx) -> io::Result<()> { + write!(w, "{:?}", idx) + } +} + +/// Define a dataflow problem with an arbitrarily complex transfer function. +pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { + /// Updates the current dataflow state with the effect of evaluating a statement. + fn apply_statement_effect( + &self, + state: &mut BitSet, + statement: &mir::Statement<'tcx>, + location: Location, + ); + + /// Updates the current dataflow state with an effect that occurs immediately *before* the + /// given statement. + /// + /// This method is useful if the consumer of the results of this analysis needs only to observe + /// *part* of the effect of a statement (e.g. for two-phase borrows). As a general rule, + /// analyses should not implement this without implementing `apply_statement_effect`. + fn apply_before_statement_effect( + &self, + _state: &mut BitSet, + _statement: &mir::Statement<'tcx>, + _location: Location, + ) {} + + /// Updates the current dataflow state with the effect of evaluating a terminator. + /// + /// The effect of a successful return from a `Call` terminator should **not** be accounted for + /// in this function. That should go in `apply_call_return_effect`. For example, in the + /// `InitializedPlaces` analyses, the return place for a function call is not marked as + /// initialized here. + fn apply_terminator_effect( + &self, + state: &mut BitSet, + terminator: &mir::Terminator<'tcx>, + location: Location, + ); + + /// Updates the current dataflow state with an effect that occurs immediately *before* the + /// given terminator. + /// + /// This method is useful if the consumer of the results of this analysis needs only to observe + /// *part* of the effect of a terminator (e.g. for two-phase borrows). As a general rule, + /// analyses should not implement this without implementing `apply_terminator_effect`. + fn apply_before_terminator_effect( + &self, + _state: &mut BitSet, + _terminator: &mir::Terminator<'tcx>, + _location: Location, + ) {} + + /// Updates the current dataflow state with the effect of a successful return from a `Call` + /// terminator. + /// + /// This is separate from `apply_terminator_effect` to properly track state across unwind + /// edges. + fn apply_call_return_effect( + &self, + state: &mut BitSet, + block: BasicBlock, + func: &mir::Operand<'tcx>, + args: &[mir::Operand<'tcx>], + return_place: &mir::Place<'tcx>, + ); +} + +/// Define a gen/kill dataflow problem. +/// +/// Each method in this trait has a corresponding one in `Analysis`. However, these methods only +/// allow modification of the dataflow state via "gen" and "kill" operations. By defining transfer +/// functions for each statement in this way, the transfer function for an entire basic block can +/// be computed efficiently. +/// +/// `Analysis` is automatically implemented for all implementers of `GenKillAnalysis`. +pub trait GenKillAnalysis<'tcx>: Analysis<'tcx> { + /// See `Analysis::apply_statement_effect`. + fn statement_effect( + &self, + trans: &mut impl GenKill, + statement: &mir::Statement<'tcx>, + location: Location, + ); + + /// See `Analysis::apply_before_statement_effect`. + fn before_statement_effect( + &self, + _trans: &mut impl GenKill, + _statement: &mir::Statement<'tcx>, + _location: Location, + ) {} + + /// See `Analysis::apply_terminator_effect`. + fn terminator_effect( + &self, + trans: &mut impl GenKill, + terminator: &mir::Terminator<'tcx>, + location: Location, + ); + + /// See `Analysis::apply_before_terminator_effect`. + fn before_terminator_effect( + &self, + _trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + _location: Location, + ) {} + + /// See `Analysis::apply_call_return_effect`. + fn call_return_effect( + &self, + trans: &mut impl GenKill, + block: BasicBlock, + func: &mir::Operand<'tcx>, + args: &[mir::Operand<'tcx>], + return_place: &mir::Place<'tcx>, + ); +} + +impl Analysis<'tcx> for A +where + A: GenKillAnalysis<'tcx> +{ + fn apply_statement_effect( + &self, + state: &mut BitSet, + statement: &mir::Statement<'tcx>, + location: Location, + ) { + self.statement_effect(state, statement, location); + } + + fn apply_before_statement_effect( + &self, + state: &mut BitSet, + statement: &mir::Statement<'tcx>, + location: Location, + ) { + self.before_statement_effect(state, statement, location); + } + + fn apply_terminator_effect( + &self, + state: &mut BitSet, + terminator: &mir::Terminator<'tcx>, + location: Location, + ) { + self.terminator_effect(state, terminator, location); + } + + fn apply_before_terminator_effect( + &self, + state: &mut BitSet, + terminator: &mir::Terminator<'tcx>, + location: Location, + ) { + self.before_terminator_effect(state, terminator, location); + } + + fn apply_call_return_effect( + &self, + state: &mut BitSet, + block: BasicBlock, + func: &mir::Operand<'tcx>, + args: &[mir::Operand<'tcx>], + return_place: &mir::Place<'tcx>, + ) { + self.call_return_effect(state, block, func, args, return_place); + } +} + +/// The legal operations for a transfer function in a gen/kill problem. +pub trait GenKill: Sized { + /// Inserts `elem` into the `gen` set, removing it the `kill` set if present. + fn gen(&mut self, elem: T); + + /// Inserts `elem` into the `kill` set, removing it the `gen` set if present. + fn kill(&mut self, elem: T); + + /// Inserts the given elements into the `gen` set, removing them from the `kill` set if present. + fn gen_all(&mut self, elems: impl IntoIterator) { + for elem in elems { + self.gen(elem); + } + } + + /// Inserts the given elements into the `kill` set, removing them from the `gen` set if present. + fn kill_all(&mut self, elems: impl IntoIterator) { + for elem in elems { + self.kill(elem); + } + } +} + +/// Stores a transfer function for a gen/kill problem. +#[derive(Clone)] +pub struct GenKillSet { + gen: HybridBitSet, + kill: HybridBitSet, +} + +impl GenKillSet { + /// Creates a new transfer function that will leave the dataflow state unchanged. + pub fn identity(universe: usize) -> Self { + GenKillSet { + gen: HybridBitSet::new_empty(universe), + kill: HybridBitSet::new_empty(universe), + } + } + + /// Applies this transfer function to the given bitset. + pub fn apply(&self, state: &mut BitSet) { + state.union(&self.gen); + state.subtract(&self.kill); + } +} + +impl GenKill for GenKillSet { + fn gen(&mut self, elem: T) { + self.gen.insert(elem); + self.kill.remove(elem); + } + + fn kill(&mut self, elem: T) { + self.kill.insert(elem); + self.gen.remove(elem); + } +} + +impl GenKill for BitSet { + fn gen(&mut self, elem: T) { + self.insert(elem); + } + + fn kill(&mut self, elem: T) { + self.remove(elem); + } +} From 3da8b21110d9f18dfa4f4292f7708adb7f7e284a Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 11:49:08 -0800 Subject: [PATCH 03/12] Remove old "generic" framework --- src/librustc_mir/dataflow/generic.rs | 613 --------------------------- 1 file changed, 613 deletions(-) delete mode 100644 src/librustc_mir/dataflow/generic.rs diff --git a/src/librustc_mir/dataflow/generic.rs b/src/librustc_mir/dataflow/generic.rs deleted file mode 100644 index dd6238b80d174..0000000000000 --- a/src/librustc_mir/dataflow/generic.rs +++ /dev/null @@ -1,613 +0,0 @@ -//! Dataflow analysis with arbitrary transfer functions. -//! -//! This module is a work in progress. You should instead use `BitDenotation` in -//! `librustc_mir/dataflow/mod.rs` and encode your transfer function as a [gen/kill set][gk]. In -//! doing so, your analysis will run faster and you will be able to generate graphviz diagrams for -//! debugging with no extra effort. The interface in this module is intended only for dataflow -//! problems that cannot be expressed using gen/kill sets. -//! -//! FIXME(ecstaticmorse): In the long term, the plan is to preserve the existing `BitDenotation` -//! interface, but make `Engine` and `ResultsCursor` the canonical way to perform and inspect a -//! dataflow analysis. This requires porting the graphviz debugging logic to this module, deciding -//! on a way to handle the `before` methods in `BitDenotation` and creating an adapter so that -//! gen-kill problems can still be evaluated efficiently. See the discussion in [#64566][] for more -//! information. -//! -//! [gk]: https://en.wikipedia.org/wiki/Data-flow_analysis#Bit_vector_problems -//! [#64566]: https://github.com/rust-lang/rust/pull/64566 - -use std::borrow::Borrow; -use std::cmp::Ordering; -use std::ffi::OsString; -use std::path::{Path, PathBuf}; -use std::{fs, io, ops}; - -use rustc::hir::def_id::DefId; -use rustc::mir::{self, traversal, BasicBlock, Location}; -use rustc::ty::{self, TyCtxt}; -use rustc_data_structures::work_queue::WorkQueue; -use rustc_index::bit_set::BitSet; -use rustc_index::vec::{Idx, IndexVec}; -use syntax::symbol::sym; - -use crate::dataflow::BottomValue; - -mod graphviz; - -/// A specific kind of dataflow analysis. -/// -/// To run a dataflow analysis, one must set the initial state of the `START_BLOCK` via -/// `initialize_start_block` and define a transfer function for each statement or terminator via -/// the various `effect` methods. The entry set for all other basic blocks is initialized to -/// `Self::BOTTOM_VALUE`. The dataflow `Engine` then iteratively updates the various entry sets for -/// each block with the cumulative effects of the transfer functions of all preceding blocks. -/// -/// You should use an `Engine` to actually run an analysis, and a `ResultsCursor` to inspect the -/// results of that analysis like so: -/// -/// ```ignore(cross-crate-imports) -/// fn do_my_analysis(body: &mir::Body<'tcx>, dead_unwinds: &BitSet) { -/// // `MyAnalysis` implements `Analysis`. -/// let analysis = MyAnalysis::new(); -/// -/// let results = Engine::new(body, dead_unwinds, analysis).iterate_to_fixpoint(); -/// let mut cursor = ResultsCursor::new(body, results); -/// -/// for (_, statement_index) in body.block_data[START_BLOCK].statements.iter_enumerated() { -/// cursor.seek_after(Location { block: START_BLOCK, statement_index }); -/// let state = cursor.get(); -/// println!("{:?}", state); -/// } -/// } -/// ``` -pub trait Analysis<'tcx>: BottomValue { - /// The index type used to access the dataflow state. - type Idx: Idx; - - /// A name, used for debugging, that describes this dataflow analysis. - /// - /// The name should be suitable as part of a filename, so avoid whitespace, slashes or periods - /// and try to keep it short. - const NAME: &'static str; - - /// How each element of your dataflow state will be displayed during debugging. - /// - /// By default, this is the `fmt::Debug` representation of `Self::Idx`. - fn pretty_print_idx(&self, w: &mut impl io::Write, idx: Self::Idx) -> io::Result<()> { - write!(w, "{:?}", idx) - } - - /// The size of each bitvector allocated for each block. - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize; - - /// Mutates the entry set of the `START_BLOCK` to contain the initial state for dataflow - /// analysis. - fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet); - - /// Updates the current dataflow state with the effect of evaluating a statement. - fn apply_statement_effect( - &self, - state: &mut BitSet, - statement: &mir::Statement<'tcx>, - location: Location, - ); - - /// Updates the current dataflow state with the effect of evaluating a terminator. - /// - /// Note that the effect of a successful return from a `Call` terminator should **not** be - /// acounted for in this function. That should go in `apply_call_return_effect`. For example, - /// in the `InitializedPlaces` analyses, the return place is not marked as initialized here. - fn apply_terminator_effect( - &self, - state: &mut BitSet, - terminator: &mir::Terminator<'tcx>, - location: Location, - ); - - /// Updates the current dataflow state with the effect of a successful return from a `Call` - /// terminator. - /// - /// This is separated from `apply_terminator_effect` to properly track state across - /// unwind edges for `Call`s. - fn apply_call_return_effect( - &self, - state: &mut BitSet, - block: BasicBlock, - func: &mir::Operand<'tcx>, - args: &[mir::Operand<'tcx>], - return_place: &mir::Place<'tcx>, - ); - - /// Applies the cumulative effect of an entire basic block to the dataflow state (except for - /// `call_return_effect`, which is handled in the `Engine`). - /// - /// The default implementation calls `statement_effect` for every statement in the block before - /// finally calling `terminator_effect`. However, some dataflow analyses are able to coalesce - /// transfer functions for an entire block and apply them at once. Such analyses should - /// override `block_effect`. - fn apply_whole_block_effect( - &self, - state: &mut BitSet, - block: BasicBlock, - block_data: &mir::BasicBlockData<'tcx>, - ) { - for (statement_index, stmt) in block_data.statements.iter().enumerate() { - let location = Location { block, statement_index }; - self.apply_statement_effect(state, stmt, location); - } - - let location = Location { block, statement_index: block_data.statements.len() }; - self.apply_terminator_effect(state, block_data.terminator(), location); - } - - /// Applies the cumulative effect of a sequence of statements (and possibly a terminator) - /// within a single basic block. - /// - /// When called with `0..block_data.statements.len() + 1` as the statement range, this function - /// is equivalent to `apply_whole_block_effect`. - fn apply_partial_block_effect( - &self, - state: &mut BitSet, - block: BasicBlock, - block_data: &mir::BasicBlockData<'tcx>, - mut range: ops::Range, - ) { - if range.is_empty() { - return; - } - - // The final location might be a terminator, so iterate through all statements until the - // final one, then check to see whether the final one is a statement or terminator. - // - // This can't cause the range to wrap-around since we check that the range contains at - // least one element above. - range.end -= 1; - let final_location = Location { block, statement_index: range.end }; - - for statement_index in range { - let location = Location { block, statement_index }; - let stmt = &block_data.statements[statement_index]; - self.apply_statement_effect(state, stmt, location); - } - - if final_location.statement_index == block_data.statements.len() { - let terminator = block_data.terminator(); - self.apply_terminator_effect(state, terminator, final_location); - } else { - let stmt = &block_data.statements[final_location.statement_index]; - self.apply_statement_effect(state, stmt, final_location); - } - } -} - -#[derive(Clone, Copy, Debug)] -enum CursorPosition { - AtBlockStart(BasicBlock), - After(Location), -} - -impl CursorPosition { - fn block(&self) -> BasicBlock { - match *self { - Self::AtBlockStart(block) => block, - Self::After(Location { block, .. }) => block, - } - } -} - -type ResultsRefCursor<'a, 'mir, 'tcx, A> = - ResultsCursor<'mir, 'tcx, A, &'a Results<'tcx, A>>; - -/// Inspect the results of dataflow analysis. -/// -/// This cursor has linear performance when visiting statements in a block in order. Visiting -/// statements within a block in reverse order is `O(n^2)`, where `n` is the number of statements -/// in that block. -pub struct ResultsCursor<'mir, 'tcx, A, R = Results<'tcx, A>> -where - A: Analysis<'tcx>, -{ - body: &'mir mir::Body<'tcx>, - results: R, - state: BitSet, - - pos: CursorPosition, - - /// Whether the effects of `apply_call_return_effect` are currently stored in `state`. - /// - /// This flag ensures that multiple calls to `seek_after_assume_call_returns` with the same - /// target only result in one invocation of `apply_call_return_effect`. - is_call_return_effect_applied: bool, -} - -impl<'mir, 'tcx, A, R> ResultsCursor<'mir, 'tcx, A, R> -where - A: Analysis<'tcx>, - R: Borrow>, -{ - /// Returns a new cursor for `results` that points to the start of the `START_BLOCK`. - pub fn new(body: &'mir mir::Body<'tcx>, results: R) -> Self { - ResultsCursor { - body, - pos: CursorPosition::AtBlockStart(mir::START_BLOCK), - is_call_return_effect_applied: false, - state: results.borrow().entry_sets[mir::START_BLOCK].clone(), - results, - } - } - - pub fn analysis(&self) -> &A { - &self.results.borrow().analysis - } - - /// Resets the cursor to the start of the given `block`. - pub fn seek_to_block_start(&mut self, block: BasicBlock) { - self.state.overwrite(&self.results.borrow().entry_sets[block]); - self.pos = CursorPosition::AtBlockStart(block); - self.is_call_return_effect_applied = false; - } - - /// Updates the cursor to hold the dataflow state immediately before `target`. - pub fn seek_before(&mut self, target: Location) { - assert!(target <= self.body.terminator_loc(target.block)); - - if target.statement_index == 0 { - self.seek_to_block_start(target.block); - } else { - self._seek_after(Location { - block: target.block, - statement_index: target.statement_index - 1, - }); - } - } - - /// Updates the cursor to hold the dataflow state at `target`. - /// - /// If `target` is a `Call` terminator, `apply_call_return_effect` will not be called. See - /// `seek_after_assume_call_returns` if you wish to observe the dataflow state upon a - /// successful return. - pub fn seek_after(&mut self, target: Location) { - assert!(target <= self.body.terminator_loc(target.block)); - - // This check ensures the correctness of a call to `seek_after_assume_call_returns` - // followed by one to `seek_after` with the same target. - if self.is_call_return_effect_applied { - self.seek_to_block_start(target.block); - } - - self._seek_after(target); - } - - /// Equivalent to `seek_after`, but also calls `apply_call_return_effect` if `target` is a - /// `Call` terminator whose callee is convergent. - pub fn seek_after_assume_call_returns(&mut self, target: Location) { - assert!(target <= self.body.terminator_loc(target.block)); - - self._seek_after(target); - - if target != self.body.terminator_loc(target.block) { - return; - } - - let term = self.body.basic_blocks()[target.block].terminator(); - if let mir::TerminatorKind::Call { - destination: Some((return_place, _)), - func, - args, - .. - } = &term.kind { - if !self.is_call_return_effect_applied { - self.is_call_return_effect_applied = true; - self.results.borrow().analysis.apply_call_return_effect( - &mut self.state, - target.block, - func, - args, - return_place, - ); - } - } - } - - fn _seek_after(&mut self, target: Location) { - let Location { block: target_block, statement_index: target_index } = target; - - if self.pos.block() != target_block { - self.seek_to_block_start(target_block); - } - - // If we're in the same block but after the target statement, we need to reset to the start - // of the block. - if let CursorPosition::After(Location { statement_index: curr_index, .. }) = self.pos { - match curr_index.cmp(&target_index) { - Ordering::Equal => return, - Ordering::Less => {}, - Ordering::Greater => self.seek_to_block_start(target_block), - } - } - - // The cursor is now in the same block as the target location pointing at an earlier - // statement. - debug_assert_eq!(self.pos.block(), target_block); - if let CursorPosition::After(Location { statement_index, .. }) = self.pos { - debug_assert!(statement_index < target_index); - } - - let first_unapplied_statement = match self.pos { - CursorPosition::AtBlockStart(_) => 0, - CursorPosition::After(Location { statement_index, .. }) => statement_index + 1, - }; - - let block_data = &self.body.basic_blocks()[target_block]; - self.results.borrow().analysis.apply_partial_block_effect( - &mut self.state, - target_block, - block_data, - first_unapplied_statement..target_index + 1, - ); - - self.pos = CursorPosition::After(target); - self.is_call_return_effect_applied = false; - } - - /// Gets the dataflow state at the current location. - pub fn get(&self) -> &BitSet { - &self.state - } -} - -/// A completed dataflow analysis. -pub struct Results<'tcx, A> -where - A: Analysis<'tcx>, -{ - analysis: A, - entry_sets: IndexVec>, -} - -/// All information required to iterate a dataflow analysis to fixpoint. -pub struct Engine<'a, 'tcx, A> -where - A: Analysis<'tcx>, -{ - analysis: A, - bits_per_block: usize, - tcx: TyCtxt<'tcx>, - body: &'a mir::Body<'tcx>, - def_id: DefId, - dead_unwinds: &'a BitSet, - entry_sets: IndexVec>, -} - -impl Engine<'a, 'tcx, A> -where - A: Analysis<'tcx>, -{ - pub fn new( - tcx: TyCtxt<'tcx>, - body: &'a mir::Body<'tcx>, - def_id: DefId, - dead_unwinds: &'a BitSet, - analysis: A, - ) -> Self { - let bits_per_block = analysis.bits_per_block(body); - - let bottom_value_set = if A::BOTTOM_VALUE == true { - BitSet::new_filled(bits_per_block) - } else { - BitSet::new_empty(bits_per_block) - }; - - let mut entry_sets = IndexVec::from_elem(bottom_value_set, body.basic_blocks()); - analysis.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); - - Engine { - analysis, - bits_per_block, - tcx, - body, - def_id, - dead_unwinds, - entry_sets, - } - } - - pub fn iterate_to_fixpoint(mut self) -> Results<'tcx, A> { - let mut temp_state = BitSet::new_empty(self.bits_per_block); - - let mut dirty_queue: WorkQueue = - WorkQueue::with_none(self.body.basic_blocks().len()); - - for (bb, _) in traversal::reverse_postorder(self.body) { - dirty_queue.insert(bb); - } - - // Add blocks that are not reachable from START_BLOCK to the work queue. These blocks will - // be processed after the ones added above. - for bb in self.body.basic_blocks().indices() { - dirty_queue.insert(bb); - } - - while let Some(bb) = dirty_queue.pop() { - let bb_data = &self.body[bb]; - let on_entry = &self.entry_sets[bb]; - - temp_state.overwrite(on_entry); - self.analysis.apply_whole_block_effect(&mut temp_state, bb, bb_data); - - self.propagate_bits_into_graph_successors_of( - &mut temp_state, - (bb, bb_data), - &mut dirty_queue, - ); - } - - let Engine { - tcx, - body, - def_id, - analysis, - entry_sets, - .. - } = self; - - let results = Results { analysis, entry_sets }; - - let attrs = tcx.get_attrs(def_id); - if let Some(path) = get_dataflow_graphviz_output_path(tcx, attrs, A::NAME) { - let result = write_dataflow_graphviz_results(body, def_id, &path, &results); - if let Err(e) = result { - warn!("Failed to write dataflow results to {}: {}", path.display(), e); - } - } - - results - } - - fn propagate_bits_into_graph_successors_of( - &mut self, - in_out: &mut BitSet, - (bb, bb_data): (BasicBlock, &'a mir::BasicBlockData<'tcx>), - dirty_list: &mut WorkQueue, - ) { - match bb_data.terminator().kind { - mir::TerminatorKind::Return - | mir::TerminatorKind::Resume - | mir::TerminatorKind::Abort - | mir::TerminatorKind::GeneratorDrop - | mir::TerminatorKind::Unreachable => {} - - mir::TerminatorKind::Goto { target } - | mir::TerminatorKind::Assert { target, cleanup: None, .. } - | mir::TerminatorKind::Yield { resume: target, drop: None, .. } - | mir::TerminatorKind::Drop { target, location: _, unwind: None } - | mir::TerminatorKind::DropAndReplace { target, value: _, location: _, unwind: None } => - { - self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); - } - - mir::TerminatorKind::Yield { resume: target, drop: Some(drop), .. } => { - self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); - self.propagate_bits_into_entry_set_for(in_out, drop, dirty_list); - } - - mir::TerminatorKind::Assert { target, cleanup: Some(unwind), .. } - | mir::TerminatorKind::Drop { target, location: _, unwind: Some(unwind) } - | mir::TerminatorKind::DropAndReplace { - target, - value: _, - location: _, - unwind: Some(unwind), - } => { - self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); - if !self.dead_unwinds.contains(bb) { - self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); - } - } - - mir::TerminatorKind::SwitchInt { ref targets, .. } => { - for target in targets { - self.propagate_bits_into_entry_set_for(in_out, *target, dirty_list); - } - } - - mir::TerminatorKind::Call { cleanup, ref destination, ref func, ref args, .. } => { - if let Some(unwind) = cleanup { - if !self.dead_unwinds.contains(bb) { - self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); - } - } - - if let Some((ref dest_place, dest_bb)) = *destination { - // N.B.: This must be done *last*, after all other - // propagation, as documented in comment above. - self.analysis.apply_call_return_effect(in_out, bb, func, args, dest_place); - self.propagate_bits_into_entry_set_for(in_out, dest_bb, dirty_list); - } - } - - mir::TerminatorKind::FalseEdges { real_target, imaginary_target } => { - self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); - self.propagate_bits_into_entry_set_for(in_out, imaginary_target, dirty_list); - } - - mir::TerminatorKind::FalseUnwind { real_target, unwind } => { - self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); - if let Some(unwind) = unwind { - if !self.dead_unwinds.contains(bb) { - self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); - } - } - } - } - } - - fn propagate_bits_into_entry_set_for( - &mut self, - in_out: &BitSet, - bb: BasicBlock, - dirty_queue: &mut WorkQueue, - ) { - let entry_set = &mut self.entry_sets[bb]; - let set_changed = self.analysis.join(entry_set, &in_out); - if set_changed { - dirty_queue.insert(bb); - } - } -} - -/// Looks for attributes like `#[rustc_mir(borrowck_graphviz_postflow="./path/to/suffix.dot")]` and -/// extracts the path with the given analysis name prepended to the suffix. -/// -/// Returns `None` if no such attribute exists. -fn get_dataflow_graphviz_output_path( - tcx: TyCtxt<'tcx>, - attrs: ty::Attributes<'tcx>, - analysis: &str, -) -> Option { - let mut rustc_mir_attrs = attrs - .into_iter() - .filter(|attr| attr.check_name(sym::rustc_mir)) - .flat_map(|attr| attr.meta_item_list().into_iter().flat_map(|v| v.into_iter())); - - let borrowck_graphviz_postflow = rustc_mir_attrs - .find(|attr| attr.check_name(sym::borrowck_graphviz_postflow))?; - - let path_and_suffix = match borrowck_graphviz_postflow.value_str() { - Some(p) => p, - None => { - tcx.sess.span_err( - borrowck_graphviz_postflow.span(), - "borrowck_graphviz_postflow requires a path", - ); - - return None; - } - }; - - // Change "path/suffix.dot" to "path/analysis_name_suffix.dot" - let mut ret = PathBuf::from(path_and_suffix.to_string()); - let suffix = ret.file_name().unwrap(); - - let mut file_name: OsString = analysis.into(); - file_name.push("_"); - file_name.push(suffix); - ret.set_file_name(file_name); - - Some(ret) -} - -fn write_dataflow_graphviz_results>( - body: &mir::Body<'tcx>, - def_id: DefId, - path: &Path, - results: &Results<'tcx, A> -) -> io::Result<()> { - debug!("printing dataflow results for {:?} to {}", def_id, path.display()); - - let mut buf = Vec::new(); - let graphviz = graphviz::Formatter::new(body, def_id, results); - - dot::render(&graphviz, &mut buf)?; - fs::write(path, buf) -} From 32c3590e579df35b5ca7010ea05d312b015dfc9c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 11:50:27 -0800 Subject: [PATCH 04/12] Use unified dataflow framework in `check_consts` --- src/librustc_mir/transform/check_consts/resolver.rs | 7 ++++++- src/librustc_mir/transform/check_consts/validation.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs index cb542484be633..ced5a1744bd6e 100644 --- a/src/librustc_mir/transform/check_consts/resolver.rs +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -173,7 +173,7 @@ impl old_dataflow::BottomValue for FlowSensitiveAnalysis<'_, '_, '_, Q> { const BOTTOM_VALUE: bool = false; } -impl dataflow::Analysis<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q> +impl dataflow::AnalysisDomain<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q> where Q: Qualif, { @@ -188,7 +188,12 @@ where fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut BitSet) { self.transfer_function(state).initialize_state(); } +} +impl dataflow::Analysis<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q> +where + Q: Qualif, +{ fn apply_statement_effect( &self, state: &mut BitSet, diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 6261315c711cd..90ae479afe285 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -38,7 +38,7 @@ impl QualifCursor<'a, 'mir, 'tcx, Q> { ) -> Self { let analysis = FlowSensitiveAnalysis::new(q, item); let results = - dataflow::Engine::new(item.tcx, &item.body, item.def_id, dead_unwinds, analysis) + dataflow::Engine::new_generic(item.tcx, &item.body, item.def_id, dead_unwinds, analysis) .iterate_to_fixpoint(); let cursor = dataflow::ResultsCursor::new(*item.body, results); From 7c7c9b3adf7fa6c4c49ebc35a6738c9de480390c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 17:53:20 -0800 Subject: [PATCH 05/12] Make `rustc_peek` use the unified dataflow framework --- src/librustc_mir/transform/rustc_peek.rs | 31 ++++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 2a81e97b8ff25..034167b5aed6b 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -9,11 +9,9 @@ use rustc::mir::{self, Body, BodyAndCache, Location, Local}; use rustc_index::bit_set::BitSet; use crate::transform::{MirPass, MirSource}; +use crate::dataflow::generic::{Analysis, Results, ResultsCursor}; use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::MoveDataParamEnv; -use crate::dataflow::BitDenotation; -use crate::dataflow::DataflowResults; -use crate::dataflow::DataflowResultsCursor; use crate::dataflow::{ DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces }; @@ -21,12 +19,14 @@ use crate::dataflow::IndirectlyMutableLocals; use crate::dataflow::move_paths::{MovePathIndex, LookupResult}; use crate::dataflow::move_paths::{HasMoveData, MoveData}; -use crate::dataflow::has_rustc_mir_with; pub struct SanityCheck; impl<'tcx> MirPass<'tcx> for SanityCheck { + #[allow(unused)] fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>) { + use crate::dataflow::has_rustc_mir_with; + let def_id = src.def_id(); if !tcx.has_attr(def_id, sym::rustc_mir) { debug!("skipping rustc_peek::SanityCheck on {}", tcx.def_path_str(def_id)); @@ -57,6 +57,8 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { IndirectlyMutableLocals::new(tcx, body, param_env), |_, i| DebugFormatted::new(&i)); + // FIXME: add these back as dataflow analyses are migrated + /* if has_rustc_mir_with(&attributes, sym::rustc_peek_maybe_init).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_inits); } @@ -82,6 +84,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(&attributes, sym::stop_after_dataflow).is_some() { tcx.sess.fatal("stop_after_dataflow ended compilation"); } + */ } } @@ -101,16 +104,16 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { /// (If there are any calls to `rustc_peek` that do not match the /// expression form above, then that emits an error as well, but those /// errors are not intended to be used for unit tests.) -pub fn sanity_check_via_rustc_peek<'tcx, O>( +pub fn sanity_check_via_rustc_peek<'tcx, A>( tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, _attributes: &[ast::Attribute], - results: &DataflowResults<'tcx, O>, -) where O: RustcPeekAt<'tcx> { + results: &Results<'tcx, A>, +) where A: RustcPeekAt<'tcx> { debug!("sanity_check_via_rustc_peek def_id: {:?}", def_id); - let mut cursor = DataflowResultsCursor::new(results, body); + let mut cursor = ResultsCursor::new(body, results); let peek_calls = body .basic_blocks() @@ -144,9 +147,9 @@ pub fn sanity_check_via_rustc_peek<'tcx, O>( | (PeekCallKind::ByVal, mir::Rvalue::Use(mir::Operand::Copy(place))) => { let loc = Location { block: bb, statement_index }; - cursor.seek(loc); + cursor.seek_before(loc); let state = cursor.get(); - results.operator().peek_at(tcx, place, state, call); + results.analysis.peek_at(tcx, place, state, call); } _ => { @@ -250,7 +253,7 @@ impl PeekCall { } } -pub trait RustcPeekAt<'tcx>: BitDenotation<'tcx> { +pub trait RustcPeekAt<'tcx>: Analysis<'tcx> { fn peek_at( &self, tcx: TyCtxt<'tcx>, @@ -260,8 +263,8 @@ pub trait RustcPeekAt<'tcx>: BitDenotation<'tcx> { ); } -impl<'tcx, O> RustcPeekAt<'tcx> for O - where O: BitDenotation<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, +impl<'tcx, A> RustcPeekAt<'tcx> for A + where A: Analysis<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, { fn peek_at( &self, @@ -287,6 +290,7 @@ impl<'tcx, O> RustcPeekAt<'tcx> for O } } +/* FIXME: Add this back once `IndirectlyMutableLocals` uses the new dataflow framework. impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> { fn peek_at( &self, @@ -308,3 +312,4 @@ impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> { } } } +*/ From a12fe27aa7fbe1656987b44b7f15b7317dd17e79 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 17:56:01 -0800 Subject: [PATCH 06/12] Ignore `rustc_peek` tests until migration to new dataflow framework --- src/test/ui/mir-dataflow/def-inits-1.rs | 2 ++ src/test/ui/mir-dataflow/indirect-mutation-offset.rs | 2 ++ src/test/ui/mir-dataflow/inits-1.rs | 2 ++ src/test/ui/mir-dataflow/uninits-1.rs | 2 ++ src/test/ui/mir-dataflow/uninits-2.rs | 2 ++ 5 files changed, 10 insertions(+) diff --git a/src/test/ui/mir-dataflow/def-inits-1.rs b/src/test/ui/mir-dataflow/def-inits-1.rs index 91d41e9b5794e..e30d38a995f68 100644 --- a/src/test/ui/mir-dataflow/def-inits-1.rs +++ b/src/test/ui/mir-dataflow/def-inits-1.rs @@ -1,5 +1,7 @@ // General test of maybe_uninits state computed by MIR dataflow. +// ignore-test Temporarily ignored while this analysis is migrated to the new framework. + #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs index 8087a3e139915..884c83b66163c 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs @@ -1,5 +1,7 @@ // compile-flags: -Zunleash-the-miri-inside-of-you +// ignore-test Temporarily ignored while this analysis is migrated to the new framework. + #![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] use std::cell::UnsafeCell; diff --git a/src/test/ui/mir-dataflow/inits-1.rs b/src/test/ui/mir-dataflow/inits-1.rs index 4a4786a2a7378..0e1839495bd39 100644 --- a/src/test/ui/mir-dataflow/inits-1.rs +++ b/src/test/ui/mir-dataflow/inits-1.rs @@ -1,5 +1,7 @@ // General test of maybe_inits state computed by MIR dataflow. +// ignore-test Temporarily ignored while this analysis is migrated to the new framework. + #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; diff --git a/src/test/ui/mir-dataflow/uninits-1.rs b/src/test/ui/mir-dataflow/uninits-1.rs index 66b3f458a5159..57589f4681ec3 100644 --- a/src/test/ui/mir-dataflow/uninits-1.rs +++ b/src/test/ui/mir-dataflow/uninits-1.rs @@ -1,5 +1,7 @@ // General test of maybe_uninits state computed by MIR dataflow. +// ignore-test Temporarily ignored while this analysis is migrated to the new framework. + #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; diff --git a/src/test/ui/mir-dataflow/uninits-2.rs b/src/test/ui/mir-dataflow/uninits-2.rs index 2ccf1c7f9d6c6..b62078745eec5 100644 --- a/src/test/ui/mir-dataflow/uninits-2.rs +++ b/src/test/ui/mir-dataflow/uninits-2.rs @@ -1,5 +1,7 @@ // General test of maybe_uninits state computed by MIR dataflow. +// ignore-test Temporarily ignored while this analysis is migrated to the new framework. + #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; From b5e1b002360ad08750c416a9a0c5e18a7be91631 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 17:27:18 -0800 Subject: [PATCH 07/12] Add `find_child` method to `MovePath` --- src/librustc_mir/dataflow/move_paths/mod.rs | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index b599f4799446d..e3361015a84b9 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -72,6 +72,40 @@ impl<'tcx> MovePath<'tcx> { parents } + + /// Looks for the first child of `self` for which `f` returns `true`. + /// + /// `f` will **not** be called on `self`. + pub fn find_child( + &self, + move_paths: &IndexVec>, + f: impl Fn(MovePathIndex) -> bool + ) -> Option { + let mut todo = if let Some(child) = self.first_child { + vec![child] + } else { + return None; + }; + + while let Some(mpi) = todo.pop() { + if f(mpi) { + return Some(mpi); + } + + let move_path = &move_paths[mpi]; + if let Some(child) = move_path.first_child { + todo.push(child); + } + + // After we've processed the original `mpi`, we should always + // traverse the siblings of any of its children. + if let Some(sibling) = move_path.next_sibling { + todo.push(sibling); + } + } + + None + } } impl<'tcx> fmt::Debug for MovePath<'tcx> { From 79f0820a9db5772d781cd0b9634fbd0a40709a25 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 17:34:32 -0800 Subject: [PATCH 08/12] Use new dataflow interface for `MaybeInitializedPlaces` --- src/librustc_mir/borrow_check/mod.rs | 16 +++--- src/librustc_mir/borrow_check/nll/mod.rs | 4 +- .../nll/type_check/liveness/mod.rs | 6 +-- .../nll/type_check/liveness/trace.rs | 40 +++++++------- .../borrow_check/nll/type_check/mod.rs | 6 +-- src/librustc_mir/dataflow/impls/mod.rs | 52 ++++++++++++------- src/librustc_mir/transform/elaborate_drops.rs | 22 ++++---- 7 files changed, 80 insertions(+), 66 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 427003f24cb14..8e3e4cb47c6b7 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -37,7 +37,7 @@ use crate::dataflow::Borrows; use crate::dataflow::DataflowResultsConsumer; use crate::dataflow::FlowAtLocation; use crate::dataflow::MoveDataParamEnv; -use crate::dataflow::{do_dataflow, DebugFormatted}; +use crate::dataflow::{self, do_dataflow, DebugFormatted}; use crate::dataflow::EverInitializedPlaces; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; @@ -185,15 +185,11 @@ fn do_mir_borrowck<'a, 'tcx>( }; let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); - let mut flow_inits = FlowAtLocation::new(do_dataflow( - tcx, - &body, - def_id, - &attributes, - &dead_unwinds, - MaybeInitializedPlaces::new(tcx, &body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i]), - )); + let flow_inits = MaybeInitializedPlaces::new(tcx, &body, &mdpe); + let mut flow_inits = + dataflow::generic::Engine::new_gen_kill(tcx, &body, def_id, &dead_unwinds, flow_inits) + .iterate_to_fixpoint() + .into_cursor(&body); let locals_are_invalidated_at_exit = tcx.hir().body_owner_kind(id).is_fn_or_closure(); let borrow_set = Rc::new(BorrowSet::build( diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index bbcb823c8f91c..595e1cdb6a1e3 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -3,8 +3,8 @@ use crate::borrow_check::location::LocationTable; use crate::borrow_check::nll::facts::AllFactsExt; use crate::borrow_check::nll::type_check::{MirTypeckResults, MirTypeckRegionConstraints}; use crate::borrow_check::nll::region_infer::values::RegionValueElements; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::move_paths::{InitLocation, MoveData, InitKind}; -use crate::dataflow::FlowAtLocation; use crate::dataflow::MaybeInitializedPlaces; use crate::transform::MirSource; use crate::borrow_check::Upvar; @@ -165,7 +165,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( upvars: &[Upvar], location_table: &LocationTable, param_env: ty::ParamEnv<'tcx>, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, + flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, errors_buffer: &mut Vec, diff --git a/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs index 8f8e9af797963..57c42dca80933 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs @@ -4,8 +4,8 @@ use crate::borrow_check::nll::facts::{AllFacts, AllFactsExt}; use crate::borrow_check::nll::region_infer::values::RegionValueElements; use crate::borrow_check::nll::universal_regions::UniversalRegions; use crate::borrow_check::nll::ToRegionVid; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::move_paths::MoveData; -use crate::dataflow::FlowAtLocation; use crate::dataflow::MaybeInitializedPlaces; use rustc::mir::{Body, Local, ReadOnlyBodyAndCache}; use rustc::ty::{RegionVid, TyCtxt}; @@ -26,11 +26,11 @@ mod trace; /// /// N.B., this computation requires normalization; therefore, it must be /// performed before -pub(super) fn generate<'tcx>( +pub(super) fn generate<'mir, 'tcx>( typeck: &mut TypeChecker<'_, 'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, elements: &Rc, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'tcx>>, + flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, location_table: &LocationTable, ) { diff --git a/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs b/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs index ba6fd75eea59c..8e670455bdf0b 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs @@ -3,9 +3,10 @@ use crate::borrow_check::nll::type_check::liveness::local_use_map::LocalUseMap; use crate::borrow_check::nll::type_check::liveness::polonius; use crate::borrow_check::nll::type_check::NormalizeLocation; use crate::borrow_check::nll::type_check::TypeChecker; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::indexes::MovePathIndex; -use crate::dataflow::move_paths::MoveData; -use crate::dataflow::{FlowAtLocation, FlowsAtLocation, MaybeInitializedPlaces}; +use crate::dataflow::move_paths::{HasMoveData, MoveData}; +use crate::dataflow::MaybeInitializedPlaces; use rustc::infer::canonical::QueryRegionConstraints; use rustc::mir::{BasicBlock, ConstraintCategory, Local, Location, ReadOnlyBodyAndCache}; use rustc::traits::query::dropck_outlives::DropckOutlivesResult; @@ -34,7 +35,7 @@ pub(super) fn trace( typeck: &mut TypeChecker<'_, 'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, elements: &Rc, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'tcx>>, + flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, live_locals: Vec, polonius_drop_used: Option>, @@ -81,7 +82,7 @@ struct LivenessContext<'me, 'typeck, 'flow, 'tcx> { /// Results of dataflow tracking which variables (and paths) have been /// initialized. - flow_inits: &'me mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'flow, 'tcx>>, + flow_inits: &'me mut ResultsCursor<'flow, 'tcx, MaybeInitializedPlaces<'flow, 'tcx>>, /// Index indicating where each variable is assigned, used, or /// dropped. @@ -390,23 +391,26 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> { } impl LivenessContext<'_, '_, '_, 'tcx> { + /// Returns `true` if the local variable (or some part of it) is initialized at the current + /// cursor position. Callers should call one of the `seek` methods immediately before to point + /// the cursor to the desired location. + fn initialized_at_curr_loc(&self, mpi: MovePathIndex) -> bool { + let state = self.flow_inits.get(); + if state.contains(mpi) { + return true; + } + + let move_paths = &self.flow_inits.analysis().move_data().move_paths; + move_paths[mpi].find_child(&move_paths, |mpi| state.contains(mpi)).is_some() + } + /// Returns `true` if the local variable (or some part of it) is initialized in /// the terminator of `block`. We need to check this to determine if a /// 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 { - // Compute the set of initialized paths at terminator of block - // by resetting to the start of the block and then applying - // the effects of all statements. This is the only way to get - // "just ahead" of a terminator. - self.flow_inits.reset_to_entry_of(block); - for statement_index in 0..self.body[block].statements.len() { - let location = Location { block, statement_index }; - self.flow_inits.reconstruct_statement_effect(location); - self.flow_inits.apply_local_effect(location); - } - - self.flow_inits.has_any_child_of(mpi).is_some() + self.flow_inits.seek_before(self.body.terminator_loc(block)); + self.initialized_at_curr_loc(mpi) } /// Returns `true` if the path `mpi` (or some part of it) is initialized at @@ -415,8 +419,8 @@ 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.reset_to_exit_of(block); - self.flow_inits.has_any_child_of(mpi).is_some() + self.flow_inits.seek_after(self.body.terminator_loc(block)); + self.initialized_at_curr_loc(mpi) } /// Stores the result that all regions in `value` are live for the diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 8d4e76cadbfc2..4f501f85ea314 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -50,7 +50,7 @@ use crate::borrow_check::nll::type_check::free_region_relations::{ CreateResult, UniversalRegionRelations, }; use crate::borrow_check::nll::universal_regions::{DefiningTy, UniversalRegions}; -use crate::dataflow::FlowAtLocation; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::MaybeInitializedPlaces; use crate::dataflow::move_paths::MoveData; use crate::transform::promote_consts::should_suggest_const_in_array_repeat_expressions_attribute; @@ -114,7 +114,7 @@ mod relate_tys; /// constraints for the regions in the types of variables /// - `flow_inits` -- results of a maybe-init dataflow analysis /// - `move_data` -- move-data constructed when performing the maybe-init dataflow analysis -pub(crate) fn type_check<'tcx>( +pub(crate) fn type_check<'mir, 'tcx>( infcx: &InferCtxt<'_, 'tcx>, param_env: ty::ParamEnv<'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, @@ -124,7 +124,7 @@ pub(crate) fn type_check<'tcx>( location_table: &LocationTable, borrow_set: &BorrowSet<'tcx>, all_facts: &mut Option, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'tcx>>, + flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, elements: &Rc, ) -> MirTypeckResults<'tcx> { diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 0fb912b5fcbd8..8cf643e95f81a 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -13,6 +13,7 @@ use crate::util::elaborate_drops::DropFlagState; use super::move_paths::{HasMoveData, MoveData, MovePathIndex, InitIndex, InitKind}; use super::{BitDenotation, BottomValue, GenKillSet}; +use super::generic::GenKill; use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; @@ -226,7 +227,7 @@ impl<'a, 'tcx> HasMoveData<'tcx> for EverInitializedPlaces<'a, 'tcx> { } impl<'a, 'tcx> MaybeInitializedPlaces<'a, 'tcx> { - fn update_bits(trans: &mut GenKillSet, + fn update_bits(trans: &mut impl GenKill, path: MovePathIndex, state: DropFlagState) { @@ -261,26 +262,36 @@ impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { } } -impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { +impl<'tcx> super::generic::AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { "maybe_init" } - fn bits_per_block(&self) -> usize { + + const NAME: &'static str = "maybe_init"; + + fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { self.move_data().move_paths.len() } - fn start_block_effect(&self, entry_set: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut BitSet) { drop_flag_effects_for_function_entry( self.tcx, self.body, self.mdpe, |path, s| { assert!(s == DropFlagState::Present); - entry_set.insert(path); + state.insert(path); }); } - fn statement_effect(&self, - trans: &mut GenKillSet, - location: Location) - { + fn pretty_print_idx(&self, w: &mut impl std::io::Write, mpi: Self::Idx) -> std::io::Result<()> { + write!(w, "{:?}", self.move_data().move_paths[mpi]) + } +} + +impl<'tcx> super::generic::GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { + fn statement_effect( + &self, + trans: &mut impl GenKill, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { drop_flag_effects_for_location( self.tcx, self.body, self.mdpe, location, @@ -288,10 +299,12 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { ) } - fn terminator_effect(&self, - trans: &mut GenKillSet, - location: Location) - { + fn terminator_effect( + &self, + trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { drop_flag_effects_for_location( self.tcx, self.body, self.mdpe, location, @@ -299,18 +312,19 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { ) } - fn propagate_call_return( + fn call_return_effect( &self, - in_out: &mut BitSet, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, + trans: &mut impl GenKill, + _block: mir::BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], dest_place: &mir::Place<'tcx>, ) { // when a call returns successfully, that means we need to set // the bits for that dest_place to 1 (initialized). on_lookup_result_bits(self.tcx, self.body, self.move_data(), self.move_data().rev_lookup.find(dest_place.as_ref()), - |mpi| { in_out.insert(mpi); }); + |mpi| { trans.gen(mpi); }); } } diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 9970752a37698..4cc74e7e96b74 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -5,6 +5,7 @@ use crate::dataflow::{on_all_children_bits, on_all_drop_children_bits}; use crate::dataflow::{drop_flag_effects_for_location, on_lookup_result_bits}; use crate::dataflow::MoveDataParamEnv; use crate::dataflow::{self, do_dataflow, DebugFormatted}; +use crate::dataflow::generic::Results; use crate::transform::{MirPass, MirSource}; use crate::util::patch::MirPatch; use crate::util::elaborate_drops::{DropFlagState, Unwind, elaborate_drop}; @@ -37,10 +38,10 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { param_env, }; let dead_unwinds = find_dead_unwinds(tcx, body, def_id, &env); - let flow_inits = - do_dataflow(tcx, body, def_id, &[], &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &env), - |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); + let flow_inits = dataflow::generic::Engine::new_gen_kill( + tcx, body, def_id, &dead_unwinds, + MaybeInitializedPlaces::new(tcx, body, &env), + ).iterate_to_fixpoint(); let flow_uninits = do_dataflow(tcx, body, def_id, &[], &dead_unwinds, MaybeUninitializedPlaces::new(tcx, body, &env), @@ -73,10 +74,10 @@ fn find_dead_unwinds<'tcx>( // We only need to do this pass once, because unwind edges can only // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); + let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env); let flow_inits = - do_dataflow(tcx, body, def_id, &[], &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &env), - |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); + dataflow::generic::Engine::new_gen_kill(tcx, body, def_id, &dead_unwinds, flow_inits) + .iterate_to_fixpoint(); for (bb, bb_data) in body.basic_blocks().iter_enumerated() { let location = match bb_data.terminator().kind { TerminatorKind::Drop { ref location, unwind: Some(_), .. } | @@ -85,7 +86,7 @@ fn find_dead_unwinds<'tcx>( }; let mut init_data = InitializationData { - live: flow_inits.sets().entry_set_for(bb.index()).to_owned(), + live: flow_inits.on_block_entry(bb).clone(), dead: BitSet::new_empty(env.move_data.move_paths.len()), }; debug!("find_dead_unwinds @ {:?}: {:?}; init_data={:?}", @@ -266,7 +267,7 @@ struct ElaborateDropsCtxt<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, env: &'a MoveDataParamEnv<'tcx>, - flow_inits: DataflowResults<'tcx, MaybeInitializedPlaces<'a, 'tcx>>, + flow_inits: Results<'tcx, MaybeInitializedPlaces<'a, 'tcx>>, flow_uninits: DataflowResults<'tcx, MaybeUninitializedPlaces<'a, 'tcx>>, drop_flags: FxHashMap, patch: MirPatch<'tcx>, @@ -281,8 +282,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { fn initialization_data_at(&self, loc: Location) -> InitializationData { let mut data = InitializationData { - live: self.flow_inits.sets().entry_set_for(loc.block.index()) - .to_owned(), + live: self.flow_inits.on_block_entry(loc.block).to_owned(), dead: self.flow_uninits.sets().entry_set_for(loc.block.index()) .to_owned(), }; From 09653fdc2e49aadf33f63088b63c88fa677b9f28 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 11 Nov 2019 18:15:47 -0800 Subject: [PATCH 09/12] Reenable `rustc_peek` tests for `MaybeInitializedPlaces` --- src/librustc_mir/transform/rustc_peek.rs | 22 ++++++++++++---------- src/test/ui/mir-dataflow/inits-1.rs | 2 -- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 034167b5aed6b..c5b408105f347 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -9,7 +9,7 @@ use rustc::mir::{self, Body, BodyAndCache, Location, Local}; use rustc_index::bit_set::BitSet; use crate::transform::{MirPass, MirSource}; -use crate::dataflow::generic::{Analysis, Results, ResultsCursor}; +use crate::dataflow::generic::{Analysis, Engine, Results, ResultsCursor}; use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::MoveDataParamEnv; use crate::dataflow::{ @@ -40,10 +40,11 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { let move_data = MoveData::gather_moves(body, tcx).unwrap(); let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env }; let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); - let flow_inits = - do_dataflow(tcx, body, def_id, &attributes, &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); + + let flow_inits = Engine::new_gen_kill( + tcx, body, def_id, &dead_unwinds, + MaybeInitializedPlaces::new(tcx, body, &mdpe), + ).iterate_to_fixpoint(); let flow_uninits = do_dataflow(tcx, body, def_id, &attributes, &dead_unwinds, MaybeUninitializedPlaces::new(tcx, body, &mdpe), @@ -57,11 +58,15 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { IndirectlyMutableLocals::new(tcx, body, param_env), |_, i| DebugFormatted::new(&i)); - // FIXME: add these back as dataflow analyses are migrated - /* if has_rustc_mir_with(&attributes, sym::rustc_peek_maybe_init).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_inits); } + if has_rustc_mir_with(&attributes, sym::stop_after_dataflow).is_some() { + tcx.sess.fatal("stop_after_dataflow ended compilation"); + } + + // FIXME: add these back as dataflow analyses are migrated + /* if has_rustc_mir_with(&attributes, sym::rustc_peek_maybe_uninit).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_uninits); } @@ -81,9 +86,6 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { &attributes, &flow_indirectly_mut); } - if has_rustc_mir_with(&attributes, sym::stop_after_dataflow).is_some() { - tcx.sess.fatal("stop_after_dataflow ended compilation"); - } */ } } diff --git a/src/test/ui/mir-dataflow/inits-1.rs b/src/test/ui/mir-dataflow/inits-1.rs index 0e1839495bd39..4a4786a2a7378 100644 --- a/src/test/ui/mir-dataflow/inits-1.rs +++ b/src/test/ui/mir-dataflow/inits-1.rs @@ -1,7 +1,5 @@ // General test of maybe_inits state computed by MIR dataflow. -// ignore-test Temporarily ignored while this analysis is migrated to the new framework. - #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; From bc1b6ba89ab9efe1600205203dc9e7c1171ecb36 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 12 Nov 2019 16:12:15 -0800 Subject: [PATCH 10/12] Add test for `ResultsCursor` This is a unit test that ensures the `seek` functions work correctly. --- src/librustc/mir/mod.rs | 24 ++ src/librustc_mir/dataflow/generic/mod.rs | 3 + src/librustc_mir/dataflow/generic/tests.rs | 297 +++++++++++++++++++++ 3 files changed, 324 insertions(+) create mode 100644 src/librustc_mir/dataflow/generic/tests.rs diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index a7f5a22692515..990199683acb3 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -194,6 +194,30 @@ impl<'tcx> Body<'tcx> { } } + /// Returns a partially initialized MIR body containing only a list of basic blocks. + /// + /// The returned MIR contains no `LocalDecl`s (even for the return place) or source scopes. It + /// is only useful for testing but cannot be `#[cfg(test)]` because it is used in a different + /// crate. + pub fn new_cfg_only(basic_blocks: IndexVec>) -> Self { + Body { + phase: MirPhase::Build, + basic_blocks, + source_scopes: IndexVec::new(), + yield_ty: None, + generator_drop: None, + generator_layout: None, + local_decls: IndexVec::new(), + user_type_annotations: IndexVec::new(), + arg_count: 0, + spread_arg: None, + span: syntax_pos::DUMMY_SP, + control_flow_destroyed: Vec::new(), + generator_kind: None, + var_debug_info: Vec::new(), + } + } + #[inline] pub fn basic_blocks(&self) -> &IndexVec> { &self.basic_blocks diff --git a/src/librustc_mir/dataflow/generic/mod.rs b/src/librustc_mir/dataflow/generic/mod.rs index 6271c37de3558..38bcf6084bb88 100644 --- a/src/librustc_mir/dataflow/generic/mod.rs +++ b/src/librustc_mir/dataflow/generic/mod.rs @@ -303,3 +303,6 @@ impl GenKill for BitSet { self.remove(elem); } } + +#[cfg(test)] +mod tests; diff --git a/src/librustc_mir/dataflow/generic/tests.rs b/src/librustc_mir/dataflow/generic/tests.rs new file mode 100644 index 0000000000000..bf2b866351ccf --- /dev/null +++ b/src/librustc_mir/dataflow/generic/tests.rs @@ -0,0 +1,297 @@ +use rustc::mir::{self, Location, BasicBlock}; +use rustc::ty; +use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; + +use crate::dataflow::BottomValue; +use super::*; + +#[test] +fn cursor_seek() { + let body = dummy_body(); + let body = &body; + let analysis = Dummy { body }; + + let mut cursor = Results { + entry_sets: analysis.entry_sets(), + analysis, + }.into_cursor(body); + + // Sanity check: the dummy call return effect is unique and actually being applied. + + let call_terminator_loc = Location { block: BasicBlock::from_usize(2), statement_index: 2 }; + assert!(is_call_terminator_non_diverging(body, call_terminator_loc)); + + let call_return_effect = + SeekTarget::AfterAssumeCallReturns(call_terminator_loc).effect(body).unwrap(); + assert_ne!(call_return_effect, SeekTarget::After(call_terminator_loc).effect(body).unwrap()); + + cursor.seek_after(call_terminator_loc); + assert!(!cursor.get().contains(call_return_effect)); + cursor.seek_after_assume_call_returns(call_terminator_loc); + assert!(cursor.get().contains(call_return_effect)); + + let every_target = || body + .basic_blocks() + .iter_enumerated() + .flat_map(|(bb, _)| SeekTarget::iter_in_block(body, bb)); + + let mut seek_to_target = |targ| { + use SeekTarget::*; + + match targ { + BlockStart(block) => cursor.seek_to_block_start(block), + Before(loc) => cursor.seek_before(loc), + After(loc) => cursor.seek_after(loc), + AfterAssumeCallReturns(loc) => cursor.seek_after_assume_call_returns(loc), + } + + assert_eq!(cursor.get(), &cursor.analysis().expected_state_at(targ)); + }; + + // Seek *to* every possible `SeekTarget` *from* every possible `SeekTarget`. + // + // By resetting the cursor to `from` each time it changes, we end up checking some edges twice. + // What we really want is an Eulerian cycle for the complete digraph over all possible + // `SeekTarget`s, but it's not worth spending the time to compute it. + for from in every_target() { + seek_to_target(from); + + for to in every_target() { + seek_to_target(to); + seek_to_target(from); + } + } +} + +const BASIC_BLOCK_OFFSET: usize = 100; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum SeekTarget { + BlockStart(BasicBlock), + Before(Location), + After(Location), + AfterAssumeCallReturns(Location), +} + +impl SeekTarget { + fn block(&self) -> BasicBlock { + use SeekTarget::*; + + match *self { + BlockStart(block) => block, + Before(loc) | After(loc) | AfterAssumeCallReturns(loc) => loc.block, + } + } + + /// Returns the largest index (not including the basic block index) that should be set when + /// seeking to this location. + /// + /// The index for `AfterAssumeCallReturns` is the same as `After` unless the cursor is pointing + /// at a `Call` terminator that can return. Returns `None` for `BlockStart`. + fn effect(&self, body: &mir::Body<'_>) -> Option { + use SeekTarget::*; + + let idx = match *self { + BlockStart(_) => return None, + + AfterAssumeCallReturns(loc) if is_call_terminator_non_diverging(body, loc) + => loc.statement_index * 2 + 2, + + Before(loc) => loc.statement_index * 2, + After(loc) | AfterAssumeCallReturns(loc) => loc.statement_index * 2 + 1, + }; + + assert!(idx < BASIC_BLOCK_OFFSET, "Too many statements in basic block"); + Some(idx) + } + + /// An iterator over all possible `SeekTarget`s in a given block in order, starting with + /// `BlockStart`. + /// + /// This includes both `After` and `AfterAssumeCallReturns` for every `Location`. + fn iter_in_block(body: &mir::Body<'_>, block: BasicBlock) -> impl Iterator { + let statements_and_terminator = (0..=body[block].statements.len()) + .flat_map(|i| (0..3).map(move |j| (i, j))) + .map(move |(i, kind)| { + let loc = Location { block, statement_index: i }; + match kind { + 0 => SeekTarget::Before(loc), + 1 => SeekTarget::After(loc), + 2 => SeekTarget::AfterAssumeCallReturns(loc), + _ => unreachable!(), + } + }); + + std::iter::once(SeekTarget::BlockStart(block)) + .chain(statements_and_terminator) + } +} + +/// A mock dataflow analysis for testing `ResultsCursor`. +/// +/// `Dummy` assigns a unique, monotonically increasing index to each possible cursor position as +/// well as one to each basic block. Instead of being iterated to fixpoint, `Dummy::entry_sets` is +/// used to construct the entry set to each block such that the dataflow state that should be +/// observed by `ResultsCursor` is unique for every location (see `expected_state_at`). +struct Dummy<'tcx> { + body: &'tcx mir::Body<'tcx>, +} + +impl Dummy<'tcx> { + fn entry_sets(&self) -> IndexVec> { + let empty = BitSet::new_empty(self.bits_per_block(self.body)); + let mut ret = IndexVec::from_elem(empty, &self.body.basic_blocks()); + + for (bb, _) in self.body.basic_blocks().iter_enumerated() { + ret[bb].insert(BASIC_BLOCK_OFFSET + bb.index()); + } + + ret + } + + /// Returns the expected state at the given `SeekTarget`. + /// + /// This is the union of index of the target basic block, the index assigned to the of the + /// target statement or terminator, and the indices of all preceding statements in the MIR. + /// + /// For example, the expected state when calling + /// `seek_before(Location { block: 2, statement_index: 2 })` would be `[102, 0, 1, 2, 3, 4]`. + fn expected_state_at(&self, target: SeekTarget) -> BitSet { + let mut ret = BitSet::new_empty(self.bits_per_block(self.body)); + ret.insert(BASIC_BLOCK_OFFSET + target.block().index()); + + if let Some(target_effect) = target.effect(self.body) { + for i in 0..=target_effect { + ret.insert(i); + } + } + + ret + } +} + +impl BottomValue for Dummy<'tcx> { + const BOTTOM_VALUE: bool = false; +} + +impl AnalysisDomain<'tcx> for Dummy<'tcx> { + type Idx = usize; + + const NAME: &'static str = "dummy"; + + fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { + BASIC_BLOCK_OFFSET + body.basic_blocks().len() + } + + fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet) { + unimplemented!(); + } +} + +impl Analysis<'tcx> for Dummy<'tcx> { + fn apply_statement_effect( + &self, + state: &mut BitSet, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { + let idx = SeekTarget::After(location).effect(self.body).unwrap(); + assert!(state.insert(idx)); + } + + fn apply_before_statement_effect( + &self, + state: &mut BitSet, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { + let idx = SeekTarget::Before(location).effect(self.body).unwrap(); + assert!(state.insert(idx)); + } + + fn apply_terminator_effect( + &self, + state: &mut BitSet, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { + let idx = SeekTarget::After(location).effect(self.body).unwrap(); + assert!(state.insert(idx)); + } + + fn apply_before_terminator_effect( + &self, + state: &mut BitSet, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { + let idx = SeekTarget::Before(location).effect(self.body).unwrap(); + assert!(state.insert(idx)); + } + + fn apply_call_return_effect( + &self, + state: &mut BitSet, + block: BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], + _return_place: &mir::Place<'tcx>, + ) { + let location = self.body.terminator_loc(block); + let idx = SeekTarget::AfterAssumeCallReturns(location).effect(self.body).unwrap(); + assert!(state.insert(idx)); + } +} + +fn is_call_terminator_non_diverging(body: &mir::Body<'_>, loc: Location) -> bool { + loc == body.terminator_loc(loc.block) + && matches!( + body[loc.block].terminator().kind, + mir::TerminatorKind::Call { destination: Some(_), .. } + ) +} + +fn dummy_body() -> mir::Body<'static> { + let span = syntax_pos::DUMMY_SP; + let source_info = mir::SourceInfo { scope: mir::OUTERMOST_SOURCE_SCOPE, span }; + + let mut blocks = IndexVec::new(); + let mut block = |n, kind| { + let nop = mir::Statement { + source_info, + kind: mir::StatementKind::Nop, + }; + + blocks.push(mir::BasicBlockData { + statements: std::iter::repeat(&nop).cloned().take(n).collect(), + terminator: Some(mir::Terminator { source_info, kind }), + is_cleanup: false, + }) + }; + + let dummy_place = mir::Place { + base: mir::PlaceBase::Local(mir::RETURN_PLACE), + projection: ty::List::empty(), + }; + + block(4, mir::TerminatorKind::Return); + block(1, mir::TerminatorKind::Return); + block(2, mir::TerminatorKind::Call { + func: mir::Operand::Copy(dummy_place.clone()), + args: vec![], + destination: Some((dummy_place.clone(), mir::START_BLOCK)), + cleanup: None, + from_hir_call: false, + }); + block(3, mir::TerminatorKind::Return); + block(4, mir::TerminatorKind::Call { + func: mir::Operand::Copy(dummy_place.clone()), + args: vec![], + destination: Some((dummy_place.clone(), mir::START_BLOCK)), + cleanup: None, + from_hir_call: false, + }); + + mir::Body::new_cfg_only(blocks) +} From ecd67eecf0e831fd9f077174b2871272be002c63 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 13 Nov 2019 20:17:07 -0800 Subject: [PATCH 11/12] Use builder method to set `dead_unwinds` `dead_unwinds` is an empty bitset in most places. This saves us from having to construct that empty bitset for every dataflow analysis. --- src/librustc_mir/borrow_check/mod.rs | 9 ++++---- src/librustc_mir/dataflow/generic/engine.rs | 22 ++++++++++--------- .../transform/check_consts/validation.rs | 8 ++----- src/librustc_mir/transform/elaborate_drops.rs | 15 +++++++------ src/librustc_mir/transform/rustc_peek.rs | 2 +- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 8e3e4cb47c6b7..3c34985adb370 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -184,12 +184,10 @@ fn do_mir_borrowck<'a, 'tcx>( param_env, }; - let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); let flow_inits = MaybeInitializedPlaces::new(tcx, &body, &mdpe); - let mut flow_inits = - dataflow::generic::Engine::new_gen_kill(tcx, &body, def_id, &dead_unwinds, flow_inits) - .iterate_to_fixpoint() - .into_cursor(&body); + let mut flow_inits = dataflow::generic::Engine::new_gen_kill(tcx, &body, def_id, flow_inits) + .iterate_to_fixpoint() + .into_cursor(&body); let locals_are_invalidated_at_exit = tcx.hir().body_owner_kind(id).is_fn_or_closure(); let borrow_set = Rc::new(BorrowSet::build( @@ -219,6 +217,7 @@ fn do_mir_borrowck<'a, 'tcx>( let regioncx = Rc::new(regioncx); + let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); let flow_borrows = FlowAtLocation::new(do_dataflow( tcx, &body, diff --git a/src/librustc_mir/dataflow/generic/engine.rs b/src/librustc_mir/dataflow/generic/engine.rs index ba182869b847e..3b2fb8f6d7792 100644 --- a/src/librustc_mir/dataflow/generic/engine.rs +++ b/src/librustc_mir/dataflow/generic/engine.rs @@ -25,7 +25,7 @@ where tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, def_id: DefId, - dead_unwinds: &'a BitSet, + dead_unwinds: Option<&'a BitSet>, entry_sets: IndexVec>, analysis: A, @@ -42,7 +42,6 @@ where tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, def_id: DefId, - dead_unwinds: &'a BitSet, analysis: A, ) -> Self { let bits_per_block = analysis.bits_per_block(body); @@ -70,7 +69,7 @@ where } } - Self::new(tcx, body, def_id, dead_unwinds, analysis, Some(trans_for_block)) + Self::new(tcx, body, def_id, analysis, Some(trans_for_block)) } } @@ -87,17 +86,15 @@ where tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, def_id: DefId, - dead_unwinds: &'a BitSet, analysis: A, ) -> Self { - Self::new(tcx, body, def_id, dead_unwinds, analysis, None) + Self::new(tcx, body, def_id, analysis, None) } fn new( tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, def_id: DefId, - dead_unwinds: &'a BitSet, analysis: A, trans_for_block: Option>>, ) -> Self { @@ -118,12 +115,17 @@ where tcx, body, def_id, - dead_unwinds, + dead_unwinds: None, entry_sets, trans_for_block, } } + pub fn dead_unwinds(mut self, dead_unwinds: &'a BitSet) -> Self { + self.dead_unwinds = Some(dead_unwinds); + self + } + pub fn iterate_to_fixpoint(mut self) -> Results<'tcx, A> { let mut temp_state = BitSet::new_empty(self.bits_per_block); @@ -229,7 +231,7 @@ where | DropAndReplace { target, value: _, location: _, unwind: Some(unwind) } => { self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); - if !self.dead_unwinds.contains(bb) { + if self.dead_unwinds.map_or(true, |bbs| !bbs.contains(bb)) { self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); } } @@ -242,7 +244,7 @@ where Call { cleanup, ref destination, ref func, ref args, .. } => { if let Some(unwind) = cleanup { - if !self.dead_unwinds.contains(bb) { + if self.dead_unwinds.map_or(true, |bbs| !bbs.contains(bb)) { self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); } } @@ -263,7 +265,7 @@ where FalseUnwind { real_target, unwind } => { self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); if let Some(unwind) = unwind { - if !self.dead_unwinds.contains(bb) { + if self.dead_unwinds.map_or(true, |bbs| !bbs.contains(bb)) { self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); } } diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 90ae479afe285..ce15567342b07 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -34,11 +34,10 @@ impl QualifCursor<'a, 'mir, 'tcx, Q> { pub fn new( q: Q, item: &'a Item<'mir, 'tcx>, - dead_unwinds: &BitSet, ) -> Self { let analysis = FlowSensitiveAnalysis::new(q, item); let results = - dataflow::Engine::new_generic(item.tcx, &item.body, item.def_id, dead_unwinds, analysis) + dataflow::Engine::new_generic(item.tcx, &item.body, item.def_id, analysis) .iterate_to_fixpoint(); let cursor = dataflow::ResultsCursor::new(*item.body, results); @@ -155,20 +154,17 @@ impl Validator<'a, 'mir, 'tcx> { pub fn new( item: &'a Item<'mir, 'tcx>, ) -> Self { - let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len()); - let needs_drop = QualifCursor::new( NeedsDrop, item, - &dead_unwinds, ); let has_mut_interior = QualifCursor::new( HasMutInterior, item, - &dead_unwinds, ); + let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len()); let indirectly_mutable = old_dataflow::do_dataflow( item.tcx, &*item.body, diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 4cc74e7e96b74..2313388c09047 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -38,10 +38,10 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { param_env, }; let dead_unwinds = find_dead_unwinds(tcx, body, def_id, &env); - let flow_inits = dataflow::generic::Engine::new_gen_kill( - tcx, body, def_id, &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &env), - ).iterate_to_fixpoint(); + let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env); + let flow_inits = dataflow::generic::Engine::new_gen_kill(tcx, body, def_id, flow_inits) + .dead_unwinds(&dead_unwinds) + .iterate_to_fixpoint(); let flow_uninits = do_dataflow(tcx, body, def_id, &[], &dead_unwinds, MaybeUninitializedPlaces::new(tcx, body, &env), @@ -74,10 +74,11 @@ fn find_dead_unwinds<'tcx>( // We only need to do this pass once, because unwind edges can only // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); + let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env); - let flow_inits = - dataflow::generic::Engine::new_gen_kill(tcx, body, def_id, &dead_unwinds, flow_inits) - .iterate_to_fixpoint(); + let flow_inits = dataflow::generic::Engine::new_gen_kill(tcx, body, def_id, flow_inits) + .iterate_to_fixpoint(); + for (bb, bb_data) in body.basic_blocks().iter_enumerated() { let location = match bb_data.terminator().kind { TerminatorKind::Drop { ref location, unwind: Some(_), .. } | diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index c5b408105f347..219e52fb23075 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -42,7 +42,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); let flow_inits = Engine::new_gen_kill( - tcx, body, def_id, &dead_unwinds, + tcx, body, def_id, MaybeInitializedPlaces::new(tcx, body, &mdpe), ).iterate_to_fixpoint(); let flow_uninits = From a9d46c7dafbe82f4dc2d341c624cd1eb63309c80 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 14 Nov 2019 17:58:24 -0800 Subject: [PATCH 12/12] Check block transfer function cache outside hot loop Keeping this DRY required a refactor since we cannot borrow `Engine` mutably while passing a closure that also holds a reference to it. This refactor also cleans up `propagate_bits_into_graph_successors_of`, mostly to keep line lengths in check. --- src/librustc_mir/dataflow/generic/engine.rs | 238 +++++++++++--------- 1 file changed, 129 insertions(+), 109 deletions(-) diff --git a/src/librustc_mir/dataflow/generic/engine.rs b/src/librustc_mir/dataflow/generic/engine.rs index 3b2fb8f6d7792..288e9ae541ee4 100644 --- a/src/librustc_mir/dataflow/generic/engine.rs +++ b/src/librustc_mir/dataflow/generic/engine.rs @@ -21,15 +21,15 @@ pub struct Engine<'a, 'tcx, A> where A: Analysis<'tcx>, { - bits_per_block: usize, tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, def_id: DefId, dead_unwinds: Option<&'a BitSet>, - entry_sets: IndexVec>, analysis: A, /// Cached, cumulative transfer functions for each block. + /// + /// These are only computable for gen-kill problems. trans_for_block: Option>>, } @@ -98,25 +98,12 @@ where analysis: A, trans_for_block: Option>>, ) -> Self { - let bits_per_block = analysis.bits_per_block(body); - - let bottom_value_set = if A::BOTTOM_VALUE == true { - BitSet::new_filled(bits_per_block) - } else { - BitSet::new_empty(bits_per_block) - }; - - let mut entry_sets = IndexVec::from_elem(bottom_value_set, body.basic_blocks()); - analysis.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); - Engine { analysis, - bits_per_block, tcx, body, def_id, dead_unwinds: None, - entry_sets, trans_for_block, } } @@ -126,37 +113,39 @@ where self } - pub fn iterate_to_fixpoint(mut self) -> Results<'tcx, A> { - let mut temp_state = BitSet::new_empty(self.bits_per_block); - - let mut dirty_queue: WorkQueue = - WorkQueue::with_none(self.body.basic_blocks().len()); - - for (bb, _) in traversal::reverse_postorder(self.body) { - dirty_queue.insert(bb); - } - - // Add blocks that are not reachable from START_BLOCK to the work queue. These blocks will - // be processed after the ones added above. - for bb in self.body.basic_blocks().indices() { - dirty_queue.insert(bb); - } + pub fn iterate_to_fixpoint(self) -> Results<'tcx, A> { + // Initialize the entry sets for each block. - while let Some(bb) = dirty_queue.pop() { - let bb_data = &self.body[bb]; - let on_entry = &self.entry_sets[bb]; + let bits_per_block = self.analysis.bits_per_block(self.body); + let bottom_value_set = if A::BOTTOM_VALUE == true { + BitSet::new_filled(bits_per_block) + } else { + BitSet::new_empty(bits_per_block) + }; - temp_state.overwrite(on_entry); - self.apply_whole_block_effect(&mut temp_state, bb, bb_data); + let mut entry_sets = IndexVec::from_elem(bottom_value_set, self.body.basic_blocks()); + self.analysis.initialize_start_block(self.body, &mut entry_sets[mir::START_BLOCK]); - self.propagate_bits_into_graph_successors_of( - &mut temp_state, - (bb, bb_data), - &mut dirty_queue, + // To improve performance, we check for the existence of cached block transfer functions + // *outside* the loop in `_iterate_to_fixpoint` below. + if let Some(trans_for_block) = &self.trans_for_block { + self._iterate_to_fixpoint( + bits_per_block, + &mut entry_sets, + |state, bb| trans_for_block[bb].apply(state), + ); + } else { + self._iterate_to_fixpoint( + bits_per_block, + &mut entry_sets, + |state, bb| { + let block_data = &self.body[bb]; + apply_whole_block_effect(&self.analysis, state, bb, block_data); + } ); } - let Engine { tcx, body, def_id, trans_for_block, entry_sets, analysis, .. } = self; + let Engine { tcx, def_id, body, analysis, trans_for_block, .. } = self; let results = Results { analysis, entry_sets }; let res = write_graphviz_results(tcx, def_id, body, &results, trans_for_block); @@ -167,124 +156,155 @@ where results } - /// Applies the cumulative effect of an entire block, excluding the call return effect if one - /// exists. - fn apply_whole_block_effect( + /// Helper function that propagates dataflow state into graph succesors until fixpoint is + /// reached. + fn _iterate_to_fixpoint( &self, - state: &mut BitSet, - block: BasicBlock, - block_data: &mir::BasicBlockData<'tcx>, + bits_per_block: usize, + entry_sets: &mut IndexVec>, + apply_block_effect: impl Fn(&mut BitSet, BasicBlock), ) { - // Use the cached block transfer function if available. - if let Some(trans_for_block) = &self.trans_for_block { - trans_for_block[block].apply(state); - return; + let body = self.body; + let mut state = BitSet::new_empty(bits_per_block); + + let mut dirty_queue: WorkQueue = + WorkQueue::with_none(body.basic_blocks().len()); + + for (bb, _) in traversal::reverse_postorder(body) { + dirty_queue.insert(bb); } - // Otherwise apply effects one-by-one. + // Add blocks that are not reachable from START_BLOCK to the work queue. These blocks will + // be processed after the ones added above. + for bb in body.basic_blocks().indices() { + dirty_queue.insert(bb); + } + + while let Some(bb) = dirty_queue.pop() { + state.overwrite(&entry_sets[bb]); + apply_block_effect(&mut state, bb); - for (statement_index, statement) in block_data.statements.iter().enumerate() { - let location = Location { block, statement_index }; - self.analysis.apply_before_statement_effect(state, statement, location); - self.analysis.apply_statement_effect(state, statement, location); + self.propagate_bits_into_graph_successors_of( + entry_sets, + &mut state, + (bb, &body[bb]), + &mut dirty_queue, + ); } + } - let terminator = block_data.terminator(); - let location = Location { block, statement_index: block_data.statements.len() }; - self.analysis.apply_before_terminator_effect(state, terminator, location); - self.analysis.apply_terminator_effect(state, terminator, location); + fn propagate_state_to( + &self, + bb: BasicBlock, + state: &BitSet, + entry_sets: &mut IndexVec>, + dirty_queue: &mut WorkQueue, + ) { + let entry_set = &mut entry_sets[bb]; + let set_changed = self.analysis.join(entry_set, state); + if set_changed { + dirty_queue.insert(bb); + } } fn propagate_bits_into_graph_successors_of( - &mut self, - in_out: &mut BitSet, - (bb, bb_data): (BasicBlock, &'a mir::BasicBlockData<'tcx>), - dirty_list: &mut WorkQueue, + &self, + entry_sets: &mut IndexVec>, + exit_state: &mut BitSet, + (bb, bb_data): (BasicBlock, &mir::BasicBlockData<'tcx>), + dirty: &mut WorkQueue, ) { - use mir::TerminatorKind::*; - - // FIXME: This should be implemented using a `for_each_successor` method on - // `TerminatorKind`. + use mir::TerminatorKind; match bb_data.terminator().kind { - | Return - | Resume - | Abort - | GeneratorDrop - | Unreachable + | TerminatorKind::Return + | TerminatorKind::Resume + | TerminatorKind::Abort + | TerminatorKind::GeneratorDrop + | TerminatorKind::Unreachable => {} - | Goto { target } - | Assert { target, cleanup: None, .. } - | Yield { resume: target, drop: None, .. } - | Drop { target, location: _, unwind: None } - | DropAndReplace { target, value: _, location: _, unwind: None } - => self.propagate_bits_into_entry_set_for(in_out, target, dirty_list), + | TerminatorKind::Goto { target } + | TerminatorKind::Assert { target, cleanup: None, .. } + | TerminatorKind::Yield { resume: target, drop: None, .. } + | TerminatorKind::Drop { target, location: _, unwind: None } + | TerminatorKind::DropAndReplace { target, value: _, location: _, unwind: None } + => self.propagate_state_to(target, exit_state, entry_sets, dirty), - Yield { resume: target, drop: Some(drop), .. } => { - self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); - self.propagate_bits_into_entry_set_for(in_out, drop, dirty_list); + TerminatorKind::Yield { resume: target, drop: Some(drop), .. } => { + self.propagate_state_to(target, exit_state, entry_sets, dirty); + self.propagate_state_to(drop, exit_state, entry_sets, dirty); } - | Assert { target, cleanup: Some(unwind), .. } - | Drop { target, location: _, unwind: Some(unwind) } - | DropAndReplace { target, value: _, location: _, unwind: Some(unwind) } + | TerminatorKind::Assert { target, cleanup: Some(unwind), .. } + | TerminatorKind::Drop { target, location: _, unwind: Some(unwind) } + | TerminatorKind::DropAndReplace { target, value: _, location: _, unwind: Some(unwind) } => { - self.propagate_bits_into_entry_set_for(in_out, target, dirty_list); + self.propagate_state_to(target, exit_state, entry_sets, dirty); if self.dead_unwinds.map_or(true, |bbs| !bbs.contains(bb)) { - self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); + self.propagate_state_to(unwind, exit_state, entry_sets, dirty); } } - SwitchInt { ref targets, .. } => { + TerminatorKind::SwitchInt { ref targets, .. } => { for target in targets { - self.propagate_bits_into_entry_set_for(in_out, *target, dirty_list); + self.propagate_state_to(*target, exit_state, entry_sets, dirty); } } - Call { cleanup, ref destination, ref func, ref args, .. } => { + TerminatorKind::Call { cleanup, ref destination, ref func, ref args, .. } => { if let Some(unwind) = cleanup { if self.dead_unwinds.map_or(true, |bbs| !bbs.contains(bb)) { - self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); + self.propagate_state_to(unwind, exit_state, entry_sets, dirty); } } if let Some((ref dest_place, dest_bb)) = *destination { // N.B.: This must be done *last*, otherwise the unwind path will see the call // return effect. - self.analysis.apply_call_return_effect(in_out, bb, func, args, dest_place); - self.propagate_bits_into_entry_set_for(in_out, dest_bb, dirty_list); + self.analysis.apply_call_return_effect(exit_state, bb, func, args, dest_place); + self.propagate_state_to(dest_bb, exit_state, entry_sets, dirty); } } - FalseEdges { real_target, imaginary_target } => { - self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); - self.propagate_bits_into_entry_set_for(in_out, imaginary_target, dirty_list); + TerminatorKind::FalseEdges { real_target, imaginary_target } => { + self.propagate_state_to(real_target, exit_state, entry_sets, dirty); + self.propagate_state_to(imaginary_target, exit_state, entry_sets, dirty); } - FalseUnwind { real_target, unwind } => { - self.propagate_bits_into_entry_set_for(in_out, real_target, dirty_list); + TerminatorKind::FalseUnwind { real_target, unwind } => { + self.propagate_state_to(real_target, exit_state, entry_sets, dirty); if let Some(unwind) = unwind { if self.dead_unwinds.map_or(true, |bbs| !bbs.contains(bb)) { - self.propagate_bits_into_entry_set_for(in_out, unwind, dirty_list); + self.propagate_state_to(unwind, exit_state, entry_sets, dirty); } } } } } +} - fn propagate_bits_into_entry_set_for( - &mut self, - in_out: &BitSet, - bb: BasicBlock, - dirty_queue: &mut WorkQueue, - ) { - let entry_set = &mut self.entry_sets[bb]; - let set_changed = self.analysis.join(entry_set, &in_out); - if set_changed { - dirty_queue.insert(bb); - } +/// Applies the cumulative effect of an entire block, excluding the call return effect if one +/// exists. +fn apply_whole_block_effect( + analysis: &'a A, + state: &mut BitSet, + block: BasicBlock, + block_data: &'a mir::BasicBlockData<'tcx>, +) +where + A: Analysis<'tcx>, +{ + for (statement_index, statement) in block_data.statements.iter().enumerate() { + let location = Location { block, statement_index }; + analysis.apply_before_statement_effect(state, statement, location); + analysis.apply_statement_effect(state, statement, location); } + + let terminator = block_data.terminator(); + let location = Location { block, statement_index: block_data.statements.len() }; + analysis.apply_before_terminator_effect(state, terminator, location); + analysis.apply_terminator_effect(state, terminator, location); } // Graphviz