From c8cbd4fc784e5d432c02b0dc14a592f112dab59f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 21 Jun 2019 12:39:01 -0700 Subject: [PATCH] Merge `BitSetOperator` and `InitialFlow` into one trait. Since the value of `InitialFlow` defines the semantics of the `join` operation, there's no reason to have seperate traits for each. We can add a default impl of `join` which branches based on `BOTTOM_VALUE`. This should get optimized away. --- src/librustc_data_structures/bit_set.rs | 5 -- .../dataflow/impls/borrowed_locals.rs | 15 +--- src/librustc_mir/dataflow/impls/borrows.rs | 19 ++--- src/librustc_mir/dataflow/impls/mod.rs | 74 ++++--------------- .../dataflow/impls/storage_liveness.rs | 15 +--- src/librustc_mir/dataflow/mod.rs | 38 +++++++--- 6 files changed, 51 insertions(+), 115 deletions(-) diff --git a/src/librustc_data_structures/bit_set.rs b/src/librustc_data_structures/bit_set.rs index 7a11ca07007..f4a60b4d2c1 100644 --- a/src/librustc_data_structures/bit_set.rs +++ b/src/librustc_data_structures/bit_set.rs @@ -273,11 +273,6 @@ fn next(&mut self) -> Option { } } -pub trait BitSetOperator { - /// Combine one bitset into another. - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool; -} - #[inline] fn bitwise(out_vec: &mut [Word], in_vec: &[Word], op: Op) -> bool where Op: Fn(Word, Word) -> Word diff --git a/src/librustc_mir/dataflow/impls/borrowed_locals.rs b/src/librustc_mir/dataflow/impls/borrowed_locals.rs index 87ee254fb74..0f7f37f2db8 100644 --- a/src/librustc_mir/dataflow/impls/borrowed_locals.rs +++ b/src/librustc_mir/dataflow/impls/borrowed_locals.rs @@ -83,18 +83,9 @@ fn propagate_call_return( } } -impl<'a, 'tcx> BitSetOperator for HaveBeenBorrowedLocals<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.union(in_set) // "maybe" means we union effects of both preds - } -} - -impl<'a, 'tcx> InitialFlow for HaveBeenBorrowedLocals<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - false // bottom = unborrowed - } +impl<'a, 'tcx> BottomValue for HaveBeenBorrowedLocals<'a, 'tcx> { + // bottom = unborrowed + const BOTTOM_VALUE: bool = false; } struct BorrowedLocalsVisitor<'gk> { diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index b29a4f351b5..53d00d44e3f 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -5,11 +5,11 @@ use rustc::ty::TyCtxt; use rustc::ty::RegionVid; -use rustc_data_structures::bit_set::{BitSet, BitSetOperator}; +use rustc_data_structures::bit_set::BitSet; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; -use crate::dataflow::{BitDenotation, InitialFlow, GenKillSet}; +use crate::dataflow::{BitDenotation, BottomValue, GenKillSet}; use crate::borrow_check::nll::region_infer::RegionInferenceContext; use crate::borrow_check::nll::ToRegionVid; use crate::borrow_check::places_conflict; @@ -331,16 +331,7 @@ fn propagate_call_return( } } -impl<'a, 'tcx> BitSetOperator for Borrows<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.union(in_set) // "maybe" means we union effects of both preds - } -} - -impl<'a, 'tcx> InitialFlow for Borrows<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - false // bottom = nothing is reserved or activated yet - } +impl<'a, 'tcx> BottomValue for Borrows<'a, 'tcx> { + /// bottom = nothing is reserved or activated yet; + const BOTTOM_VALUE: bool = false; } diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index ad92c8783d3..065cfe8a4e8 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -4,7 +4,7 @@ use rustc::ty::TyCtxt; use rustc::mir::{self, Body, Location}; -use rustc_data_structures::bit_set::{BitSet, BitSetOperator}; +use rustc_data_structures::bit_set::BitSet; use rustc_data_structures::indexed_vec::Idx; use super::MoveDataParamEnv; @@ -12,7 +12,7 @@ use crate::util::elaborate_drops::DropFlagState; use super::move_paths::{HasMoveData, MoveData, MovePathIndex, InitIndex, InitKind}; -use super::{BitDenotation, InitialFlow, GenKillSet}; +use super::{BitDenotation, BottomValue, GenKillSet}; use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; @@ -505,68 +505,22 @@ fn propagate_call_return( } } -impl<'a, 'tcx> BitSetOperator for MaybeInitializedPlaces<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.union(in_set) // "maybe" means we union effects of both preds - } +impl<'a, 'tcx> BottomValue for MaybeInitializedPlaces<'a, 'tcx> { + /// bottom = uninitialized + const BOTTOM_VALUE: bool = false; } -impl<'a, 'tcx> BitSetOperator for MaybeUninitializedPlaces<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.union(in_set) // "maybe" means we union effects of both preds - } +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> BitSetOperator for DefinitelyInitializedPlaces<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.intersect(in_set) // "definitely" means we intersect effects of both preds - } +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> BitSetOperator for EverInitializedPlaces<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.union(in_set) // inits from both preds are in scope - } -} - -// The way that dataflow fixed point iteration works, you want to -// start at bottom and work your way to a fixed point. Control-flow -// merges will apply the `join` operator to each block entry's current -// state (which starts at that bottom value). -// -// 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. - -impl<'a, 'tcx> InitialFlow for MaybeInitializedPlaces<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - false // bottom = uninitialized - } -} - -impl<'a, 'tcx> InitialFlow for MaybeUninitializedPlaces<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - false // bottom = initialized (start_block_effect counters this at outset) - } -} - -impl<'a, 'tcx> InitialFlow for DefinitelyInitializedPlaces<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - true // bottom = initialized (start_block_effect counters this at outset) - } -} - -impl<'a, 'tcx> InitialFlow for EverInitializedPlaces<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - false // bottom = no initialized variables by default - } +impl<'a, 'tcx> BottomValue for EverInitializedPlaces<'a, 'tcx> { + /// bottom = no initialized variables by default + const BOTTOM_VALUE: bool = false; } diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index a717a3947e5..d2003993d45 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -59,16 +59,7 @@ fn propagate_call_return( } } -impl<'a, 'tcx> BitSetOperator for MaybeStorageLive<'a, 'tcx> { - #[inline] - fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { - inout_set.union(in_set) // "maybe" means we union effects of both preds - } -} - -impl<'a, 'tcx> InitialFlow for MaybeStorageLive<'a, 'tcx> { - #[inline] - fn bottom_value() -> bool { - false // bottom = dead - } +impl<'a, 'tcx> BottomValue for MaybeStorageLive<'a, 'tcx> { + /// bottom = dead + const BOTTOM_VALUE: bool = false; } diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index d4a0ec78c5b..444f73d7356 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -1,7 +1,7 @@ use syntax::ast::{self, MetaItem}; use syntax::symbol::{Symbol, sym}; -use rustc_data_structures::bit_set::{BitSet, BitSetOperator, HybridBitSet}; +use rustc_data_structures::bit_set::{BitSet, HybridBitSet}; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::work_queue::WorkQueue; @@ -552,14 +552,28 @@ pub fn kill_set_for(&self, block_idx: usize) -> &HybridBitSet { } /// Parameterization for the precise form of data flow that is used. -/// `InitialFlow` handles initializing the bitvectors before any -/// code is inspected by the analysis. Analyses that need more nuanced -/// initialization (e.g., they need to consult the results of some other -/// dataflow analysis to set up the initial bitvectors) should not -/// implement this. -pub trait InitialFlow { - /// Specifies the initial value for each bit in the `on_entry` set - fn bottom_value() -> bool; +/// +/// `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. + #[inline] + fn join(&self, inout_set: &mut BitSet, in_set: &BitSet) -> bool { + if Self::BOTTOM_VALUE == false { + inout_set.union(in_set) + } else { + inout_set.intersect(in_set) + } + } } /// A specific flavor of dataflow analysis. @@ -567,10 +581,10 @@ pub trait InitialFlow { /// To run a dataflow analysis, one sets up an initial state for the /// `START_BLOCK` via `start_block_effect` and a transfer function (`trans`) /// for each block individually. The entry set for all other basic blocks is -/// initialized to `InitialFlow::bottom_value`. The dataflow analysis then +/// initialized to `Self::BOTTOM_VALUE`. The dataflow analysis then /// iteratively modifies the various entry sets (but leaves the the transfer /// function unchanged). -pub trait BitDenotation<'tcx>: BitSetOperator + InitialFlow { +pub trait BitDenotation<'tcx>: BottomValue { /// Specifies what index type is used to access the bitvector. type Idx: Idx; @@ -688,7 +702,7 @@ pub fn new(body: &'a Body<'tcx>, let bits_per_block = denotation.bits_per_block(); let num_blocks = body.basic_blocks().len(); - let on_entry = if D::bottom_value() { + let on_entry = if D::BOTTOM_VALUE == true { vec![BitSet::new_filled(bits_per_block); num_blocks] } else { vec![BitSet::new_empty(bits_per_block); num_blocks]