From 149307efb742bf25fc1c8da85f1e6560691ba3dd Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 27 Aug 2020 18:07:27 -0700 Subject: [PATCH 1/8] Improve `BitSet` APIs A few small cleanups to `BitSet` and friends: - Overload `clone_from` for `BitSet`. - Improve `Debug` represenation of `HybridBitSet`. - Make `HybridBitSet::domain_size` public. - Don't require `T: Idx` at the type level. The `Idx` bound is still on most `BitSet` methods, but like `HashMap`, it doesn't need to be satisfied for the type to exist. --- compiler/rustc_index/src/bit_set.rs | 54 ++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index c43d1a6830d..8e00e54650d 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -28,13 +28,20 @@ pub const WORD_BITS: usize = WORD_BYTES * 8; /// will panic if the bitsets have differing domain sizes. /// /// [`GrowableBitSet`]: struct.GrowableBitSet.html -#[derive(Clone, Eq, PartialEq, Decodable, Encodable)] -pub struct BitSet { +#[derive(Eq, PartialEq, Decodable, Encodable)] +pub struct BitSet { domain_size: usize, words: Vec, marker: PhantomData, } +impl BitSet { + /// Gets the domain size. + pub fn domain_size(&self) -> usize { + self.domain_size + } +} + impl BitSet { /// Creates a new, empty bitset with a given `domain_size`. #[inline] @@ -52,11 +59,6 @@ impl BitSet { result } - /// Gets the domain size. - pub fn domain_size(&self) -> usize { - self.domain_size - } - /// Clear all elements. #[inline] pub fn clear(&mut self) { @@ -75,12 +77,6 @@ impl BitSet { } } - /// Efficiently overwrite `self` with `other`. - pub fn overwrite(&mut self, other: &BitSet) { - assert!(self.domain_size == other.domain_size); - self.words.clone_from_slice(&other.words); - } - /// Count the number of set bits in the set. pub fn count(&self) -> usize { self.words.iter().map(|e| e.count_ones() as usize).sum() @@ -243,6 +239,21 @@ impl SubtractFromBitSet for BitSet { } } +impl Clone for BitSet { + fn clone(&self) -> Self { + BitSet { domain_size: self.domain_size, words: self.words.clone(), marker: PhantomData } + } + + fn clone_from(&mut self, from: &Self) { + if self.domain_size != from.domain_size { + self.words.resize(from.domain_size, 0); + self.domain_size = from.domain_size; + } + + self.words.copy_from_slice(&from.words); + } +} + impl fmt::Debug for BitSet { fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { w.debug_list().entries(self.iter()).finish() @@ -363,7 +374,7 @@ const SPARSE_MAX: usize = 8; /// /// This type is used by `HybridBitSet`; do not use directly. #[derive(Clone, Debug)] -pub struct SparseBitSet { +pub struct SparseBitSet { domain_size: usize, elems: ArrayVec<[T; SPARSE_MAX]>, } @@ -464,18 +475,27 @@ impl SubtractFromBitSet for SparseBitSet { /// All operations that involve an element will panic if the element is equal /// to or greater than the domain size. All operations that involve two bitsets /// will panic if the bitsets have differing domain sizes. -#[derive(Clone, Debug)] -pub enum HybridBitSet { +#[derive(Clone)] +pub enum HybridBitSet { Sparse(SparseBitSet), Dense(BitSet), } +impl fmt::Debug for HybridBitSet { + fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Sparse(b) => b.fmt(w), + Self::Dense(b) => b.fmt(w), + } + } +} + impl HybridBitSet { pub fn new_empty(domain_size: usize) -> Self { HybridBitSet::Sparse(SparseBitSet::new_empty(domain_size)) } - fn domain_size(&self) -> usize { + pub fn domain_size(&self) -> usize { match self { HybridBitSet::Sparse(sparse) => sparse.domain_size, HybridBitSet::Dense(dense) => dense.domain_size, From a88dc37c54fa7ed2d1758d06508223677b65d387 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 27 Aug 2020 18:10:57 -0700 Subject: [PATCH 2/8] Add `regex` dependency to `librustc_mir` --- Cargo.lock | 1 + compiler/rustc_mir/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ffc1f0dec1d..ee259b257ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3758,6 +3758,7 @@ dependencies = [ "itertools 0.8.2", "log_settings", "polonius-engine", + "regex", "rustc_apfloat", "rustc_ast", "rustc_attr", diff --git a/compiler/rustc_mir/Cargo.toml b/compiler/rustc_mir/Cargo.toml index 6b0412ece7a..0a22bc7d762 100644 --- a/compiler/rustc_mir/Cargo.toml +++ b/compiler/rustc_mir/Cargo.toml @@ -14,6 +14,7 @@ itertools = "0.8" tracing = "0.1" log_settings = "0.1.1" polonius-engine = "0.12.0" +regex = "1" rustc_middle = { path = "../rustc_middle" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } From 9e45e90596faf6e741665d1c4ff6b94ad3885dbe Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 27 Aug 2020 23:03:21 -0700 Subject: [PATCH 3/8] Allow access to the underlying `Results` from a `ResultsCursor` --- compiler/rustc_mir/src/dataflow/framework/cursor.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir/src/dataflow/framework/cursor.rs b/compiler/rustc_mir/src/dataflow/framework/cursor.rs index 4f5930dc3f5..8ec0169d8c1 100644 --- a/compiler/rustc_mir/src/dataflow/framework/cursor.rs +++ b/compiler/rustc_mir/src/dataflow/framework/cursor.rs @@ -68,7 +68,12 @@ where self.body } - /// Returns the `Analysis` used to generate the underlying results. + /// Returns the underlying `Results`. + pub fn results(&self) -> &Results<'tcx, A> { + &self.results.borrow() + } + + /// Returns the `Analysis` used to generate the underlying `Results`. pub fn analysis(&self) -> &A { &self.results.borrow().analysis } From 3233fb18a891363a2da36ce69ca16fbb219c96be Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 27 Aug 2020 23:02:46 -0700 Subject: [PATCH 4/8] Extend dataflow framework to support arbitrary lattices --- .../src/dataflow/framework/cursor.rs | 34 +- .../src/dataflow/framework/direction.rs | 28 +- .../src/dataflow/framework/engine.rs | 105 ++- .../rustc_mir/src/dataflow/framework/fmt.rs | 172 +++++ .../src/dataflow/framework/graphviz.rs | 609 ++++++++---------- .../src/dataflow/framework/lattice.rs | 196 ++++++ .../rustc_mir/src/dataflow/framework/mod.rs | 113 ++-- .../rustc_mir/src/dataflow/framework/tests.rs | 29 +- .../src/dataflow/framework/visitor.rs | 13 +- compiler/rustc_mir/src/dataflow/mod.rs | 6 +- compiler/rustc_mir/src/lib.rs | 1 + 11 files changed, 765 insertions(+), 541 deletions(-) create mode 100644 compiler/rustc_mir/src/dataflow/framework/fmt.rs create mode 100644 compiler/rustc_mir/src/dataflow/framework/lattice.rs diff --git a/compiler/rustc_mir/src/dataflow/framework/cursor.rs b/compiler/rustc_mir/src/dataflow/framework/cursor.rs index 8ec0169d8c1..4942bed656c 100644 --- a/compiler/rustc_mir/src/dataflow/framework/cursor.rs +++ b/compiler/rustc_mir/src/dataflow/framework/cursor.rs @@ -4,6 +4,7 @@ use std::borrow::Borrow; use std::cmp::Ordering; use rustc_index::bit_set::BitSet; +use rustc_index::vec::Idx; use rustc_middle::mir::{self, BasicBlock, Location}; use super::{Analysis, Direction, Effect, EffectIndex, Results}; @@ -26,7 +27,7 @@ where { body: &'mir mir::Body<'tcx>, results: R, - state: BitSet, + state: A::Domain, pos: CursorPosition, @@ -46,17 +47,16 @@ where { /// Returns a new cursor that can inspect `results`. pub fn new(body: &'mir mir::Body<'tcx>, results: R) -> Self { - let bits_per_block = results.borrow().entry_set_for_block(mir::START_BLOCK).domain_size(); - + let bottom_value = results.borrow().analysis.bottom_value(body); ResultsCursor { body, results, - // Initialize to an empty `BitSet` and set `state_needs_reset` to tell the cursor that + // Initialize to the `bottom_value` and set `state_needs_reset` to tell the cursor that // it needs to reset to block entry before the first seek. The cursor position is // immaterial. state_needs_reset: true, - state: BitSet::new_empty(bits_per_block), + state: bottom_value, pos: CursorPosition::block_entry(mir::START_BLOCK), #[cfg(debug_assertions)] @@ -79,17 +79,10 @@ where } /// Returns the dataflow state at the current location. - pub fn get(&self) -> &BitSet { + pub fn get(&self) -> &A::Domain { &self.state } - /// Returns `true` if the dataflow state at the current location contains the given element. - /// - /// Shorthand for `self.get().contains(elem)` - pub fn contains(&self, elem: A::Idx) -> bool { - self.state.contains(elem) - } - /// Resets the cursor to hold the entry set for the given basic block. /// /// For forward dataflow analyses, this is the dataflow state prior to the first statement. @@ -99,7 +92,7 @@ where #[cfg(debug_assertions)] assert!(self.reachable_blocks.contains(block)); - self.state.overwrite(&self.results.borrow().entry_set_for_block(block)); + self.state.clone_from(&self.results.borrow().entry_set_for_block(block)); self.pos = CursorPosition::block_entry(block); self.state_needs_reset = false; } @@ -207,12 +200,23 @@ where /// /// This can be used, e.g., to apply the call return effect directly to the cursor without /// creating an extra copy of the dataflow state. - pub fn apply_custom_effect(&mut self, f: impl FnOnce(&A, &mut BitSet)) { + pub fn apply_custom_effect(&mut self, f: impl FnOnce(&A, &mut A::Domain)) { f(&self.results.borrow().analysis, &mut self.state); self.state_needs_reset = true; } } +impl<'mir, 'tcx, A, R, T> ResultsCursor<'mir, 'tcx, A, R> +where + A: Analysis<'tcx, Domain = BitSet>, + T: Idx, + R: Borrow>, +{ + pub fn contains(&self, elem: T) -> bool { + self.get().contains(elem) + } +} + #[derive(Clone, Copy, Debug)] struct CursorPosition { block: BasicBlock, diff --git a/compiler/rustc_mir/src/dataflow/framework/direction.rs b/compiler/rustc_mir/src/dataflow/framework/direction.rs index 4512ae96c08..94d299bd088 100644 --- a/compiler/rustc_mir/src/dataflow/framework/direction.rs +++ b/compiler/rustc_mir/src/dataflow/framework/direction.rs @@ -18,7 +18,7 @@ pub trait Direction { /// `effects.start()` must precede or equal `effects.end()` in this direction. fn apply_effects_in_range( analysis: &A, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, effects: RangeInclusive, @@ -27,7 +27,7 @@ pub trait Direction { fn apply_effects_in_block( analysis: &A, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, ) where @@ -55,9 +55,9 @@ pub trait Direction { tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, dead_unwinds: Option<&BitSet>, - exit_state: &mut BitSet, + exit_state: &mut A::Domain, block: (BasicBlock, &'_ mir::BasicBlockData<'tcx>), - propagate: impl FnMut(BasicBlock, &BitSet), + propagate: impl FnMut(BasicBlock, &A::Domain), ) where A: Analysis<'tcx>; } @@ -72,7 +72,7 @@ impl Direction for Backward { fn apply_effects_in_block( analysis: &A, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, ) where @@ -112,7 +112,7 @@ impl Direction for Backward { fn apply_effects_in_range( analysis: &A, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, effects: RangeInclusive, @@ -224,9 +224,9 @@ impl Direction for Backward { _tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, dead_unwinds: Option<&BitSet>, - exit_state: &mut BitSet, + exit_state: &mut A::Domain, (bb, _bb_data): (BasicBlock, &'_ mir::BasicBlockData<'tcx>), - mut propagate: impl FnMut(BasicBlock, &BitSet), + mut propagate: impl FnMut(BasicBlock, &A::Domain), ) where A: Analysis<'tcx>, { @@ -281,7 +281,7 @@ impl Direction for Forward { fn apply_effects_in_block( analysis: &A, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, ) where @@ -321,7 +321,7 @@ impl Direction for Forward { fn apply_effects_in_range( analysis: &A, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, block_data: &mir::BasicBlockData<'tcx>, effects: RangeInclusive, @@ -428,9 +428,9 @@ impl Direction for Forward { tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, dead_unwinds: Option<&BitSet>, - exit_state: &mut BitSet, + exit_state: &mut A::Domain, (bb, bb_data): (BasicBlock, &'_ mir::BasicBlockData<'tcx>), - mut propagate: impl FnMut(BasicBlock, &BitSet), + mut propagate: impl FnMut(BasicBlock, &A::Domain), ) where A: Analysis<'tcx>, { @@ -499,7 +499,7 @@ impl Direction for Forward { // MIR building adds discriminants to the `values` array in the same order as they // are yielded by `AdtDef::discriminants`. We rely on this to match each // discriminant in `values` to its corresponding variant in linear time. - let mut tmp = BitSet::new_empty(exit_state.domain_size()); + let mut tmp = analysis.bottom_value(body); let mut discriminants = enum_def.discriminants(tcx); for (value, target) in values.iter().zip(targets.iter().copied()) { let (variant_idx, _) = @@ -508,7 +508,7 @@ impl Direction for Forward { from that of `SwitchInt::values`", ); - tmp.overwrite(exit_state); + tmp.clone_from(exit_state); analysis.apply_discriminant_switch_effect( &mut tmp, bb, diff --git a/compiler/rustc_mir/src/dataflow/framework/engine.rs b/compiler/rustc_mir/src/dataflow/framework/engine.rs index b703852b1de..07f5ab5e7bd 100644 --- a/compiler/rustc_mir/src/dataflow/framework/engine.rs +++ b/compiler/rustc_mir/src/dataflow/framework/engine.rs @@ -1,5 +1,6 @@ //! A solver for dataflow problems. +use std::borrow::BorrowMut; use std::ffi::OsString; use std::fs; use std::path::PathBuf; @@ -9,14 +10,16 @@ use rustc_data_structures::work_queue::WorkQueue; use rustc_graphviz as dot; use rustc_hir::def_id::DefId; use rustc_index::bit_set::BitSet; -use rustc_index::vec::IndexVec; +use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::mir::{self, traversal, BasicBlock}; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::symbol::{sym, Symbol}; +use super::fmt::DebugWithContext; use super::graphviz; use super::{ - visit_results, Analysis, Direction, GenKillAnalysis, GenKillSet, ResultsCursor, ResultsVisitor, + visit_results, Analysis, Direction, GenKill, GenKillAnalysis, GenKillSet, JoinSemiLattice, + ResultsCursor, ResultsVisitor, }; use crate::util::pretty::dump_enabled; @@ -26,7 +29,7 @@ where A: Analysis<'tcx>, { pub analysis: A, - pub(super) entry_sets: IndexVec>, + pub(super) entry_sets: IndexVec, } impl Results<'tcx, A> @@ -39,7 +42,7 @@ where } /// Gets the dataflow state for the given block. - pub fn entry_set_for_block(&self, block: BasicBlock) -> &BitSet { + pub fn entry_set_for_block(&self, block: BasicBlock) -> &A::Domain { &self.entry_sets[block] } @@ -47,7 +50,7 @@ where &self, body: &'mir mir::Body<'tcx>, blocks: impl IntoIterator, - vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = BitSet>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = A::Domain>, ) { visit_results(body, blocks, self, vis) } @@ -55,7 +58,7 @@ where pub fn visit_reachable_with( &self, body: &'mir mir::Body<'tcx>, - vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = BitSet>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = A::Domain>, ) { let blocks = mir::traversal::reachable(body); visit_results(body, blocks.map(|(bb, _)| bb), self, vis) @@ -64,7 +67,7 @@ where pub fn visit_in_rpo_with( &self, body: &'mir mir::Body<'tcx>, - vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = BitSet>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = A::Domain>, ) { let blocks = mir::traversal::reverse_postorder(body); visit_results(body, blocks.map(|(bb, _)| bb), self, vis) @@ -76,21 +79,22 @@ 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>, + entry_sets: IndexVec, analysis: A, /// Cached, cumulative transfer functions for each block. - trans_for_block: Option>>, + apply_trans_for_block: Option>, } -impl Engine<'a, 'tcx, A> +impl Engine<'a, 'tcx, A> where - A: GenKillAnalysis<'tcx>, + A: GenKillAnalysis<'tcx, Idx = T, Domain = D>, + D: Clone + JoinSemiLattice + GenKill + BorrowMut>, + T: Idx, { /// Creates a new `Engine` to solve a gen-kill dataflow problem. pub fn new_gen_kill( @@ -109,22 +113,26 @@ where // Otherwise, compute and store the cumulative transfer function for each block. - 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()); + let identity = GenKillSet::identity(analysis.bottom_value(body).borrow().domain_size()); + let mut trans_for_block = IndexVec::from_elem(identity, body.basic_blocks()); for (block, block_data) in body.basic_blocks().iter_enumerated() { let trans = &mut trans_for_block[block]; A::Direction::gen_kill_effects_in_block(&analysis, trans, block, block_data); } - Self::new(tcx, body, def_id, analysis, Some(trans_for_block)) + let apply_trans = Box::new(move |bb: BasicBlock, state: &mut A::Domain| { + trans_for_block[bb].apply(state.borrow_mut()); + }); + + Self::new(tcx, body, def_id, analysis, Some(apply_trans as Box<_>)) } } -impl Engine<'a, 'tcx, A> +impl Engine<'a, 'tcx, A> where - A: Analysis<'tcx>, + A: Analysis<'tcx, Domain = D>, + D: Clone + JoinSemiLattice, { /// Creates a new `Engine` to solve a dataflow problem with an arbitrary transfer /// function. @@ -145,32 +153,24 @@ where body: &'a mir::Body<'tcx>, def_id: DefId, analysis: A, - trans_for_block: Option>>, + apply_trans_for_block: Option>, ) -> Self { - let bits_per_block = analysis.bits_per_block(body); - - let bottom_value_set = if A::BOTTOM_VALUE { - BitSet::new_filled(bits_per_block) - } else { - BitSet::new_empty(bits_per_block) - }; - - let mut entry_sets = IndexVec::from_elem(bottom_value_set.clone(), body.basic_blocks()); + let bottom_value = analysis.bottom_value(body); + let mut entry_sets = IndexVec::from_elem(bottom_value.clone(), body.basic_blocks()); analysis.initialize_start_block(body, &mut entry_sets[mir::START_BLOCK]); - if A::Direction::is_backward() && entry_sets[mir::START_BLOCK] != bottom_value_set { + if A::Direction::is_backward() && entry_sets[mir::START_BLOCK] != bottom_value { bug!("`initialize_start_block` is not yet supported for backward dataflow analyses"); } Engine { analysis, - bits_per_block, tcx, body, def_id, dead_unwinds: None, entry_sets, - trans_for_block, + apply_trans_for_block, } } @@ -185,16 +185,18 @@ where } /// Computes the fixpoint for this dataflow problem and returns it. - pub fn iterate_to_fixpoint(self) -> Results<'tcx, A> { + pub fn iterate_to_fixpoint(self) -> Results<'tcx, A> + where + A::Domain: DebugWithContext, + { let Engine { analysis, - bits_per_block, body, dead_unwinds, def_id, mut entry_sets, tcx, - trans_for_block, + apply_trans_for_block, .. } = self; @@ -213,14 +215,14 @@ where } } - let mut state = BitSet::new_empty(bits_per_block); + let mut state = analysis.bottom_value(body); while let Some(bb) = dirty_queue.pop() { let bb_data = &body[bb]; // Apply the block transfer function, using the cached one if it exists. - state.overwrite(&entry_sets[bb]); - match &trans_for_block { - Some(trans_for_block) => trans_for_block[bb].apply(&mut state), + state.clone_from(&entry_sets[bb]); + match &apply_trans_for_block { + Some(apply) => apply(bb, &mut state), None => A::Direction::apply_effects_in_block(&analysis, &mut state, bb, bb_data), } @@ -231,8 +233,8 @@ where dead_unwinds, &mut state, (bb, bb_data), - |target: BasicBlock, state: &BitSet| { - let set_changed = analysis.join(&mut entry_sets[target], state); + |target: BasicBlock, state: &A::Domain| { + let set_changed = entry_sets[target].join(state); if set_changed { dirty_queue.insert(target); } @@ -242,7 +244,7 @@ where let results = Results { analysis, entry_sets }; - let res = write_graphviz_results(tcx, def_id, &body, &results, trans_for_block); + let res = write_graphviz_results(tcx, def_id, &body, &results); if let Err(e) = res { warn!("Failed to write graphviz dataflow results: {}", e); } @@ -260,10 +262,10 @@ fn write_graphviz_results( def_id: DefId, body: &mir::Body<'tcx>, results: &Results<'tcx, A>, - block_transfer_functions: Option>>, ) -> std::io::Result<()> where A: Analysis<'tcx>, + A::Domain: DebugWithContext, { let attrs = match RustcMirAttrs::parse(tcx, def_id) { Ok(attrs) => attrs, @@ -290,26 +292,15 @@ where 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(body, &results)) - } - } - - // Default to the `SimpleDiff` output style. - _ => Box::new(graphviz::SimpleDiff::new(body, &results)), + let style = match attrs.formatter { + Some(sym::two_phase) => graphviz::OutputStyle::BeforeAndAfter, + _ => graphviz::OutputStyle::AfterOnly, }; 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); + let graphviz = graphviz::Formatter::new(body, def_id, results, style); dot::render_opts(&graphviz, &mut buf, &[dot::RenderOption::Monospace])?; if let Some(parent) = path.parent() { diff --git a/compiler/rustc_mir/src/dataflow/framework/fmt.rs b/compiler/rustc_mir/src/dataflow/framework/fmt.rs new file mode 100644 index 00000000000..0140a750544 --- /dev/null +++ b/compiler/rustc_mir/src/dataflow/framework/fmt.rs @@ -0,0 +1,172 @@ +//! Custom formatting traits used when outputting Graphviz diagrams with the results of a dataflow +//! analysis. + +use rustc_index::bit_set::{BitSet, HybridBitSet}; +use rustc_index::vec::Idx; +use std::fmt; + +/// An extension to `fmt::Debug` for data that can be better printed with some auxiliary data `C`. +pub trait DebugWithContext: Eq + fmt::Debug { + fn fmt_with(&self, _ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(self, f) + } + + /// Print the difference between `self` and `old`. + /// + /// This should print nothing if `self == old`. + /// + /// `+` and `-` are typically used to indicate differences. However, these characters are + /// fairly common and may be needed to print a types representation. If using them to indicate + /// a diff, prefix them with the "Unit Separator" control character (␟ U+001F). + fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self == old { + return Ok(()); + } + + write!(f, "\u{001f}+")?; + self.fmt_with(ctxt, f)?; + + if f.alternate() { + write!(f, "\n")?; + } else { + write!(f, "\t")?; + } + + write!(f, "\u{001f}-")?; + self.fmt_with(ctxt, f) + } +} + +/// Implements `fmt::Debug` by deferring to `>::fmt_with`. +pub struct DebugWithAdapter<'a, T, C> { + pub this: T, + pub ctxt: &'a C, +} + +impl fmt::Debug for DebugWithAdapter<'_, T, C> +where + T: DebugWithContext, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.this.fmt_with(self.ctxt, f) + } +} + +/// Implements `fmt::Debug` by deferring to `>::fmt_diff_with`. +pub struct DebugDiffWithAdapter<'a, T, C> { + pub new: T, + pub old: T, + pub ctxt: &'a C, +} + +impl fmt::Debug for DebugDiffWithAdapter<'_, T, C> +where + T: DebugWithContext, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.new.fmt_diff_with(&self.old, self.ctxt, f) + } +} + +// Impls + +impl DebugWithContext for BitSet +where + T: Idx + DebugWithContext, +{ + fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_set().entries(self.iter().map(|i| DebugWithAdapter { this: i, ctxt })).finish() + } + + fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let size = self.domain_size(); + assert_eq!(size, old.domain_size()); + + let mut set_in_self = HybridBitSet::new_empty(size); + let mut cleared_in_self = HybridBitSet::new_empty(size); + + for i in (0..size).map(T::new) { + match (self.contains(i), old.contains(i)) { + (true, false) => set_in_self.insert(i), + (false, true) => cleared_in_self.insert(i), + _ => continue, + }; + } + + let mut first = true; + for idx in set_in_self.iter() { + let delim = if first { + "\u{001f}+" + } else if f.alternate() { + "\n\u{001f}+" + } else { + ", " + }; + + write!(f, "{}", delim)?; + idx.fmt_with(ctxt, f)?; + first = false; + } + + if !f.alternate() { + first = true; + if !set_in_self.is_empty() && !cleared_in_self.is_empty() { + write!(f, "\t")?; + } + } + + for idx in cleared_in_self.iter() { + let delim = if first { + "\u{001f}-" + } else if f.alternate() { + "\n\u{001f}-" + } else { + ", " + }; + + write!(f, "{}", delim)?; + idx.fmt_with(ctxt, f)?; + first = false; + } + + Ok(()) + } +} + +impl DebugWithContext for &'_ T +where + T: DebugWithContext, +{ + fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (*self).fmt_with(ctxt, f) + } + + fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (*self).fmt_diff_with(*old, ctxt, f) + } +} + +impl DebugWithContext for rustc_middle::mir::Local {} +impl DebugWithContext for crate::dataflow::move_paths::InitIndex {} + +impl<'tcx, C> DebugWithContext for crate::dataflow::move_paths::MovePathIndex +where + C: crate::dataflow::move_paths::HasMoveData<'tcx>, +{ + fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", ctxt.move_data().move_paths[*self]) + } +} + +impl DebugWithContext for crate::dataflow::lattice::Dual +where + T: DebugWithContext, +{ + fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (self.0).fmt_with(ctxt, f) + } + + fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (self.0).fmt_diff_with(&old.0, ctxt, f) + } +} diff --git a/compiler/rustc_mir/src/dataflow/framework/graphviz.rs b/compiler/rustc_mir/src/dataflow/framework/graphviz.rs index 896616a2175..179c471cf48 100644 --- a/compiler/rustc_mir/src/dataflow/framework/graphviz.rs +++ b/compiler/rustc_mir/src/dataflow/framework/graphviz.rs @@ -1,26 +1,40 @@ //! A helpful diagram for debugging dataflow problems. -use std::cell::RefCell; +use std::borrow::Cow; use std::{io, ops, str}; +use regex::Regex; use rustc_graphviz as dot; use rustc_hir::def_id::DefId; -use rustc_index::bit_set::{BitSet, HybridBitSet}; -use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::mir::{self, BasicBlock, Body, Location}; -use super::{Analysis, Direction, GenKillSet, Results, ResultsRefCursor}; +use super::fmt::{DebugDiffWithAdapter, DebugWithAdapter, DebugWithContext}; +use super::{Analysis, Direction, Results, ResultsRefCursor, ResultsVisitor}; use crate::util::graphviz_safe_def_name; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum OutputStyle { + AfterOnly, + BeforeAndAfter, +} + +impl OutputStyle { + fn num_state_columns(&self) -> usize { + match self { + Self::AfterOnly => 1, + Self::BeforeAndAfter => 2, + } + } +} + pub struct Formatter<'a, 'tcx, A> where A: Analysis<'tcx>, { body: &'a Body<'tcx>, def_id: DefId, - - // This must be behind a `RefCell` because `dot::Labeller` takes `&self`. - block_formatter: RefCell>, + results: &'a Results<'tcx, A>, + style: OutputStyle, } impl Formatter<'a, 'tcx, A> @@ -31,15 +45,9 @@ where body: &'a Body<'tcx>, def_id: DefId, results: &'a Results<'tcx, A>, - state_formatter: &'a mut dyn StateFormatter<'tcx, A>, + style: OutputStyle, ) -> Self { - let block_formatter = BlockFormatter { - bg: Background::Light, - results: ResultsRefCursor::new(body, results), - state_formatter, - }; - - Formatter { body, def_id, block_formatter: RefCell::new(block_formatter) } + Formatter { body, def_id, results, style } } } @@ -62,6 +70,7 @@ fn dataflow_successors(body: &Body<'tcx>, bb: BasicBlock) -> Vec { impl dot::Labeller<'_> for Formatter<'a, 'tcx, A> where A: Analysis<'tcx>, + A::Domain: DebugWithContext, { type Node = BasicBlock; type Edge = CfgEdge; @@ -77,7 +86,13 @@ where fn node_label(&self, block: &Self::Node) -> dot::LabelText<'_> { let mut label = Vec::new(); - self.block_formatter.borrow_mut().write_node_label(&mut label, self.body, *block).unwrap(); + let mut fmt = BlockFormatter { + results: ResultsRefCursor::new(self.body, self.results), + style: self.style, + bg: Background::Light, + }; + + fmt.write_node_label(&mut label, self.body, *block).unwrap(); dot::LabelText::html(String::from_utf8(label).unwrap()) } @@ -126,19 +141,16 @@ where { results: ResultsRefCursor<'a, 'a, 'tcx, A>, bg: Background, - state_formatter: &'a mut dyn StateFormatter<'tcx, A>, + style: OutputStyle, } impl BlockFormatter<'a, 'tcx, A> where A: Analysis<'tcx>, + A::Domain: DebugWithContext, { 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; @@ -187,40 +199,30 @@ where write!(w, r#""#, fmt = table_fmt)?; // 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)?; + match self.style { + OutputStyle::AfterOnly => self.write_block_header_simple(w, block)?, + OutputStyle::BeforeAndAfter => { + self.write_block_header_with_state_columns(w, block, &["BEFORE", "AFTER"])? + } } // C: State at start of block self.bg = Background::Light; self.results.seek_to_block_start(block); - let block_entry_state = self.results.get().clone(); - + let block_start_state = self.results.get().clone(); self.write_row_with_full_state(w, "", "(on start)")?; - // D: Statement transfer functions - for (i, statement) in body[block].statements.iter().enumerate() { - let location = Location { block, statement_index: i }; - 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 terminator_loc = body.terminator_loc(block); - let mut terminator_str = String::new(); - terminator.kind.fmt_head(&mut terminator_str).unwrap(); - - self.write_row_for_location(w, "T", &terminator_str, terminator_loc)?; + // D + E: Statement and terminator transfer functions + self.write_statements_and_terminator(w, body, block)?; // F: State at end of block + let terminator = body[block].terminator(); + // Write the full dataflow state immediately after the terminator if it differs from the // state at block entry. self.results.seek_to_block_end(block); - if self.results.get() != &block_entry_state || A::Direction::is_backward() { + if self.results.get() != &block_start_state || A::Direction::is_backward() { let after_terminator_name = match terminator.kind { mir::TerminatorKind::Call { destination: Some(_), .. } => "(on unwind)", _ => "(on end)", @@ -229,8 +231,11 @@ where self.write_row_with_full_state(w, "", after_terminator_name)?; } - // Write any changes caused by terminator-specific effects - let num_state_columns = self.num_state_columns(); + // Write any changes caused by terminator-specific effects. + // + // FIXME: These should really be printed as part of each outgoing edge rather than the node + // for the basic block itself. That way, we could display terminator-specific effects for + // backward dataflow analyses as well as effects for `SwitchInt` terminators. match terminator.kind { mir::TerminatorKind::Call { destination: Some((return_place, _)), @@ -239,44 +244,43 @@ where .. } => { self.write_row(w, "", "(on successful return)", |this, w, fmt| { - write!( - w, - r#""#, - colspan = num_state_columns, - fmt = fmt, - )?; - let state_on_unwind = this.results.get().clone(); this.results.apply_custom_effect(|analysis, state| { analysis.apply_call_return_effect(state, block, func, args, return_place); }); - write_diff(w, this.results.analysis(), &state_on_unwind, this.results.get())?; - write!(w, "") + write!( + w, + r#"{diff}"#, + colspan = this.style.num_state_columns(), + fmt = fmt, + diff = diff_pretty( + this.results.get(), + &state_on_unwind, + this.results.analysis() + ), + ) })?; } mir::TerminatorKind::Yield { resume, resume_arg, .. } => { self.write_row(w, "", "(on yield resume)", |this, w, fmt| { - write!( - w, - r#""#, - colspan = num_state_columns, - fmt = fmt, - )?; - let state_on_generator_drop = this.results.get().clone(); this.results.apply_custom_effect(|analysis, state| { analysis.apply_yield_resume_effect(state, resume, resume_arg); }); - write_diff( + write!( w, - this.results.analysis(), - &state_on_generator_drop, - this.results.get(), - )?; - write!(w, "") + r#"{diff}"#, + colspan = this.style.num_state_columns(), + fmt = fmt, + diff = diff_pretty( + this.results.get(), + &state_on_generator_drop, + this.results.analysis() + ), + ) })?; } @@ -322,6 +326,7 @@ where &mut self, w: &mut impl io::Write, block: BasicBlock, + state_column_names: &[&str], ) -> io::Result<()> { // +------------------------------------+-------------+ // A | bb4 | STATE | @@ -330,8 +335,6 @@ where // +-+----------------------------------+------+------+ // | | ... | | | - let state_column_names = self.state_formatter.column_names(); - // A write!( w, @@ -357,6 +360,56 @@ where write!(w, "") } + fn write_statements_and_terminator( + &mut self, + w: &mut impl io::Write, + body: &'a Body<'tcx>, + block: BasicBlock, + ) -> io::Result<()> { + let diffs = StateDiffCollector::run(body, block, self.results.results(), self.style); + + let mut befores = diffs.before.map(|v| v.into_iter()); + let mut afters = diffs.after.into_iter(); + + let next_in_dataflow_order = |it: &mut std::vec::IntoIter<_>| { + if A::Direction::is_forward() { it.next().unwrap() } else { it.next_back().unwrap() } + }; + + for (i, statement) in body[block].statements.iter().enumerate() { + let statement_str = format!("{:?}", statement); + let index_str = format!("{}", i); + + let after = next_in_dataflow_order(&mut afters); + let before = befores.as_mut().map(next_in_dataflow_order); + + self.write_row(w, &index_str, &statement_str, |_this, w, fmt| { + if let Some(before) = before { + write!(w, r#"{diff}"#, fmt = fmt, diff = before)?; + } + + write!(w, r#"{diff}"#, fmt = fmt, diff = after) + })?; + } + + let after = next_in_dataflow_order(&mut afters); + let before = befores.as_mut().map(next_in_dataflow_order); + + assert!(afters.is_empty()); + assert!(befores.as_ref().map_or(true, ExactSizeIterator::is_empty)); + + let terminator = body[block].terminator(); + let mut terminator_str = String::new(); + terminator.kind.fmt_head(&mut terminator_str).unwrap(); + + self.write_row(w, "T", &terminator_str, |_this, w, fmt| { + if let Some(before) = before { + write!(w, r#"{diff}"#, fmt = fmt, diff = before)?; + } + + write!(w, r#"{diff}"#, fmt = fmt, diff = after) + }) + } + /// Write a row with the given index and MIR, using the function argument to fill in the /// "STATE" column(s). fn write_row( @@ -397,319 +450,169 @@ where let state = this.results.get(); let analysis = this.results.analysis(); + // FIXME: The full state vector can be quite long. It would be nice to split on commas + // and use some text wrapping algorithm. write!( w, - r#"{{"#, - colspan = this.num_state_columns(), + r#"{state}"#, + colspan = this.style.num_state_columns(), fmt = fmt, - )?; - pretty_print_state_elems(w, analysis, state.iter(), ", ", LIMIT_30_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) + state = format!("{:?}", DebugWithAdapter { this: state, ctxt: analysis }), + ) }) } } -/// Controls what gets printed under the `STATE` header. -pub trait StateFormatter<'tcx, A> +struct StateDiffCollector<'a, '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<()>; + analysis: &'a A, + prev_state: A::Domain, + before: Option>, + after: Vec, } -/// Prints a single column containing the state vector immediately *after* each statement. -pub struct SimpleDiff<'a, 'tcx, A> +impl StateDiffCollector<'a, 'tcx, A> where A: Analysis<'tcx>, + A::Domain: DebugWithContext, { - prev_state: ResultsRefCursor<'a, 'a, 'tcx, A>, -} - -impl SimpleDiff<'a, 'tcx, A> -where - A: Analysis<'tcx>, -{ - pub fn new(body: &'a Body<'tcx>, results: &'a Results<'tcx, A>) -> Self { - SimpleDiff { prev_state: ResultsRefCursor::new(body, results) } - } -} - -impl StateFormatter<'tcx, A> for SimpleDiff<'_, 'tcx, A> -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 A::Direction::is_forward() { - if location.statement_index == 0 { - self.prev_state.seek_to_block_start(location.block); - } else { - self.prev_state.seek_after_primary_effect(Location { - statement_index: location.statement_index - 1, - ..location - }); - } - } else { - if location == results.body().terminator_loc(location.block) { - self.prev_state.seek_to_block_end(location.block); - } else { - self.prev_state.seek_after_primary_effect(location.successor_within_block()); - } - } - - write!(w, r#""#, fmt = fmt)?; - results.seek_after_primary_effect(location); - let curr_state = results.get(); - write_diff(&mut w, results.analysis(), self.prev_state.get(), 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 { - pub fn new(bits_per_block: usize) -> Self { - TwoPhaseDiff { prev_state: BitSet::new_empty(bits_per_block), prev_loc: Location::START } - } -} - -impl StateFormatter<'tcx, A> for TwoPhaseDiff -where - A: Analysis<'tcx>, -{ - fn column_names(&self) -> &[&str] { - &["BEFORE", " AFTER"] - } - - 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_entry(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); - } - - self.prev_loc = location; - - // Before - - write!(w, r#""#, fmt = fmt)?; - results.seek_before_primary_effect(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, "")?; - - // After - - write!(w, r#""#, fmt = fmt)?; - results.seek_after_primary_effect(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 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 BlockTransferFunc<'mir, 'tcx, T> { - pub fn new( - body: &'mir mir::Body<'tcx>, - trans_for_block: IndexVec>, + fn run( + body: &'a mir::Body<'tcx>, + block: BasicBlock, + results: &'a Results<'tcx, A>, + style: OutputStyle, ) -> 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(()); - } - - 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(), BR_LEFT, None)?; - write!(w, "")?; - } - - Ok(()) - } -} - -/// 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(A::Idx::new) { - match (from.contains(i), to.contains(i)) { - (false, true) => set.insert(i), - (true, false) => clear.insert(i), - _ => continue, + let mut collector = StateDiffCollector { + analysis: &results.analysis, + prev_state: results.analysis.bottom_value(body), + after: vec![], + before: (style == OutputStyle::BeforeAndAfter).then_some(vec![]), }; - } - if !set.is_empty() { - write!(w, r#"+"#)?; - pretty_print_state_elems(w, analysis, set.iter(), ", ", LIMIT_30_ALIGN_1)?; - write!(w, r#""#)?; + results.visit_with(body, std::iter::once(block), &mut collector); + collector } - - if !set.is_empty() && !clear.is_empty() { - write!(w, "{}", BR_LEFT)?; - } - - if !clear.is_empty() { - write!(w, r#"-"#)?; - pretty_print_state_elems(w, analysis, clear.iter(), ", ", LIMIT_30_ALIGN_1)?; - write!(w, r#""#)?; - } - - Ok(()) } -const BR_LEFT: &str = r#"
"#; -const BR_LEFT_SPACE: &str = r#"
"#; - -/// Line break policy that breaks at 40 characters and starts the next line with a single space. -const LIMIT_30_ALIGN_1: Option = Some(LineBreak { sequence: BR_LEFT_SPACE, limit: 30 }); - -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, - sep: &str, - line_break: Option, -) -> io::Result +impl ResultsVisitor<'a, 'tcx> for StateDiffCollector<'a, 'tcx, A> where A: Analysis<'tcx>, + A::Domain: DebugWithContext, { - let sep_width = sep.chars().count(); + type FlowState = A::Domain; - 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 { - 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 first { - first = false; - } else { - write!(w, "{}", sep)?; - curr_line_width += sep_width; - - 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; - } - } + fn visit_block_start( + &mut self, + state: &Self::FlowState, + _block_data: &'mir mir::BasicBlockData<'tcx>, + _block: BasicBlock, + ) { + if A::Direction::is_forward() { + self.prev_state.clone_from(state); } - - write!(w, "{}", escaped)?; - curr_line_width += escaped_width; } - Ok(line_break_inserted) + fn visit_block_end( + &mut self, + state: &Self::FlowState, + _block_data: &'mir mir::BasicBlockData<'tcx>, + _block: BasicBlock, + ) { + if A::Direction::is_backward() { + self.prev_state.clone_from(state); + } + } + + fn visit_statement_before_primary_effect( + &mut self, + state: &Self::FlowState, + _statement: &'mir mir::Statement<'tcx>, + _location: Location, + ) { + if let Some(before) = self.before.as_mut() { + before.push(diff_pretty(state, &self.prev_state, self.analysis)); + self.prev_state.clone_from(state) + } + } + + fn visit_statement_after_primary_effect( + &mut self, + state: &Self::FlowState, + _statement: &'mir mir::Statement<'tcx>, + _location: Location, + ) { + self.after.push(diff_pretty(state, &self.prev_state, self.analysis)); + self.prev_state.clone_from(state) + } + + fn visit_terminator_before_primary_effect( + &mut self, + state: &Self::FlowState, + _terminator: &'mir mir::Terminator<'tcx>, + _location: Location, + ) { + if let Some(before) = self.before.as_mut() { + before.push(diff_pretty(state, &self.prev_state, self.analysis)); + self.prev_state.clone_from(state) + } + } + + fn visit_terminator_after_primary_effect( + &mut self, + state: &Self::FlowState, + _terminator: &'mir mir::Terminator<'tcx>, + _location: Location, + ) { + self.after.push(diff_pretty(state, &self.prev_state, self.analysis)); + self.prev_state.clone_from(state) + } +} + +fn diff_pretty(new: T, old: T, ctxt: &C) -> String +where + T: DebugWithContext, +{ + if new == old { + return String::new(); + } + + let re = Regex::new("\u{001f}([+-])").unwrap(); + + let raw_diff = format!("{:#?}", DebugDiffWithAdapter { new, old, ctxt }); + + // Replace newlines in the `Debug` output with `
` + let raw_diff = raw_diff.replace('\n', r#"
"#); + + let mut inside_font_tag = false; + let html_diff = re.replace_all(&raw_diff, |captures: ®ex::Captures<'_>| { + let mut ret = String::new(); + if inside_font_tag { + ret.push_str(r#""#); + } + + let tag = match &captures[1] { + "+" => r#"+"#, + "-" => r#"-"#, + _ => unreachable!(), + }; + + inside_font_tag = true; + ret.push_str(tag); + ret + }); + + let mut html_diff = match html_diff { + Cow::Borrowed(_) => return raw_diff, + Cow::Owned(s) => s, + }; + + if inside_font_tag { + html_diff.push_str(""); + } + + html_diff } /// The background color used for zebra-striping the table. diff --git a/compiler/rustc_mir/src/dataflow/framework/lattice.rs b/compiler/rustc_mir/src/dataflow/framework/lattice.rs new file mode 100644 index 00000000000..9294e0cc453 --- /dev/null +++ b/compiler/rustc_mir/src/dataflow/framework/lattice.rs @@ -0,0 +1,196 @@ +//! Traits used to represent [lattices] for use as the domain of a dataflow analysis. +//! +//! ## Implementation Notes +//! +//! Given that they represent partially ordered sets, you may be surprised that [`MeetSemiLattice`] +//! and [`JoinSemiLattice`] do not have [`PartialOrd`][std::cmp::PartialOrd] as a supertrait. This +//! is because most standard library types use lexicographic ordering instead of [set inclusion] +//! for their `PartialOrd` impl. Since we do not actually need to compare lattice elements to run a +//! dataflow analysis, there's no need for a hypothetical `SetInclusion` newtype with a custom +//! `PartialOrd` impl. The only benefit would be the ability to check (in debug mode) that the +//! least upper (or greatest lower) bound returned by the lattice join (or meet) operator was in +//! fact greater (or lower) than the inputs. +//! +//! [lattices]: https://en.wikipedia.org/wiki/Lattice_(order) +//! [set inclusion]: https://en.wikipedia.org/wiki/Subset + +use rustc_index::bit_set::BitSet; +use rustc_index::vec::{Idx, IndexVec}; + +/// A [partially ordered set][poset] that has a [least upper bound][lub] for any pair of elements +/// in the set. +/// +/// [lub]: https://en.wikipedia.org/wiki/Infimum_and_supremum +/// [poset]: https://en.wikipedia.org/wiki/Partially_ordered_set +pub trait JoinSemiLattice: Eq { + /// Computes the least upper bound of two elements, storing the result in `self` and returning + /// `true` if `self` has changed. + /// + /// The lattice join operator is abbreviated as `∨`. + fn join(&mut self, other: &Self) -> bool; +} + +/// A [partially ordered set][poset] that has a [greatest lower bound][glb] for any pair of +/// elements in the set. +/// +/// Dataflow analyses only require that their domains implement [`JoinSemiLattice`], not +/// `MeetSemiLattice`. However, types that will be used as dataflow domains should implement both +/// so that they can be used with [`Dual`]. +/// +/// [glb]: https://en.wikipedia.org/wiki/Infimum_and_supremum +/// [poset]: https://en.wikipedia.org/wiki/Partially_ordered_set +pub trait MeetSemiLattice: Eq { + /// Computes the greatest lower bound of two elements, storing the result in `self` and + /// returning `true` if `self` has changed. + /// + /// The lattice meet operator is abbreviated as `∧`. + fn meet(&mut self, other: &Self) -> bool; +} + +/// A `bool` is a "two-point" lattice with `true` as the top element and `false` as the bottom. +impl JoinSemiLattice for bool { + fn join(&mut self, other: &Self) -> bool { + if let (false, true) = (*self, *other) { + *self = true; + return true; + } + + false + } +} + +impl MeetSemiLattice for bool { + fn meet(&mut self, other: &Self) -> bool { + if let (true, false) = (*self, *other) { + *self = false; + return true; + } + + false + } +} + +/// A tuple or list of lattices is itself a lattice whose least upper bound is the concatenation of +/// the least upper bounds of each element of the tuple or list. +impl JoinSemiLattice for IndexVec { + fn join(&mut self, other: &Self) -> bool { + assert_eq!(self.len(), other.len()); + + let mut changed = false; + for (a, b) in self.iter_mut().zip(other.iter()) { + changed |= a.join(b); + } + changed + } +} + +impl MeetSemiLattice for IndexVec { + fn meet(&mut self, other: &Self) -> bool { + assert_eq!(self.len(), other.len()); + + let mut changed = false; + for (a, b) in self.iter_mut().zip(other.iter()) { + changed |= a.meet(b); + } + changed + } +} + +/// A `BitSet` is an efficent way to store a tuple of "two-point" lattices. Equivalently, it is the +/// lattice corresponding to the powerset of the set of all possibe values of the index type `T` +/// ordered by inclusion. +impl JoinSemiLattice for BitSet { + fn join(&mut self, other: &Self) -> bool { + self.union(other) + } +} + +impl MeetSemiLattice for BitSet { + fn meet(&mut self, other: &Self) -> bool { + self.intersect(other) + } +} + +/// The counterpart of a given semilattice `T` using the [inverse order]. +/// +/// The dual of a join-semilattice is a meet-semilattice and vice versa. For example, the dual of a +/// powerset has the empty set as its top element and the full set as its bottom element and uses +/// set *intersection* as its join operator. +/// +/// [inverse order]: https://en.wikipedia.org/wiki/Duality_(order_theory) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Dual(pub T); + +impl std::borrow::Borrow for Dual { + fn borrow(&self) -> &T { + &self.0 + } +} + +impl std::borrow::BorrowMut for Dual { + fn borrow_mut(&mut self) -> &mut T { + &mut self.0 + } +} + +impl JoinSemiLattice for Dual { + fn join(&mut self, other: &Self) -> bool { + self.0.meet(&other.0) + } +} + +impl MeetSemiLattice for Dual { + fn meet(&mut self, other: &Self) -> bool { + self.0.join(&other.0) + } +} + +/// Extends a type `T` with top and bottom elements to make it a partially ordered set in which no +/// value of `T` is comparable with any other. A flat set has the following [Hasse +/// diagram](https://en.wikipedia.org/wiki/Hasse_diagram): +/// +/// ```text +/// top +/// / / \ \ +/// all possible values of `T` +/// \ \ / / +/// bottom +/// ``` +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FlatSet { + Bottom, + Elem(T), + Top, +} + +impl JoinSemiLattice for FlatSet { + fn join(&mut self, other: &Self) -> bool { + let result = match (&*self, other) { + (Self::Top, _) | (_, Self::Bottom) => return false, + (Self::Elem(a), Self::Elem(b)) if a == b => return false, + + (Self::Bottom, Self::Elem(x)) => Self::Elem(x.clone()), + + _ => Self::Top, + }; + + *self = result; + true + } +} + +impl MeetSemiLattice for FlatSet { + fn meet(&mut self, other: &Self) -> bool { + let result = match (&*self, other) { + (Self::Bottom, _) | (_, Self::Top) => return false, + (Self::Elem(ref a), Self::Elem(ref b)) if a == b => return false, + + (Self::Top, Self::Elem(ref x)) => Self::Elem(x.clone()), + + _ => Self::Bottom, + }; + + *self = result; + true + } +} diff --git a/compiler/rustc_mir/src/dataflow/framework/mod.rs b/compiler/rustc_mir/src/dataflow/framework/mod.rs index a21bbacb467..eba3b88d47e 100644 --- a/compiler/rustc_mir/src/dataflow/framework/mod.rs +++ b/compiler/rustc_mir/src/dataflow/framework/mod.rs @@ -30,8 +30,8 @@ //! //! [gen-kill]: https://en.wikipedia.org/wiki/Data-flow_analysis#Bit_vector_problems +use std::borrow::BorrowMut; use std::cmp::Ordering; -use std::io; use rustc_hir::def_id::DefId; use rustc_index::bit_set::{BitSet, HybridBitSet}; @@ -43,67 +43,24 @@ use rustc_target::abi::VariantIdx; mod cursor; mod direction; mod engine; +pub mod fmt; mod graphviz; +pub mod lattice; mod visitor; pub use self::cursor::{ResultsCursor, ResultsRefCursor}; pub use self::direction::{Backward, Direction, Forward}; pub use self::engine::{Engine, Results}; +pub use self::lattice::{JoinSemiLattice, MeetSemiLattice}; pub use self::visitor::{visit_results, ResultsVisitor}; pub use self::visitor::{BorrowckFlowState, BorrowckResults}; -/// Parameterization for the precise form of data flow that is used. -/// -/// `BottomValue` determines whether the initial entry set for each basic block is empty or full. -/// This also determines the semantics of the lattice `join` operator used to merge dataflow -/// results, since dataflow works by starting at the bottom and moving monotonically to a fixed -/// point. -/// -/// This means, for propagation across the graph, that you either want to start at all-zeroes and -/// then use Union as your merge when propagating, or you start at all-ones and then use Intersect -/// as your merge when propagating. -pub trait BottomValue { - /// Specifies the initial value for each bit in the entry set for each basic block. - const BOTTOM_VALUE: bool; - - /// Merges `in_set` into `inout_set`, returning `true` if `inout_set` changed. - /// - /// It is almost certainly wrong to override this, since it automatically applies - /// * `inout_set & in_set` if `BOTTOM_VALUE == true` - /// * `inout_set | in_set` if `BOTTOM_VALUE == false` - /// - /// This means that if a bit is not `BOTTOM_VALUE`, it is propagated into all target blocks. - /// For clarity, the above statement again from a different perspective: - /// A bit in the block's entry set is `!BOTTOM_VALUE` if *any* predecessor block's bit value is - /// `!BOTTOM_VALUE`. - /// - /// There are situations where you want the opposite behaviour: propagate only if *all* - /// predecessor blocks's value is `!BOTTOM_VALUE`. - /// E.g. if you want to know whether a bit is *definitely* set at a specific location. This - /// means that all code paths leading to the location must have set the bit, instead of any - /// code path leading there. - /// - /// If you want this kind of "definitely set" analysis, you need to - /// 1. Invert `BOTTOM_VALUE` - /// 2. Reset the `entry_set` in `start_block_effect` to `!BOTTOM_VALUE` - /// 3. Override `join` to do the opposite from what it's doing now. - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - if !Self::BOTTOM_VALUE { inout_set.union(in_set) } else { inout_set.intersect(in_set) } - } -} - /// 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; +/// This trait specifies the lattice on which this analysis operates (the domain) as well as its +/// initial value at the entry point of each basic block. +pub trait AnalysisDomain<'tcx> { + type Domain: Clone + JoinSemiLattice; /// The direction of this analyis. Either `Forward` or `Backward`. type Direction: Direction = Forward; @@ -114,8 +71,7 @@ pub trait AnalysisDomain<'tcx>: BottomValue { /// 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; + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain; /// Mutates the entry set of the `START_BLOCK` to contain the initial state for dataflow /// analysis. @@ -126,12 +82,7 @@ pub trait AnalysisDomain<'tcx>: BottomValue { // FIXME: For backward dataflow analyses, the initial state should be applied to every basic // block where control flow could exit the MIR body (e.g., those terminated with `return` or // `resume`). It's not obvious how to handle `yield` points in generators, however. - 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) - } + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut Self::Domain); } /// A dataflow problem with an arbitrarily complex transfer function. @@ -139,7 +90,7 @@ 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, + state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, ); @@ -152,7 +103,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// analyses should not implement this without implementing `apply_statement_effect`. fn apply_before_statement_effect( &self, - _state: &mut BitSet, + _state: &mut Self::Domain, _statement: &mir::Statement<'tcx>, _location: Location, ) { @@ -166,7 +117,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// initialized here. fn apply_terminator_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ); @@ -179,7 +130,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// analyses should not implement this without implementing `apply_terminator_effect`. fn apply_before_terminator_effect( &self, - _state: &mut BitSet, + _state: &mut Self::Domain, _terminator: &mir::Terminator<'tcx>, _location: Location, ) { @@ -192,7 +143,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// edges. fn apply_call_return_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, block: BasicBlock, func: &mir::Operand<'tcx>, args: &[mir::Operand<'tcx>], @@ -207,7 +158,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// By default, no effects happen. fn apply_yield_resume_effect( &self, - _state: &mut BitSet, + _state: &mut Self::Domain, _resume_block: BasicBlock, _resume_place: mir::Place<'tcx>, ) { @@ -222,7 +173,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// FIXME: This class of effects is not supported for backward dataflow analyses. fn apply_discriminant_switch_effect( &self, - _state: &mut BitSet, + _state: &mut Self::Domain, _block: BasicBlock, _enum_place: mir::Place<'tcx>, _adt: &ty::AdtDef, @@ -264,6 +215,8 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// /// `Analysis` is automatically implemented for all implementers of `GenKillAnalysis`. pub trait GenKillAnalysis<'tcx>: Analysis<'tcx> { + type Idx: Idx; + /// See `Analysis::apply_statement_effect`. fn statement_effect( &self, @@ -332,10 +285,11 @@ pub trait GenKillAnalysis<'tcx>: Analysis<'tcx> { impl Analysis<'tcx> for A where A: GenKillAnalysis<'tcx>, + A::Domain: GenKill + BorrowMut>, { fn apply_statement_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, statement: &mir::Statement<'tcx>, location: Location, ) { @@ -344,7 +298,7 @@ where fn apply_before_statement_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, statement: &mir::Statement<'tcx>, location: Location, ) { @@ -353,7 +307,7 @@ where fn apply_terminator_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ) { @@ -362,7 +316,7 @@ where fn apply_before_terminator_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ) { @@ -371,7 +325,7 @@ where fn apply_call_return_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, func: &mir::Operand<'tcx>, args: &[mir::Operand<'tcx>], @@ -382,7 +336,7 @@ where fn apply_yield_resume_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, resume_block: BasicBlock, resume_place: mir::Place<'tcx>, ) { @@ -391,7 +345,7 @@ where fn apply_discriminant_switch_effect( &self, - state: &mut BitSet, + state: &mut A::Domain, block: BasicBlock, enum_place: mir::Place<'tcx>, adt: &ty::AdtDef, @@ -450,7 +404,7 @@ pub trait GenKill { /// applied multiple times efficiently. When there are multiple calls to `gen` and/or `kill` for /// the same element, the most recent one takes precedence. #[derive(Clone)] -pub struct GenKillSet { +pub struct GenKillSet { gen: HybridBitSet, kill: HybridBitSet, } @@ -464,7 +418,6 @@ impl GenKillSet { } } - /// Applies this transfer function to the given state vector. pub fn apply(&self, state: &mut BitSet) { state.union(&self.gen); state.subtract(&self.kill); @@ -493,6 +446,16 @@ impl GenKill for BitSet { } } +impl GenKill for lattice::Dual> { + fn gen(&mut self, elem: T) { + self.0.insert(elem); + } + + fn kill(&mut self, elem: T) { + self.0.remove(elem); + } +} + // NOTE: DO NOT CHANGE VARIANT ORDER. The derived `Ord` impls rely on the current order. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum Effect { diff --git a/compiler/rustc_mir/src/dataflow/framework/tests.rs b/compiler/rustc_mir/src/dataflow/framework/tests.rs index 9349f5133a5..a5989121679 100644 --- a/compiler/rustc_mir/src/dataflow/framework/tests.rs +++ b/compiler/rustc_mir/src/dataflow/framework/tests.rs @@ -9,7 +9,6 @@ use rustc_middle::ty; use rustc_span::DUMMY_SP; use super::*; -use crate::dataflow::BottomValue; /// Creates a `mir::Body` with a few disconnected basic blocks. /// @@ -92,13 +91,13 @@ impl MockAnalysis<'tcx, D> { /// The entry set for each `BasicBlock` is the ID of that block offset by a fixed amount to /// avoid colliding with the statement/terminator effects. fn mock_entry_set(&self, bb: BasicBlock) -> BitSet { - let mut ret = BitSet::new_empty(self.bits_per_block(self.body)); + let mut ret = self.bottom_value(self.body); ret.insert(Self::BASIC_BLOCK_OFFSET + bb.index()); ret } fn mock_entry_sets(&self) -> IndexVec> { - let empty = BitSet::new_empty(self.bits_per_block(self.body)); + let empty = self.bottom_value(self.body); let mut ret = IndexVec::from_elem(empty, &self.body.basic_blocks()); for (bb, _) in self.body.basic_blocks().iter_enumerated() { @@ -130,7 +129,7 @@ impl MockAnalysis<'tcx, D> { /// would be `[102, 0, 1, 2, 3, 4]`. fn expected_state_at_target(&self, target: SeekTarget) -> BitSet { let block = target.block(); - let mut ret = BitSet::new_empty(self.bits_per_block(self.body)); + let mut ret = self.bottom_value(self.body); ret.insert(Self::BASIC_BLOCK_OFFSET + block.index()); let target = match target { @@ -161,21 +160,17 @@ impl MockAnalysis<'tcx, D> { } } -impl BottomValue for MockAnalysis<'tcx, D> { - const BOTTOM_VALUE: bool = false; -} - impl AnalysisDomain<'tcx> for MockAnalysis<'tcx, D> { - type Idx = usize; + type Domain = BitSet; type Direction = D; const NAME: &'static str = "mock"; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - Self::BASIC_BLOCK_OFFSET + body.basic_blocks().len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + BitSet::new_empty(Self::BASIC_BLOCK_OFFSET + body.basic_blocks().len()) } - fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut Self::Domain) { unimplemented!("This is never called since `MockAnalysis` is never iterated to fixpoint"); } } @@ -183,7 +178,7 @@ impl AnalysisDomain<'tcx> for MockAnalysis<'tcx, D> { impl Analysis<'tcx> for MockAnalysis<'tcx, D> { fn apply_statement_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, ) { @@ -193,7 +188,7 @@ impl Analysis<'tcx> for MockAnalysis<'tcx, D> { fn apply_before_statement_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, _statement: &mir::Statement<'tcx>, location: Location, ) { @@ -203,7 +198,7 @@ impl Analysis<'tcx> for MockAnalysis<'tcx, D> { fn apply_terminator_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, _terminator: &mir::Terminator<'tcx>, location: Location, ) { @@ -213,7 +208,7 @@ impl Analysis<'tcx> for MockAnalysis<'tcx, D> { fn apply_before_terminator_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, _terminator: &mir::Terminator<'tcx>, location: Location, ) { @@ -223,7 +218,7 @@ impl Analysis<'tcx> for MockAnalysis<'tcx, D> { fn apply_call_return_effect( &self, - _state: &mut BitSet, + _state: &mut Self::Domain, _block: BasicBlock, _func: &mir::Operand<'tcx>, _args: &[mir::Operand<'tcx>], diff --git a/compiler/rustc_mir/src/dataflow/framework/visitor.rs b/compiler/rustc_mir/src/dataflow/framework/visitor.rs index 257f3cb9a6d..82eb734ed06 100644 --- a/compiler/rustc_mir/src/dataflow/framework/visitor.rs +++ b/compiler/rustc_mir/src/dataflow/framework/visitor.rs @@ -1,4 +1,3 @@ -use rustc_index::bit_set::BitSet; use rustc_middle::mir::{self, BasicBlock, Location}; use super::{Analysis, Direction, Results}; @@ -139,16 +138,16 @@ impl<'tcx, A> ResultsVisitable<'tcx> for Results<'tcx, A> where A: Analysis<'tcx>, { - type FlowState = BitSet; + type FlowState = A::Domain; type Direction = A::Direction; fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState { - BitSet::new_empty(self.analysis.bits_per_block(body)) + self.analysis.bottom_value(body) } fn reset_to_block_entry(&self, state: &mut Self::FlowState, block: BasicBlock) { - state.overwrite(&self.entry_set_for_block(block)); + state.clone_from(&self.entry_set_for_block(block)); } fn reconstruct_before_statement_effect( @@ -217,11 +216,11 @@ macro_rules! impl_visitable { $( $A: Analysis<'tcx, Direction = D>, )* { type Direction = D; - type FlowState = $T<$( BitSet<$A::Idx> ),*>; + type FlowState = $T<$( $A::Domain ),*>; fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState { $T { - $( $field: BitSet::new_empty(self.$field.analysis.bits_per_block(body)) ),* + $( $field: self.$field.analysis.bottom_value(body) ),* } } @@ -230,7 +229,7 @@ macro_rules! impl_visitable { state: &mut Self::FlowState, block: BasicBlock, ) { - $( state.$field.overwrite(&self.$field.entry_set_for_block(block)); )* + $( state.$field.clone_from(&self.$field.entry_set_for_block(block)); )* } fn reconstruct_before_statement_effect( diff --git a/compiler/rustc_mir/src/dataflow/mod.rs b/compiler/rustc_mir/src/dataflow/mod.rs index a0c24636059..5575a97982f 100644 --- a/compiler/rustc_mir/src/dataflow/mod.rs +++ b/compiler/rustc_mir/src/dataflow/mod.rs @@ -5,9 +5,9 @@ use rustc_span::symbol::{sym, Symbol}; pub(crate) use self::drop_flag_effects::*; pub use self::framework::{ - visit_results, Analysis, AnalysisDomain, Backward, BorrowckFlowState, BorrowckResults, - BottomValue, Engine, Forward, GenKill, GenKillAnalysis, Results, ResultsCursor, - ResultsRefCursor, ResultsVisitor, + fmt, lattice, visit_results, Analysis, AnalysisDomain, Backward, BorrowckFlowState, + BorrowckResults, Engine, Forward, GenKill, GenKillAnalysis, JoinSemiLattice, Results, + ResultsCursor, ResultsRefCursor, ResultsVisitor, }; use self::move_paths::MoveData; diff --git a/compiler/rustc_mir/src/lib.rs b/compiler/rustc_mir/src/lib.rs index 2e3b5084635..42717f27384 100644 --- a/compiler/rustc_mir/src/lib.rs +++ b/compiler/rustc_mir/src/lib.rs @@ -14,6 +14,7 @@ Rust MIR: a lowered representation of Rust. #![feature(crate_visibility_modifier)] #![feature(decl_macro)] #![feature(drain_filter)] +#![feature(exact_size_is_empty)] #![feature(exhaustive_patterns)] #![feature(iter_order_by)] #![feature(never_type)] From b19b8ea6113a6e912d508c11a25567b05d7db54c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 28 Aug 2020 13:26:25 -0700 Subject: [PATCH 5/8] Update dataflow analyses to use new interface --- .../src/dataflow/impls/borrowed_locals.rs | 17 ++-- .../rustc_mir/src/dataflow/impls/borrows.rs | 26 +++--- .../src/dataflow/impls/init_locals.rs | 18 ++-- .../rustc_mir/src/dataflow/impls/liveness.rs | 18 ++-- compiler/rustc_mir/src/dataflow/impls/mod.rs | 85 +++++++------------ .../src/dataflow/impls/storage_liveness.rs | 33 +++---- .../src/transform/check_consts/resolver.rs | 18 ++-- .../rustc_mir/src/transform/rustc_peek.rs | 17 ++-- .../clippy_lints/src/redundant_clone.rs | 17 ++-- 9 files changed, 106 insertions(+), 143 deletions(-) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrowed_locals.rs b/compiler/rustc_mir/src/dataflow/impls/borrowed_locals.rs index a3fc51cad65..65e04ed6831 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrowed_locals.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrowed_locals.rs @@ -82,15 +82,15 @@ impl AnalysisDomain<'tcx> for MaybeBorrowedLocals where K: BorrowAnalysisKind<'tcx>, { - type Idx = Local; - + type Domain = BitSet; const NAME: &'static str = K::ANALYSIS_NAME; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls().len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = unborrowed + BitSet::new_empty(body.local_decls().len()) } - fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut Self::Domain) { // No locals are aliased on function entry } } @@ -99,6 +99,8 @@ impl GenKillAnalysis<'tcx> for MaybeBorrowedLocals where K: BorrowAnalysisKind<'tcx>, { + type Idx = Local; + fn statement_effect( &self, trans: &mut impl GenKill, @@ -128,11 +130,6 @@ where } } -impl BottomValue for MaybeBorrowedLocals { - // bottom = unborrowed - const BOTTOM_VALUE: bool = false; -} - /// A `Visitor` that defines the transfer function for `MaybeBorrowedLocals`. struct TransferFunction<'a, T, K> { trans: &'a mut T, diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index aeb7ffe3e3b..0be13b6ba81 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -8,9 +8,9 @@ use rustc_index::bit_set::BitSet; use crate::borrow_check::{ places_conflict, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, ToRegionVid, }; -use crate::dataflow::BottomValue; -use crate::dataflow::{self, GenKill}; +use crate::dataflow::{self, fmt::DebugWithContext, GenKill}; +use std::fmt; use std::rc::Rc; rustc_index::newtype_index! { @@ -227,25 +227,24 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { } impl<'tcx> dataflow::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> { - type Idx = BorrowIndex; + type Domain = BitSet; const NAME: &'static str = "borrows"; - fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { - self.borrow_set.len() * 2 + fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { + // bottom = nothing is reserved or activated yet; + BitSet::new_empty(self.borrow_set.len() * 2) } - fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut Self::Domain) { // no borrows of code region_scopes have been taken prior to // function execution, so this method has no effect. } - - fn pretty_print_idx(&self, w: &mut impl std::io::Write, idx: Self::Idx) -> std::io::Result<()> { - write!(w, "{:?}", self.location(idx)) - } } impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { + type Idx = BorrowIndex; + fn before_statement_effect( &self, trans: &mut impl GenKill, @@ -344,7 +343,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { } } -impl<'a, 'tcx> BottomValue for Borrows<'a, 'tcx> { - /// bottom = nothing is reserved or activated yet; - const BOTTOM_VALUE: bool = false; +impl DebugWithContext> for BorrowIndex { + fn fmt_with(&self, ctxt: &Borrows<'_, '_>, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", ctxt.location(*self)) + } } diff --git a/compiler/rustc_mir/src/dataflow/impls/init_locals.rs b/compiler/rustc_mir/src/dataflow/impls/init_locals.rs index 0e7cd1bb0e4..5da302cd1fd 100644 --- a/compiler/rustc_mir/src/dataflow/impls/init_locals.rs +++ b/compiler/rustc_mir/src/dataflow/impls/init_locals.rs @@ -2,7 +2,7 @@ //! //! A local will be maybe initialized if *any* projections of that local might be initialized. -use crate::dataflow::{self, BottomValue, GenKill}; +use crate::dataflow::{self, GenKill}; use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::{PlaceContext, Visitor}; @@ -10,21 +10,17 @@ use rustc_middle::mir::{self, BasicBlock, Local, Location}; pub struct MaybeInitializedLocals; -impl BottomValue for MaybeInitializedLocals { - /// bottom = uninit - const BOTTOM_VALUE: bool = false; -} - impl dataflow::AnalysisDomain<'tcx> for MaybeInitializedLocals { - type Idx = Local; + type Domain = BitSet; const NAME: &'static str = "maybe_init_locals"; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls.len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = uninit + BitSet::new_empty(body.local_decls.len()) } - fn initialize_start_block(&self, body: &mir::Body<'tcx>, entry_set: &mut BitSet) { + fn initialize_start_block(&self, body: &mir::Body<'tcx>, entry_set: &mut Self::Domain) { // Function arguments are initialized to begin with. for arg in body.args_iter() { entry_set.insert(arg); @@ -33,6 +29,8 @@ impl dataflow::AnalysisDomain<'tcx> for MaybeInitializedLocals { } impl dataflow::GenKillAnalysis<'tcx> for MaybeInitializedLocals { + type Idx = Local; + fn statement_effect( &self, trans: &mut impl GenKill, diff --git a/compiler/rustc_mir/src/dataflow/impls/liveness.rs b/compiler/rustc_mir/src/dataflow/impls/liveness.rs index 784b0bd9293..b0da28156d1 100644 --- a/compiler/rustc_mir/src/dataflow/impls/liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/liveness.rs @@ -2,7 +2,7 @@ use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::{self, Local, Location}; -use crate::dataflow::{AnalysisDomain, Backward, BottomValue, GenKill, GenKillAnalysis}; +use crate::dataflow::{AnalysisDomain, Backward, GenKill, GenKillAnalysis}; /// A [live-variable dataflow analysis][liveness]. /// @@ -22,27 +22,25 @@ impl MaybeLiveLocals { } } -impl BottomValue for MaybeLiveLocals { - // bottom = not live - const BOTTOM_VALUE: bool = false; -} - impl AnalysisDomain<'tcx> for MaybeLiveLocals { - type Idx = Local; + type Domain = BitSet; type Direction = Backward; const NAME: &'static str = "liveness"; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls.len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = not live + BitSet::new_empty(body.local_decls.len()) } - fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut Self::Domain) { // No variables are live until we observe a use } } impl GenKillAnalysis<'tcx> for MaybeLiveLocals { + type Idx = Local; + fn statement_effect( &self, trans: &mut impl GenKill, diff --git a/compiler/rustc_mir/src/dataflow/impls/mod.rs b/compiler/rustc_mir/src/dataflow/impls/mod.rs index 8975faec487..c42d5867856 100644 --- a/compiler/rustc_mir/src/dataflow/impls/mod.rs +++ b/compiler/rustc_mir/src/dataflow/impls/mod.rs @@ -13,7 +13,7 @@ use super::MoveDataParamEnv; use crate::util::elaborate_drops::DropFlagState; use super::move_paths::{HasMoveData, InitIndex, InitKind, MoveData, MovePathIndex}; -use super::{AnalysisDomain, BottomValue, GenKill, GenKillAnalysis}; +use super::{lattice, AnalysisDomain, GenKill, GenKillAnalysis}; use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; @@ -290,27 +290,25 @@ impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { } impl<'tcx> AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { - type Idx = MovePathIndex; - + type Domain = BitSet; const NAME: &'static str = "maybe_init"; - fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { - self.move_data().move_paths.len() + fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { + // bottom = uninitialized + BitSet::new_empty(self.move_data().move_paths.len()) } - fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut Self::Domain) { drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| { assert!(s == DropFlagState::Present); state.insert(path); }); } - - 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> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { + type Idx = MovePathIndex; + fn statement_effect( &self, trans: &mut impl GenKill, @@ -376,18 +374,18 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { } impl<'tcx> AnalysisDomain<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { - type Idx = MovePathIndex; + type Domain = BitSet; const NAME: &'static str = "maybe_uninit"; - fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { - self.move_data().move_paths.len() + fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { + // bottom = initialized (start_block_effect counters this at outset) + BitSet::new_empty(self.move_data().move_paths.len()) } // sets on_entry bits for Arg places - fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut Self::Domain) { // set all bits to 1 (uninit) before gathering counterevidence - assert!(self.bits_per_block(body) == state.domain_size()); state.insert_all(); drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| { @@ -395,13 +393,11 @@ impl<'tcx> AnalysisDomain<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { state.remove(path); }); } - - 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> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { + type Idx = MovePathIndex; + fn statement_effect( &self, trans: &mut impl GenKill, @@ -471,30 +467,30 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } impl<'a, 'tcx> AnalysisDomain<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { - type Idx = MovePathIndex; + /// Use set intersection as the join operator. + type Domain = lattice::Dual>; const NAME: &'static str = "definite_init"; - fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { - self.move_data().move_paths.len() + fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { + // bottom = initialized (start_block_effect counters this at outset) + lattice::Dual(BitSet::new_filled(self.move_data().move_paths.len())) } // sets on_entry bits for Arg places - fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut BitSet) { - state.clear(); + fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut Self::Domain) { + state.0.clear(); drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| { assert!(s == DropFlagState::Present); - state.insert(path); + state.0.insert(path); }); } - - 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> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> { + type Idx = MovePathIndex; + fn statement_effect( &self, trans: &mut impl GenKill, @@ -540,15 +536,16 @@ impl<'tcx> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> { } impl<'tcx> AnalysisDomain<'tcx> for EverInitializedPlaces<'_, 'tcx> { - type Idx = InitIndex; + type Domain = BitSet; const NAME: &'static str = "ever_init"; - fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { - self.move_data().inits.len() + fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { + // bottom = no initialized variables by default + BitSet::new_empty(self.move_data().inits.len()) } - fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet) { + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut Self::Domain) { for arg_init in 0..body.arg_count { state.insert(InitIndex::new(arg_init)); } @@ -556,6 +553,8 @@ impl<'tcx> AnalysisDomain<'tcx> for EverInitializedPlaces<'_, 'tcx> { } impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { + type Idx = InitIndex; + fn statement_effect( &self, trans: &mut impl GenKill, @@ -625,23 +624,3 @@ impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { } } } - -impl<'a, 'tcx> BottomValue for MaybeInitializedPlaces<'a, 'tcx> { - /// bottom = uninitialized - const BOTTOM_VALUE: bool = false; -} - -impl<'a, 'tcx> BottomValue for MaybeUninitializedPlaces<'a, 'tcx> { - /// bottom = initialized (start_block_effect counters this at outset) - const BOTTOM_VALUE: bool = false; -} - -impl<'a, 'tcx> BottomValue for DefinitelyInitializedPlaces<'a, 'tcx> { - /// bottom = initialized (start_block_effect counters this at outset) - const BOTTOM_VALUE: bool = true; -} - -impl<'a, 'tcx> BottomValue for EverInitializedPlaces<'a, 'tcx> { - /// bottom = no initialized variables by default - const BOTTOM_VALUE: bool = false; -} diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index 21623e3cad5..9250cd40847 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -1,6 +1,5 @@ pub use super::*; -use crate::dataflow::BottomValue; use crate::dataflow::{self, GenKill, Results, ResultsRefCursor}; use crate::util::storage::AlwaysLiveLocals; use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; @@ -19,15 +18,16 @@ impl MaybeStorageLive { } impl dataflow::AnalysisDomain<'tcx> for MaybeStorageLive { - type Idx = Local; + type Domain = BitSet; const NAME: &'static str = "maybe_storage_live"; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls.len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = dead + BitSet::new_empty(body.local_decls.len()) } - fn initialize_start_block(&self, body: &mir::Body<'tcx>, on_entry: &mut BitSet) { + fn initialize_start_block(&self, body: &mir::Body<'tcx>, on_entry: &mut Self::Domain) { assert_eq!(body.local_decls.len(), self.always_live_locals.domain_size()); for local in self.always_live_locals.iter() { on_entry.insert(local); @@ -40,6 +40,8 @@ impl dataflow::AnalysisDomain<'tcx> for MaybeStorageLive { } impl dataflow::GenKillAnalysis<'tcx> for MaybeStorageLive { + type Idx = Local; + fn statement_effect( &self, trans: &mut impl GenKill, @@ -74,11 +76,6 @@ impl dataflow::GenKillAnalysis<'tcx> for MaybeStorageLive { } } -impl BottomValue for MaybeStorageLive { - /// bottom = dead - const BOTTOM_VALUE: bool = false; -} - type BorrowedLocalsResults<'a, 'tcx> = ResultsRefCursor<'a, 'a, 'tcx, MaybeBorrowedLocals>; /// Dataflow analysis that determines whether each local requires storage at a @@ -101,15 +98,16 @@ impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> { } impl<'mir, 'tcx> dataflow::AnalysisDomain<'tcx> for MaybeRequiresStorage<'mir, 'tcx> { - type Idx = Local; + type Domain = BitSet; const NAME: &'static str = "requires_storage"; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls.len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = dead + BitSet::new_empty(body.local_decls.len()) } - fn initialize_start_block(&self, body: &mir::Body<'tcx>, on_entry: &mut BitSet) { + fn initialize_start_block(&self, body: &mir::Body<'tcx>, on_entry: &mut Self::Domain) { // The resume argument is live on function entry (we don't care about // the `self` argument) for arg in body.args_iter().skip(1) { @@ -119,6 +117,8 @@ impl<'mir, 'tcx> dataflow::AnalysisDomain<'tcx> for MaybeRequiresStorage<'mir, ' } impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tcx> { + type Idx = Local; + fn before_statement_effect( &self, trans: &mut impl GenKill, @@ -285,11 +285,6 @@ impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> { } } -impl<'mir, 'tcx> BottomValue for MaybeRequiresStorage<'mir, 'tcx> { - /// bottom = dead - const BOTTOM_VALUE: bool = false; -} - struct MoveVisitor<'a, 'mir, 'tcx, T> { borrowed_locals: &'a RefCell>, trans: &'a mut T, diff --git a/compiler/rustc_mir/src/transform/check_consts/resolver.rs b/compiler/rustc_mir/src/transform/check_consts/resolver.rs index b8104292aab..a00301952b3 100644 --- a/compiler/rustc_mir/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_mir/src/transform/check_consts/resolver.rs @@ -165,23 +165,19 @@ where } } -impl dataflow::BottomValue for FlowSensitiveAnalysis<'_, '_, '_, Q> { - const BOTTOM_VALUE: bool = false; -} - impl dataflow::AnalysisDomain<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q> where Q: Qualif, { - type Idx = Local; + type Domain = BitSet; const NAME: &'static str = Q::ANALYSIS_NAME; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls.len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + BitSet::new_empty(body.local_decls.len()) } - fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut BitSet) { + fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut Self::Domain) { self.transfer_function(state).initialize_state(); } } @@ -192,7 +188,7 @@ where { fn apply_statement_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, ) { @@ -201,7 +197,7 @@ where fn apply_terminator_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ) { @@ -210,7 +206,7 @@ where fn apply_call_return_effect( &self, - state: &mut BitSet, + state: &mut Self::Domain, block: BasicBlock, func: &mir::Operand<'tcx>, args: &[mir::Operand<'tcx>], diff --git a/compiler/rustc_mir/src/transform/rustc_peek.rs b/compiler/rustc_mir/src/transform/rustc_peek.rs index 00d269a4af8..07ce3100d89 100644 --- a/compiler/rustc_mir/src/transform/rustc_peek.rs +++ b/compiler/rustc_mir/src/transform/rustc_peek.rs @@ -1,4 +1,6 @@ -use rustc_ast as ast; +use std::borrow::Borrow; + +use rustc_ast::ast; use rustc_span::symbol::sym; use rustc_span::Span; use rustc_target::spec::abi::Abi; @@ -16,7 +18,7 @@ use crate::dataflow::impls::{ use crate::dataflow::move_paths::{HasMoveData, MoveData}; use crate::dataflow::move_paths::{LookupResult, MovePathIndex}; use crate::dataflow::MoveDataParamEnv; -use crate::dataflow::{Analysis, Results, ResultsCursor}; +use crate::dataflow::{Analysis, JoinSemiLattice, Results, ResultsCursor}; pub struct SanityCheck; @@ -248,25 +250,26 @@ pub trait RustcPeekAt<'tcx>: Analysis<'tcx> { &self, tcx: TyCtxt<'tcx>, place: mir::Place<'tcx>, - flow_state: &BitSet, + flow_state: &Self::Domain, call: PeekCall, ); } -impl<'tcx, A> RustcPeekAt<'tcx> for A +impl<'tcx, A, D> RustcPeekAt<'tcx> for A where - A: Analysis<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, + A: Analysis<'tcx, Domain = D> + HasMoveData<'tcx>, + D: JoinSemiLattice + Clone + Borrow>, { fn peek_at( &self, tcx: TyCtxt<'tcx>, place: mir::Place<'tcx>, - flow_state: &BitSet, + flow_state: &Self::Domain, call: PeekCall, ) { match self.move_data().rev_lookup.find(place.as_ref()) { LookupResult::Exact(peek_mpi) => { - let bit_state = flow_state.contains(peek_mpi); + let bit_state = flow_state.borrow().contains(peek_mpi); debug!("rustc_peek({:?} = &{:?}) bit_state: {}", call.arg, place, bit_state); if !bit_state { tcx.sess.span_err(call.span, "rustc_peek: bit not set"); diff --git a/src/tools/clippy/clippy_lints/src/redundant_clone.rs b/src/tools/clippy/clippy_lints/src/redundant_clone.rs index 7932be0d4b1..aec99cb4063 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_clone.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_clone.rs @@ -14,7 +14,6 @@ use rustc_middle::mir::{ visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _}, }; use rustc_middle::ty::{self, fold::TypeVisitor, Ty}; -use rustc_mir::dataflow::BottomValue; use rustc_mir::dataflow::{Analysis, AnalysisDomain, GenKill, GenKillAnalysis, ResultsCursor}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::{BytePos, Span}; @@ -411,14 +410,15 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor { struct MaybeStorageLive; impl<'tcx> AnalysisDomain<'tcx> for MaybeStorageLive { - type Idx = mir::Local; + type Domain = BitSet; const NAME: &'static str = "maybe_storage_live"; - fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { - body.local_decls.len() + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = dead + BitSet::new_empty(body.local_decls.len()) } - fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet) { + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut Self::Domain) { for arg in body.args_iter() { state.insert(arg); } @@ -426,6 +426,8 @@ impl<'tcx> AnalysisDomain<'tcx> for MaybeStorageLive { } impl<'tcx> GenKillAnalysis<'tcx> for MaybeStorageLive { + type Idx = mir::Local; + fn statement_effect(&self, trans: &mut impl GenKill, stmt: &mir::Statement<'tcx>, _: mir::Location) { match stmt.kind { mir::StatementKind::StorageLive(l) => trans.gen(l), @@ -454,11 +456,6 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeStorageLive { } } -impl BottomValue for MaybeStorageLive { - /// bottom = dead - const BOTTOM_VALUE: bool = false; -} - /// Collects the possible borrowers of each local. /// For example, `b = &a; c = &a;` will make `b` and (transitively) `c` /// possible borrowers of `a`. From c03eba2d0833a21205b00e4b71e920df709a2658 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 28 Aug 2020 14:20:14 -0700 Subject: [PATCH 6/8] Add `FIXME` for faster cached block transfer functions I've tried a few ways of implementing this, but each fell short. Adding an auxiliary `_Idx` associated type to `Analysis` that defaults to `!` but is overridden in the blanket impl of `Analysis` for `A: GenKillAnalysis` to `A::Idx` seems promising, but the trait solver is unable to prove equivalence between `A::Idx` and `A::_Idx` within the overridden version of `into_engine`. Without full-featured specialization, removing `into_engine` or splitting it into a different trait would have a significant ergonomic penalty. Alternatively, we could erase the index type and store a `GenKillSet` as well as a function pointer for transmuting between `&mut A::Domain` and `&mut BitSet` in the hopes that LLVM can devirtualize a simple function pointer better than the boxed closure. However, this is brittle, requires `unsafe` code, and doesn't work for index types that aren't the same size as a `u32` (e.g. `usize`) since `GenKillSet` stores a `HybridBitSet`, which may be a `Vec`. Perhaps safe transmute could help here? --- compiler/rustc_mir/src/dataflow/framework/engine.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_mir/src/dataflow/framework/engine.rs b/compiler/rustc_mir/src/dataflow/framework/engine.rs index 07f5ab5e7bd..d3ad42f6bbc 100644 --- a/compiler/rustc_mir/src/dataflow/framework/engine.rs +++ b/compiler/rustc_mir/src/dataflow/framework/engine.rs @@ -87,6 +87,11 @@ where analysis: A, /// Cached, cumulative transfer functions for each block. + // + // FIXME(ecstaticmorse): This boxed `Fn` trait object is invoked inside a tight loop for + // gen/kill problems on cyclic CFGs. This is not ideal, but it doesn't seem to degrade + // performance in practice. I've tried a few ways to avoid this, but they have downsides. See + // the message for the commit that added this FIXME for more information. apply_trans_for_block: Option>, } From e178a870367259b1f0b68658f4af96010d6ba9a4 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 30 Aug 2020 13:27:07 -0700 Subject: [PATCH 7/8] Expand documentation for the `lattice` module --- .../src/dataflow/framework/lattice.rs | 68 ++++++++++++++----- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_mir/src/dataflow/framework/lattice.rs b/compiler/rustc_mir/src/dataflow/framework/lattice.rs index 9294e0cc453..e7ef9267db5 100644 --- a/compiler/rustc_mir/src/dataflow/framework/lattice.rs +++ b/compiler/rustc_mir/src/dataflow/framework/lattice.rs @@ -1,18 +1,42 @@ //! Traits used to represent [lattices] for use as the domain of a dataflow analysis. //! -//! ## Implementation Notes +//! # Overview //! -//! Given that they represent partially ordered sets, you may be surprised that [`MeetSemiLattice`] -//! and [`JoinSemiLattice`] do not have [`PartialOrd`][std::cmp::PartialOrd] as a supertrait. This -//! is because most standard library types use lexicographic ordering instead of [set inclusion] -//! for their `PartialOrd` impl. Since we do not actually need to compare lattice elements to run a -//! dataflow analysis, there's no need for a hypothetical `SetInclusion` newtype with a custom -//! `PartialOrd` impl. The only benefit would be the ability to check (in debug mode) that the -//! least upper (or greatest lower) bound returned by the lattice join (or meet) operator was in -//! fact greater (or lower) than the inputs. +//! The most common lattice is a powerset of some set `S`, ordered by [set inclusion]. The [Hasse +//! diagram] for the powerset of a set with two elements (`X` and `Y`) is shown below. Note that +//! distinct elements at the same height in a Hasse diagram (e.g. `{X}` and `{Y}`) are +//! *incomparable*, not equal. +//! +//! ```text +//! {X, Y} <- top +//! / \ +//! {X} {Y} +//! \ / +//! {} <- bottom +//! +//! ``` +//! +//! The defining characteristic of a lattice—the one that differentiates it from a [partially +//! ordered set][poset]—is the existence of a *unique* least upper and greatest lower bound for +//! every pair of elements. The lattice join operator (`∨`) returns the least upper bound, and the +//! lattice meet operator (`∧`) returns the greatest lower bound. Types that implement one operator +//! but not the other are known as semilattices. Dataflow analysis only uses the join operator and +//! will work with any join-semilattice, but both should be specified when possible. +//! +//! ## `PartialOrd` +//! +//! Given that they represent partially ordered sets, you may be surprised that [`JoinSemiLattice`] +//! and [`MeetSemiLattice`] do not have [`PartialOrd`][std::cmp::PartialOrd] as a supertrait. This +//! is because most standard library types use lexicographic ordering instead of set inclusion for +//! their `PartialOrd` impl. Since we do not actually need to compare lattice elements to run a +//! dataflow analysis, there's no need for a newtype wrapper with a custom `PartialOrd` impl. The +//! only benefit would be the ability to check that the least upper (or greatest lower) bound +//! returned by the lattice join (or meet) operator was in fact greater (or lower) than the inputs. //! //! [lattices]: https://en.wikipedia.org/wiki/Lattice_(order) //! [set inclusion]: https://en.wikipedia.org/wiki/Subset +//! [Hasse diagram]: https://en.wikipedia.org/wiki/Hasse_diagram +//! [poset]: https://en.wikipedia.org/wiki/Partially_ordered_set use rustc_index::bit_set::BitSet; use rustc_index::vec::{Idx, IndexVec}; @@ -47,7 +71,13 @@ pub trait MeetSemiLattice: Eq { fn meet(&mut self, other: &Self) -> bool; } -/// A `bool` is a "two-point" lattice with `true` as the top element and `false` as the bottom. +/// A `bool` is a "two-point" lattice with `true` as the top element and `false` as the bottom: +/// +/// ```text +/// true +/// | +/// false +/// ``` impl JoinSemiLattice for bool { fn join(&mut self, other: &Self) -> bool { if let (false, true) = (*self, *other) { @@ -70,8 +100,11 @@ impl MeetSemiLattice for bool { } } -/// A tuple or list of lattices is itself a lattice whose least upper bound is the concatenation of -/// the least upper bounds of each element of the tuple or list. +/// A tuple (or list) of lattices is itself a lattice whose least upper bound is the concatenation +/// of the least upper bounds of each element of the tuple (or list). +/// +/// In other words: +/// (A₀, A₁, ..., Aₙ) ∨ (B₀, B₁, ..., Bₙ) = (A₀∨B₀, A₁∨B₁, ..., Aₙ∨Bₙ) impl JoinSemiLattice for IndexVec { fn join(&mut self, other: &Self) -> bool { assert_eq!(self.len(), other.len()); @@ -96,9 +129,9 @@ impl MeetSemiLattice for IndexVec { } } -/// A `BitSet` is an efficent way to store a tuple of "two-point" lattices. Equivalently, it is the -/// lattice corresponding to the powerset of the set of all possibe values of the index type `T` -/// ordered by inclusion. +/// A `BitSet` represents the lattice formed by the powerset of all possible values of +/// the index type `T` ordered by inclusion. Equivalently, it is a tuple of "two-point" lattices, +/// one for each possible value of `T`. impl JoinSemiLattice for BitSet { fn join(&mut self, other: &Self) -> bool { self.union(other) @@ -146,8 +179,7 @@ impl MeetSemiLattice for Dual { } /// Extends a type `T` with top and bottom elements to make it a partially ordered set in which no -/// value of `T` is comparable with any other. A flat set has the following [Hasse -/// diagram](https://en.wikipedia.org/wiki/Hasse_diagram): +/// value of `T` is comparable with any other. A flat set has the following [Hasse diagram]: /// /// ```text /// top @@ -156,6 +188,8 @@ impl MeetSemiLattice for Dual { /// \ \ / / /// bottom /// ``` +/// +/// [Hasse diagram]: https://en.wikipedia.org/wiki/Hasse_diagram #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum FlatSet { Bottom, From b015109ba9bce939beef2d3e4fa3597ea0db1f27 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 30 Aug 2020 14:18:49 -0700 Subject: [PATCH 8/8] Add documentation to the `Analysis` traits --- .../rustc_mir/src/dataflow/framework/mod.rs | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir/src/dataflow/framework/mod.rs b/compiler/rustc_mir/src/dataflow/framework/mod.rs index eba3b88d47e..eefa1395a62 100644 --- a/compiler/rustc_mir/src/dataflow/framework/mod.rs +++ b/compiler/rustc_mir/src/dataflow/framework/mod.rs @@ -60,9 +60,10 @@ pub use self::visitor::{BorrowckFlowState, BorrowckResults}; /// This trait specifies the lattice on which this analysis operates (the domain) as well as its /// initial value at the entry point of each basic block. pub trait AnalysisDomain<'tcx> { + /// The type that holds the dataflow state at any given point in the program. type Domain: Clone + JoinSemiLattice; - /// The direction of this analyis. Either `Forward` or `Backward`. + /// The direction of this analysis. Either `Forward` or `Backward`. type Direction: Direction = Forward; /// A descriptive name for this analysis. Used only for debugging. @@ -71,10 +72,10 @@ pub trait AnalysisDomain<'tcx> { /// suitable as part of a filename. const NAME: &'static str; + /// The initial value of the dataflow state upon entry to each basic block. fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain; - /// Mutates the entry set of the `START_BLOCK` to contain the initial state for dataflow - /// analysis. + /// Mutates the initial value of the dataflow state upon entry to the `START_BLOCK`. /// /// For backward analyses, initial state besides the bottom value is not yet supported. Trying /// to mutate the initial state will result in a panic. @@ -86,6 +87,21 @@ pub trait AnalysisDomain<'tcx> { } /// A dataflow problem with an arbitrarily complex transfer function. +/// +/// # Convergence +/// +/// When implementing this trait directly (not via [`GenKillAnalysis`]), it's possible to choose a +/// transfer function such that the analysis does not reach fixpoint. To guarantee convergence, +/// your transfer functions must maintain the following invariant: +/// +/// > If the dataflow state **before** some point in the program changes to be greater +/// than the prior state **before** that point, the dataflow state **after** that point must +/// also change to be greater than the prior state **after** that point. +/// +/// This invariant guarantees that the dataflow state at a given point in the program increases +/// monotonically until fixpoint is reached. Note that this monotonicity requirement only applies +/// to the same point in the program at different points in time. The dataflow state at a given +/// point in the program may or may not be greater than the state at any preceding point. pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// Updates the current dataflow state with the effect of evaluating a statement. fn apply_statement_effect(