From bb183e2f17793886ffab4f2c9c2374830dc7861d Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 14 May 2016 03:25:44 +0300 Subject: [PATCH 01/24] Distinct CFG type for MIR, traversals to librustc --- src/librustc/lib.rs | 3 + src/librustc/mir/cfg.rs | 146 ++++++++++++++++++ src/librustc/mir/repr.rs | 14 +- .../mir/{transform.rs => transform/mod.rs} | 0 .../mir}/traversal.rs | 6 +- src/librustc/mir/visit.rs | 6 +- .../borrowck/mir/dataflow/mod.rs | 4 +- src/librustc_borrowck/borrowck/mir/patch.rs | 12 +- src/librustc_data_structures/bitvec.rs | 16 ++ src/librustc_mir/build/cfg.rs | 85 +--------- src/librustc_mir/build/mod.rs | 6 +- src/librustc_mir/build/scope.rs | 3 +- src/librustc_mir/lib.rs | 1 - src/librustc_mir/transform/add_call_guards.rs | 9 +- src/librustc_mir/transform/promote_consts.rs | 16 +- src/librustc_mir/transform/qualify_consts.rs | 4 +- .../transform/remove_dead_blocks.rs | 8 +- src/librustc_mir/transform/simplify_cfg.rs | 9 +- src/librustc_mir/transform/type_check.rs | 2 +- src/librustc_trans/mir/analyze.rs | 2 +- src/librustc_trans/mir/mod.rs | 2 +- 21 files changed, 224 insertions(+), 130 deletions(-) create mode 100644 src/librustc/mir/cfg.rs rename src/librustc/mir/{transform.rs => transform/mod.rs} (100%) rename src/{librustc_mir => librustc/mir}/traversal.rs (98%) diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index e1fb701e641bf..0932f1320e8f7 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -24,6 +24,7 @@ #![cfg_attr(not(stage0), deny(warnings))] #![feature(associated_consts)] +#![feature(inclusive_range_syntax)] #![feature(box_patterns)] #![feature(box_syntax)] #![feature(collections)] @@ -107,6 +108,8 @@ pub mod mir { pub mod visit; pub mod transform; pub mod mir_map; + pub mod cfg; + pub mod traversal; } pub mod session; diff --git a/src/librustc/mir/cfg.rs b/src/librustc/mir/cfg.rs new file mode 100644 index 0000000000000..3b6df96ec4fe6 --- /dev/null +++ b/src/librustc/mir/cfg.rs @@ -0,0 +1,146 @@ +use mir::repr::*; + +use std::ops::{Index, IndexMut}; +use syntax::codemap::Span; + +#[derive(Clone, RustcEncodable, RustcDecodable)] +pub struct CFG<'tcx> { + pub basic_blocks: Vec>, +} + +pub struct PredecessorIter(::std::vec::IntoIter); +impl Iterator for PredecessorIter { + type Item = BasicBlock; + fn next(&mut self) -> Option { + self.0.next() + } +} + +pub struct SuccessorIter(::std::vec::IntoIter); +impl<'a> Iterator for SuccessorIter { + type Item = BasicBlock; + fn next(&mut self) -> Option { + self.0.next() + } +} + +pub struct SuccessorIterMut<'a>(::std::vec::IntoIter<&'a mut BasicBlock>); +impl<'a> Iterator for SuccessorIterMut<'a> { + type Item = &'a mut BasicBlock; + fn next(&mut self) -> Option<&'a mut BasicBlock> { + self.0.next() + } +} + +impl<'tcx> CFG<'tcx> { + pub fn len(&self) -> usize { + self.basic_blocks.len() + } + + pub fn predecessors(&self, b: BasicBlock) -> PredecessorIter { + let mut preds = vec![]; + for idx in 0..self.len() { + let bb = BasicBlock::new(idx); + if let Some(_) = self.successors(bb).find(|&x| x == b) { + preds.push(bb) + } + } + PredecessorIter(preds.into_iter()) + } + + pub fn successors(&self, b: BasicBlock) -> SuccessorIter { + let v: Vec = self[b].terminator().kind.successors().into_owned(); + SuccessorIter(v.into_iter()) + } + + pub fn successors_mut(&mut self, b: BasicBlock) -> SuccessorIterMut { + SuccessorIterMut(self[b].terminator_mut().kind.successors_mut().into_iter()) + } + + + pub fn swap(&mut self, b1: BasicBlock, b2: BasicBlock) { + // TODO: find all the branches to b2 from subgraph starting at b2 and replace them with b1. + self.basic_blocks.swap(b1.index(), b2.index()); + } + + pub fn start_new_block(&mut self) -> BasicBlock { + let node_index = self.basic_blocks.len(); + self.basic_blocks.push(BasicBlockData::new(None)); + BasicBlock::new(node_index) + } + + pub fn start_new_cleanup_block(&mut self) -> BasicBlock { + let bb = self.start_new_block(); + self[bb].is_cleanup = true; + bb + } + + pub fn push(&mut self, block: BasicBlock, statement: Statement<'tcx>) { + debug!("push({:?}, {:?})", block, statement); + self[block].statements.push(statement); + } + + pub fn terminate(&mut self, + block: BasicBlock, + scope: ScopeId, + span: Span, + kind: TerminatorKind<'tcx>) { + debug_assert!(self[block].terminator.is_none(), + "terminate: block {:?} already has a terminator set", block); + self[block].terminator = Some(Terminator { + span: span, + scope: scope, + kind: kind, + }); + } + + pub fn push_assign(&mut self, + block: BasicBlock, + scope: ScopeId, + span: Span, + lvalue: &Lvalue<'tcx>, + rvalue: Rvalue<'tcx>) { + self.push(block, Statement { + scope: scope, + span: span, + kind: StatementKind::Assign(lvalue.clone(), rvalue) + }); + } + + pub fn push_assign_constant(&mut self, + block: BasicBlock, + scope: ScopeId, + span: Span, + temp: &Lvalue<'tcx>, + constant: Constant<'tcx>) { + self.push_assign(block, scope, span, temp, + Rvalue::Use(Operand::Constant(constant))); + } + + pub fn push_assign_unit(&mut self, + block: BasicBlock, + scope: ScopeId, + span: Span, + lvalue: &Lvalue<'tcx>) { + self.push_assign(block, scope, span, lvalue, Rvalue::Aggregate( + AggregateKind::Tuple, vec![] + )); + } +} + +impl<'tcx> Index for CFG<'tcx> { + type Output = BasicBlockData<'tcx>; + + #[inline] + fn index(&self, index: BasicBlock) -> &BasicBlockData<'tcx> { + &self.basic_blocks[index.index()] + } +} + +impl<'tcx> IndexMut for CFG<'tcx> { + #[inline] + fn index_mut(&mut self, index: BasicBlock) -> &mut BasicBlockData<'tcx> { + &mut self.basic_blocks[index.index()] + } +} + diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index 9666741d032b8..adfe8ad5c6ae6 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +pub use mir::cfg::*; + use graphviz::IntoCow; use middle::const_val::ConstVal; use rustc_const_math::{ConstUsize, ConstInt, ConstMathErr}; @@ -30,7 +32,7 @@ use syntax::codemap::Span; pub struct Mir<'tcx> { /// List of basic blocks. References to basic block use a newtyped index type `BasicBlock` /// that indexes into this vector. - pub basic_blocks: Vec>, + pub cfg: CFG<'tcx>, /// List of lexical scopes; these are referenced by statements and /// used (eventually) for debuginfo. Indexed by a `ScopeId`. @@ -70,17 +72,17 @@ pub const START_BLOCK: BasicBlock = BasicBlock(0); impl<'tcx> Mir<'tcx> { pub fn all_basic_blocks(&self) -> Vec { - (0..self.basic_blocks.len()) + (0..self.cfg.len()) .map(|i| BasicBlock::new(i)) .collect() } pub fn basic_block_data(&self, bb: BasicBlock) -> &BasicBlockData<'tcx> { - &self.basic_blocks[bb.index()] + &self.cfg[bb] } pub fn basic_block_data_mut(&mut self, bb: BasicBlock) -> &mut BasicBlockData<'tcx> { - &mut self.basic_blocks[bb.index()] + &mut self.cfg[bb] } } @@ -611,7 +613,7 @@ impl<'tcx> Debug for Statement<'tcx> { /// A path to a value; something that can be evaluated without /// changing or disturbing program state. -#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable)] +#[derive(Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] pub enum Lvalue<'tcx> { /// local variable declared by the user Var(u32), @@ -796,7 +798,7 @@ pub struct ScopeData { /// These are values that can appear inside an rvalue (or an index /// lvalue). They are intentionally limited to prevent rvalues from /// being nested in one another. -#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable)] +#[derive(Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] pub enum Operand<'tcx> { Consume(Lvalue<'tcx>), Constant(Constant<'tcx>), diff --git a/src/librustc/mir/transform.rs b/src/librustc/mir/transform/mod.rs similarity index 100% rename from src/librustc/mir/transform.rs rename to src/librustc/mir/transform/mod.rs diff --git a/src/librustc_mir/traversal.rs b/src/librustc/mir/traversal.rs similarity index 98% rename from src/librustc_mir/traversal.rs rename to src/librustc/mir/traversal.rs index c58b5c8772461..8e130721db854 100644 --- a/src/librustc_mir/traversal.rs +++ b/src/librustc/mir/traversal.rs @@ -12,7 +12,7 @@ use std::vec; use rustc_data_structures::bitvec::BitVector; -use rustc::mir::repr::*; +use mir::repr::*; /// Preorder traversal of a graph. /// @@ -44,7 +44,7 @@ impl<'a, 'tcx> Preorder<'a, 'tcx> { Preorder { mir: mir, - visited: BitVector::new(mir.basic_blocks.len()), + visited: BitVector::new(mir.cfg.basic_blocks.len()), worklist: worklist } } @@ -106,7 +106,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> { pub fn new(mir: &'a Mir<'tcx>, root: BasicBlock) -> Postorder<'a, 'tcx> { let mut po = Postorder { mir: mir, - visited: BitVector::new(mir.basic_blocks.len()), + visited: BitVector::new(mir.cfg.basic_blocks.len()), visit_stack: Vec::new() }; diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 5c9582b945bb8..d06b3550d5278 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -247,7 +247,7 @@ macro_rules! make_mir_visitor { fn super_mir(&mut self, mir: & $($mutability)* Mir<'tcx>) { let Mir { - ref $($mutability)* basic_blocks, + ref $($mutability)* cfg, ref $($mutability)* scopes, promoted: _, // Visited by passes separately. ref $($mutability)* return_ty, @@ -258,6 +258,10 @@ macro_rules! make_mir_visitor { ref $($mutability)* span, } = *mir; + let CFG { + ref $($mutability)* basic_blocks + } = *cfg; + for (index, data) in basic_blocks.into_iter().enumerate() { let block = BasicBlock::new(index); self.visit_basic_block_data(block, data); diff --git a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs index 81655b5e386f6..2f6a9e1b74b7f 100644 --- a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs @@ -112,7 +112,7 @@ impl<'b, 'a: 'b, 'tcx: 'a, BD> PropagationContext<'b, 'a, 'tcx, BD> fn walk_cfg(&mut self, in_out: &mut IdxSet) { let mir = self.builder.mir; - for (bb_idx, bb_data) in mir.basic_blocks.iter().enumerate() { + for (bb_idx, bb_data) in mir.cfg.basic_blocks.iter().enumerate() { let builder = &mut self.builder; { let sets = builder.flow_state.sets.for_block(bb_idx); @@ -396,7 +396,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> // (now rounded up to multiple of word size) let bits_per_block = words_per_block * usize_bits; - let num_blocks = mir.basic_blocks.len(); + let num_blocks = mir.cfg.basic_blocks.len(); let num_overall = num_blocks * bits_per_block; let zeroes = Bits::new(IdxSetBuf::new_empty(num_overall)); diff --git a/src/librustc_borrowck/borrowck/mir/patch.rs b/src/librustc_borrowck/borrowck/mir/patch.rs index b390c19af1a5b..2326f2b275960 100644 --- a/src/librustc_borrowck/borrowck/mir/patch.rs +++ b/src/librustc_borrowck/borrowck/mir/patch.rs @@ -32,7 +32,7 @@ impl<'tcx> MirPatch<'tcx> { pub fn new(mir: &Mir<'tcx>) -> Self { let mut result = MirPatch { patch_map: iter::repeat(None) - .take(mir.basic_blocks.len()).collect(), + .take(mir.cfg.basic_blocks.len()).collect(), new_blocks: vec![], new_temps: vec![], new_statements: vec![], @@ -86,7 +86,7 @@ impl<'tcx> MirPatch<'tcx> { } pub fn terminator_loc(&self, mir: &Mir<'tcx>, bb: BasicBlock) -> Location { - let offset = match bb.index().checked_sub(mir.basic_blocks.len()) { + let offset = match bb.index().checked_sub(mir.cfg.basic_blocks.len()) { Some(index) => self.new_blocks[index].statements.len(), None => mir.basic_block_data(bb).statements.len() }; @@ -131,13 +131,13 @@ impl<'tcx> MirPatch<'tcx> { debug!("MirPatch: {:?} new temps, starting from index {}: {:?}", self.new_temps.len(), mir.temp_decls.len(), self.new_temps); debug!("MirPatch: {} new blocks, starting from index {}", - self.new_blocks.len(), mir.basic_blocks.len()); - mir.basic_blocks.extend(self.new_blocks); + self.new_blocks.len(), mir.cfg.basic_blocks.len()); + mir.cfg.basic_blocks.extend(self.new_blocks); mir.temp_decls.extend(self.new_temps); for (src, patch) in self.patch_map.into_iter().enumerate() { if let Some(patch) = patch { debug!("MirPatch: patching block {:?}", src); - mir.basic_blocks[src].terminator_mut().kind = patch; + mir.cfg.basic_blocks[src].terminator_mut().kind = patch; } } @@ -175,7 +175,7 @@ impl<'tcx> MirPatch<'tcx> { } pub fn context_for_location(&self, mir: &Mir, loc: Location) -> (Span, ScopeId) { - let data = match loc.block.index().checked_sub(mir.basic_blocks.len()) { + let data = match loc.block.index().checked_sub(mir.cfg.basic_blocks.len()) { Some(new) => &self.new_blocks[new], None => mir.basic_block_data(loc.block) }; diff --git a/src/librustc_data_structures/bitvec.rs b/src/librustc_data_structures/bitvec.rs index 79d3f0cf68846..c8981555813ec 100644 --- a/src/librustc_data_structures/bitvec.rs +++ b/src/librustc_data_structures/bitvec.rs @@ -27,6 +27,12 @@ impl BitVector { (self.data[word] & mask) != 0 } + pub fn clear(&mut self) { + for datum in &mut self.data { + *datum = 0; + } + } + /// Returns true if the bit has changed. pub fn insert(&mut self, bit: usize) -> bool { let (word, mask) = word_mask(bit); @@ -37,6 +43,16 @@ impl BitVector { new_value != value } + /// Returns true if the bit has changed. + pub fn remove(&mut self, bit: usize) -> bool { + let (word, mask) = word_mask(bit); + let data = &mut self.data[word]; + let value = *data; + let new_value = value & !mask; + *data = new_value; + new_value != value + } + pub fn insert_all(&mut self, all: &BitVector) -> bool { assert!(self.data.len() == all.data.len()); let mut changed = false; diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs index 4859257f291c9..041b626b73276 100644 --- a/src/librustc_mir/build/cfg.rs +++ b/src/librustc_mir/build/cfg.rs @@ -8,90 +8,19 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - - - //! Routines for manipulating the control-flow graph. -use build::{CFG, Location}; +use build::Location; use rustc::mir::repr::*; -use syntax::codemap::Span; - -impl<'tcx> CFG<'tcx> { - pub fn block_data(&self, blk: BasicBlock) -> &BasicBlockData<'tcx> { - &self.basic_blocks[blk.index()] - } - pub fn block_data_mut(&mut self, blk: BasicBlock) -> &mut BasicBlockData<'tcx> { - &mut self.basic_blocks[blk.index()] - } - - pub fn start_new_block(&mut self) -> BasicBlock { - let node_index = self.basic_blocks.len(); - self.basic_blocks.push(BasicBlockData::new(None)); - BasicBlock::new(node_index) - } - - pub fn start_new_cleanup_block(&mut self) -> BasicBlock { - let bb = self.start_new_block(); - self.block_data_mut(bb).is_cleanup = true; - bb - } +pub trait CfgExt<'tcx> { + fn current_location(&mut self, block: BasicBlock) -> Location; - pub fn push(&mut self, block: BasicBlock, statement: Statement<'tcx>) { - debug!("push({:?}, {:?})", block, statement); - self.block_data_mut(block).statements.push(statement); - } +} - pub fn current_location(&mut self, block: BasicBlock) -> Location { - let index = self.block_data(block).statements.len(); +impl<'tcx> CfgExt<'tcx> for CFG<'tcx> { + fn current_location(&mut self, block: BasicBlock) -> Location { + let index = self[block].statements.len(); Location { block: block, statement_index: index } } - - pub fn push_assign(&mut self, - block: BasicBlock, - scope: ScopeId, - span: Span, - lvalue: &Lvalue<'tcx>, - rvalue: Rvalue<'tcx>) { - self.push(block, Statement { - scope: scope, - span: span, - kind: StatementKind::Assign(lvalue.clone(), rvalue) - }); - } - - pub fn push_assign_constant(&mut self, - block: BasicBlock, - scope: ScopeId, - span: Span, - temp: &Lvalue<'tcx>, - constant: Constant<'tcx>) { - self.push_assign(block, scope, span, temp, - Rvalue::Use(Operand::Constant(constant))); - } - - pub fn push_assign_unit(&mut self, - block: BasicBlock, - scope: ScopeId, - span: Span, - lvalue: &Lvalue<'tcx>) { - self.push_assign(block, scope, span, lvalue, Rvalue::Aggregate( - AggregateKind::Tuple, vec![] - )); - } - - pub fn terminate(&mut self, - block: BasicBlock, - scope: ScopeId, - span: Span, - kind: TerminatorKind<'tcx>) { - debug_assert!(self.block_data(block).terminator.is_none(), - "terminate: block {:?} already has a terminator set", block); - self.block_data_mut(block).terminator = Some(Terminator { - span: span, - scope: scope, - kind: kind, - }); - } } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 9d7818a9ba4d6..7250d4f404f4c 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -57,10 +57,6 @@ pub struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { cached_return_block: Option, } -struct CFG<'tcx> { - basic_blocks: Vec>, -} - /// For each scope, we track the extent (from the HIR) and a /// single-entry-multiple-exit subgraph that contains all the /// statements/terminators within it. @@ -293,7 +289,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } (Mir { - basic_blocks: self.cfg.basic_blocks, + cfg: self.cfg, scopes: self.scope_datas, promoted: vec![], var_decls: self.var_decls, diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 209649dd2fd18..93d87bf78c643 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -86,7 +86,8 @@ should go to. */ -use build::{BlockAnd, BlockAndExtension, Builder, CFG, ScopeAuxiliary}; +use build::{BlockAnd, BlockAndExtension, Builder, ScopeAuxiliary}; +use build::cfg::CfgExt; use rustc::middle::region::{CodeExtent, CodeExtentData}; use rustc::middle::lang_items; use rustc::ty::subst::{Substs, Subst, VecPerParamSpace}; diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 79d11e78bde5a..3d1ef31bd5c2a 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -49,4 +49,3 @@ mod hair; pub mod mir_map; pub mod pretty; pub mod transform; -pub mod traversal; diff --git a/src/librustc_mir/transform/add_call_guards.rs b/src/librustc_mir/transform/add_call_guards.rs index bcdd62c189972..fa6bbda54c79e 100644 --- a/src/librustc_mir/transform/add_call_guards.rs +++ b/src/librustc_mir/transform/add_call_guards.rs @@ -11,11 +11,10 @@ use rustc::ty::TyCtxt; use rustc::mir::repr::*; use rustc::mir::transform::{MirPass, MirSource, Pass}; +use rustc::mir::traversal; use pretty; -use traversal; - pub struct AddCallGuards; /** @@ -40,7 +39,7 @@ pub struct AddCallGuards; impl<'tcx> MirPass<'tcx> for AddCallGuards { fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { - let mut pred_count = vec![0u32; mir.basic_blocks.len()]; + let mut pred_count = vec![0u32; mir.cfg.basic_blocks.len()]; // Build the precedecessor map for the MIR for (_, data) in traversal::preorder(mir) { @@ -55,7 +54,7 @@ impl<'tcx> MirPass<'tcx> for AddCallGuards { let mut new_blocks = Vec::new(); let bbs = mir.all_basic_blocks(); - let cur_len = mir.basic_blocks.len(); + let cur_len = mir.cfg.basic_blocks.len(); for &bb in &bbs { let data = mir.basic_block_data_mut(bb); @@ -91,7 +90,7 @@ impl<'tcx> MirPass<'tcx> for AddCallGuards { pretty::dump_mir(tcx, "break_cleanup_edges", &0, src, mir, None); debug!("Broke {} N edges", new_blocks.len()); - mir.basic_blocks.extend_from_slice(&new_blocks); + mir.cfg.basic_blocks.extend_from_slice(&new_blocks); } } diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index d81c4e2dfb68e..ca59340fe6362 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -24,11 +24,11 @@ use rustc::mir::repr::*; use rustc::mir::visit::{LvalueContext, MutVisitor, Visitor}; +use rustc::mir::traversal::ReversePostorder; use rustc::ty::{self, TyCtxt}; use syntax::codemap::Span; use build::Location; -use traversal::ReversePostorder; use std::mem; @@ -163,8 +163,8 @@ struct Promoter<'a, 'tcx: 'a> { impl<'a, 'tcx> Promoter<'a, 'tcx> { fn new_block(&mut self) -> BasicBlock { - let index = self.promoted.basic_blocks.len(); - self.promoted.basic_blocks.push(BasicBlockData { + let index = self.promoted.cfg.basic_blocks.len(); + self.promoted.cfg.basic_blocks.push(BasicBlockData { statements: vec![], terminator: Some(Terminator { span: self.promoted.span, @@ -177,7 +177,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { } fn assign(&mut self, dest: Lvalue<'tcx>, rvalue: Rvalue<'tcx>, span: Span) { - let data = self.promoted.basic_blocks.last_mut().unwrap(); + let data = self.promoted.cfg.basic_blocks.last_mut().unwrap(); data.statements.push(Statement { span: span, scope: ScopeId::new(0), @@ -268,7 +268,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { if stmt_idx < no_stmts { self.assign(new_temp, rvalue.unwrap(), span); } else { - let last = self.promoted.basic_blocks.len() - 1; + let last = self.promoted.cfg.basic_blocks.len() - 1; let new_target = self.new_block(); let mut call = call.unwrap(); match call { @@ -277,7 +277,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { } _ => bug!() } - let terminator = &mut self.promoted.basic_blocks[last].terminator_mut(); + let terminator = &mut self.promoted.cfg.basic_blocks[last].terminator_mut(); terminator.span = span; terminator.kind = call; } @@ -366,7 +366,7 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, let mut promoter = Promoter { source: mir, promoted: Mir { - basic_blocks: vec![], + cfg: CFG { basic_blocks: vec![] }, scopes: vec![ScopeData { span: span, parent_scope: None @@ -388,7 +388,7 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, // Eliminate assignments to, and drops of promoted temps. let promoted = |index: u32| temps[index as usize] == TempState::PromotedOut; - for block in &mut mir.basic_blocks { + for block in &mut mir.cfg.basic_blocks { block.statements.retain(|statement| { match statement.kind { StatementKind::Assign(Lvalue::Temp(index), _) => { diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index b9eec6ecd9c58..52d45b8ac6650 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -26,6 +26,7 @@ use rustc::mir::repr::*; use rustc::mir::mir_map::MirMap; use rustc::mir::transform::{Pass, MirMapPass, MirSource}; use rustc::mir::visit::{LvalueContext, Visitor}; +use rustc::mir::traversal::{self, ReversePostorder}; use rustc::util::nodemap::DefIdMap; use syntax::abi::Abi; use syntax::codemap::Span; @@ -35,7 +36,6 @@ use std::collections::hash_map::Entry; use std::fmt; use build::Location; -use traversal::{self, ReversePostorder}; use super::promote_consts::{self, Candidate, TempState}; @@ -336,7 +336,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { fn qualify_const(&mut self) -> Qualif { let mir = self.mir; - let mut seen_blocks = BitVector::new(mir.basic_blocks.len()); + let mut seen_blocks = BitVector::new(mir.cfg.basic_blocks.len()); let mut bb = START_BLOCK; loop { seen_blocks.insert(bb.index()); diff --git a/src/librustc_mir/transform/remove_dead_blocks.rs b/src/librustc_mir/transform/remove_dead_blocks.rs index 44f3ce7361cf4..5b71c912bf28d 100644 --- a/src/librustc_mir/transform/remove_dead_blocks.rs +++ b/src/librustc_mir/transform/remove_dead_blocks.rs @@ -42,7 +42,7 @@ pub struct RemoveDeadBlocks; impl<'tcx> MirPass<'tcx> for RemoveDeadBlocks { fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut Mir<'tcx>) { - let mut seen = BitVector::new(mir.basic_blocks.len()); + let mut seen = BitVector::new(mir.cfg.basic_blocks.len()); // This block is always required. seen.insert(START_BLOCK.index()); @@ -63,7 +63,7 @@ impl Pass for RemoveDeadBlocks {} /// Mass removal of basic blocks to keep the ID-remapping cheap. fn retain_basic_blocks(mir: &mut Mir, keep: &BitVector) { - let num_blocks = mir.basic_blocks.len(); + let num_blocks = mir.cfg.basic_blocks.len(); let mut replacements: Vec<_> = (0..num_blocks).map(BasicBlock::new).collect(); let mut used_blocks = 0; @@ -72,11 +72,11 @@ fn retain_basic_blocks(mir: &mut Mir, keep: &BitVector) { if alive_index != used_blocks { // Swap the next alive block data with the current available slot. Since alive_index is // non-decreasing this is a valid operation. - mir.basic_blocks.swap(alive_index, used_blocks); + mir.cfg.basic_blocks.swap(alive_index, used_blocks); } used_blocks += 1; } - mir.basic_blocks.truncate(used_blocks); + mir.cfg.basic_blocks.truncate(used_blocks); for bb in mir.all_basic_blocks() { for target in mir.basic_block_data_mut(bb).terminator_mut().successors_mut() { diff --git a/src/librustc_mir/transform/simplify_cfg.rs b/src/librustc_mir/transform/simplify_cfg.rs index d008918026ab8..b5fe091a3e9de 100644 --- a/src/librustc_mir/transform/simplify_cfg.rs +++ b/src/librustc_mir/transform/simplify_cfg.rs @@ -13,13 +13,12 @@ use rustc::middle::const_val::ConstVal; use rustc::ty::TyCtxt; use rustc::mir::repr::*; use rustc::mir::transform::{MirPass, MirSource, Pass}; +use rustc::mir::traversal; use pretty; use std::mem; use super::remove_dead_blocks::RemoveDeadBlocks; -use traversal; - pub struct SimplifyCfg; impl SimplifyCfg { @@ -37,7 +36,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyCfg { pretty::dump_mir(tcx, "simplify_cfg", &0, src, mir, None); // FIXME: Should probably be moved into some kind of pass manager - mir.basic_blocks.shrink_to_fit(); + mir.cfg.basic_blocks.shrink_to_fit(); } } @@ -45,7 +44,7 @@ impl Pass for SimplifyCfg {} fn merge_consecutive_blocks(mir: &mut Mir) { // Build the precedecessor map for the MIR - let mut pred_count = vec![0u32; mir.basic_blocks.len()]; + let mut pred_count = vec![0u32; mir.cfg.basic_blocks.len()]; for (_, data) in traversal::preorder(mir) { if let Some(ref term) = data.terminator { for &tgt in term.successors().iter() { @@ -56,7 +55,7 @@ fn merge_consecutive_blocks(mir: &mut Mir) { loop { let mut changed = false; - let mut seen = BitVector::new(mir.basic_blocks.len()); + let mut seen = BitVector::new(mir.cfg.basic_blocks.len()); let mut worklist = vec![START_BLOCK]; while let Some(bb) = worklist.pop() { // Temporarily take ownership of the terminator we're modifying to keep borrowck happy diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 019ed670d1f83..f3c8b6a7f65dc 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -615,7 +615,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { fn typeck_mir(&mut self, mir: &Mir<'tcx>) { self.last_span = mir.span; debug!("run_on_mir: {:?}", mir.span); - for block in &mir.basic_blocks { + for block in &mir.cfg.basic_blocks { for stmt in &block.statements { if stmt.span != DUMMY_SP { self.last_span = stmt.span; diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs index 59143bc01bf7a..9e2e23729315a 100644 --- a/src/librustc_trans/mir/analyze.rs +++ b/src/librustc_trans/mir/analyze.rs @@ -14,8 +14,8 @@ use rustc_data_structures::bitvec::BitVector; use rustc::mir::repr as mir; use rustc::mir::repr::TerminatorKind; +use rustc::mir::traversal; use rustc::mir::visit::{Visitor, LvalueContext}; -use rustc_mir::traversal; use common::{self, Block, BlockAndBuilder}; use super::rvalue; diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index d1206550b13d6..9f91cb445d611 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/src/librustc_trans/mir/mod.rs @@ -14,6 +14,7 @@ use llvm::debuginfo::DIScope; use rustc::ty; use rustc::mir::repr as mir; use rustc::mir::tcx::LvalueTy; +use rustc::mir::traversal; use session::config::FullDebugInfo; use base; use common::{self, Block, BlockAndBuilder, CrateContext, FunctionContext}; @@ -34,7 +35,6 @@ use rustc_data_structures::bitvec::BitVector; pub use self::constant::trans_static_initializer; use self::lvalue::{LvalueRef, get_dataptr, get_meta}; -use rustc_mir::traversal; use self::operand::{OperandRef, OperandValue}; From d7a04667f9a1d1777e01f6dcd15da764bb67e804 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 14 May 2016 03:27:31 +0300 Subject: [PATCH 02/24] Impl a generic lattice-based DF framework Only a forward one so far --- src/librustc/mir/transform/dataflow.rs | 419 +++++++++++++++++++++++++ src/librustc/mir/transform/lattice.rs | 115 +++++++ 2 files changed, 534 insertions(+) create mode 100644 src/librustc/mir/transform/dataflow.rs create mode 100644 src/librustc/mir/transform/lattice.rs diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs new file mode 100644 index 0000000000000..508f74e356a0f --- /dev/null +++ b/src/librustc/mir/transform/dataflow.rs @@ -0,0 +1,419 @@ +use mir::repr as mir; +use mir::cfg::CFG; +use mir::repr::BasicBlock; +use rustc_data_structures::bitvec::BitVector; + +use mir::transform::lattice::Lattice; + + +pub trait DataflowPass<'tcx> { + type Lattice: Lattice; + type Rewrite: Rewrite<'tcx, Self::Lattice>; + type Transfer: Transfer<'tcx, Self::Lattice>; +} + +pub trait Rewrite<'tcx, L: Lattice> { + /// The rewrite function which given a statement optionally produces an alternative graph to be + /// placed in place of the original statement. + /// + /// The 2nd BasicBlock *MUST NOT* have the terminator set. + /// + /// Correctness precondition: + /// * transfer_stmt(statement, fact) == transfer_stmt(rewrite_stmt(statement, fact)) + /// that is, given some fact `fact` true before both the statement and relacement graph, and + /// a fact `fact2` which is true after the statement, the same `fact2` must be true after the + /// replacement graph too. + fn stmt(&mir::Statement<'tcx>, &L, &mut CFG<'tcx>) -> StatementChange<'tcx>; + + /// The rewrite function which given a terminator optionally produces an alternative graph to + /// be placed in place of the original statement. + /// + /// The 2nd BasicBlock *MUST* have the terminator set. + /// + /// Correctness precondition: + /// * transfer_stmt(terminator, fact) == transfer_stmt(rewrite_term(terminator, fact)) + /// that is, given some fact `fact` true before both the terminator and relacement graph, and + /// a fact `fact2` which is true after the statement, the same `fact2` must be true after the + /// replacement graph too. + fn term(&mir::Terminator<'tcx>, &L, &mut CFG<'tcx>) -> TerminatorChange<'tcx>; +} + +/// This combinator has the following behaviour: +/// +/// * Rewrite the node with the first rewriter. +/// * if the first rewriter replaced the node, 2nd rewriter is used to rewrite the replacement. +/// * otherwise 2nd rewriter is used to rewrite the original node. +pub struct RewriteAndThen<'tcx, R1, R2>(::std::marker::PhantomData<(&'tcx (), R1, R2)>); +impl<'tcx, L, R1, R2> Rewrite<'tcx, L> for RewriteAndThen<'tcx, R1, R2> +where L: Lattice, R1: Rewrite<'tcx, L>, R2: Rewrite<'tcx, L> { + fn stmt(s: &mir::Statement<'tcx>, l: &L, c: &mut CFG<'tcx>) -> StatementChange<'tcx> { + let rs = >::stmt(s, l, c); + match rs { + StatementChange::None => >::stmt(s, l, c), + StatementChange::Remove => StatementChange::Remove, + StatementChange::Statement(ns) => + match >::stmt(&ns, l, c) { + StatementChange::None => StatementChange::Statement(ns), + x => x + }, + StatementChange::Statements(nss) => { + // We expect the common case of all statements in this vector being replaced/not + // replaced by other statements 1:1 + let mut new_new_stmts = Vec::with_capacity(nss.len()); + for s in nss { + match >::stmt(&s, l, c) { + StatementChange::None => new_new_stmts.push(s), + StatementChange::Remove => {}, + StatementChange::Statement(ns) => new_new_stmts.push(ns), + StatementChange::Statements(nss) => new_new_stmts.extend(nss) + } + } + StatementChange::Statements(new_new_stmts) + } + } + } + + fn term(t: &mir::Terminator<'tcx>, l: &L, c: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { + let rt = >::term(t, l, c); + match rt { + TerminatorChange::None => >::term(t, l, c), + TerminatorChange::Terminator(nt) => match >::term(&nt, l, c) { + TerminatorChange::None => TerminatorChange::Terminator(nt), + x => x + } + } + } +} + +pub enum TerminatorChange<'tcx> { + /// No change + None, + /// Replace with another terminator + Terminator(mir::Terminator<'tcx>), +} + +pub enum StatementChange<'tcx> { + /// No change + None, + /// Remove the statement + Remove, + /// Replace with another single statement + Statement(mir::Statement<'tcx>), + /// Replace with a list of statements + Statements(Vec>), +} + +impl<'tcx> StatementChange<'tcx> { + fn normalise(&mut self) { + let old = ::std::mem::replace(self, StatementChange::None); + *self = match old { + StatementChange::Statements(mut stmts) => { + match stmts.len() { + 0 => StatementChange::Remove, + 1 => StatementChange::Statement(stmts.pop().unwrap()), + _ => StatementChange::Statements(stmts) + } + } + o => o + } + } +} + +pub trait Transfer<'tcx, L: Lattice> { + type TerminatorOut; + /// The transfer function which given a statement and a fact produces a fact which is true + /// after the statement. + fn stmt(&mir::Statement<'tcx>, L) -> L; + + /// The transfer function which given a terminator and a fact produces a fact for each + /// successor of the terminator. + /// + /// Corectness precondtition: + /// * The list of facts produced should only contain the facts for blocks which are successors + /// of the terminator being transfered. + fn term(&mir::Terminator<'tcx>, L) -> Self::TerminatorOut; +} + + +/// Facts is a mapping from basic block label (index) to the fact which is true about the first +/// statement in the block. +pub struct Facts(Vec); + +impl Facts { + pub fn new() -> Facts { + Facts(vec![]) + } + + fn put(&mut self, index: BasicBlock, value: F) { + let len = self.0.len(); + self.0.extend((len...index.index()).map(|_| ::bottom())); + self.0[index.index()] = value; + } +} + +impl ::std::ops::Index for Facts { + type Output = F; + fn index(&self, index: BasicBlock) -> &F { + &self.0.get(index.index()).expect("facts indexed immutably and the user is buggy!") + } +} + +impl ::std::ops::IndexMut for Facts { + fn index_mut(&mut self, index: BasicBlock) -> &mut F { + if let None = self.0.get_mut(index.index()) { + self.put(index, ::bottom()); + } + self.0.get_mut(index.index()).unwrap() + } +} + +/// Analyse and rewrite using dataflow in the forward direction +pub fn ar_forward<'tcx, T, P>(cfg: &CFG<'tcx>, fs: Facts, mut queue: BitVector) +-> (CFG<'tcx>, Facts) +// FIXME: shouldn’t need that T generic. +where T: Transfer<'tcx, P::Lattice, TerminatorOut=Vec>, + P: DataflowPass<'tcx, Transfer=T> +{ + fixpoint(cfg, Direction::Forward, |bb, fact, cfg| { + let new_graph = cfg.start_new_block(); + let mut fact = fact.clone(); + let mut changed = false; + // Swap out the vector of old statements for a duration of statement inspection. + let old_statements = ::std::mem::replace(&mut cfg[bb].statements, Vec::new()); + for stmt in &old_statements { + // Given a fact and statement produce a new fact and optionally a replacement + // graph. + let mut new_repl = P::Rewrite::stmt(&stmt, &fact, cfg); + new_repl.normalise(); + match new_repl { + StatementChange::None => { + fact = P::Transfer::stmt(stmt, fact); + cfg.push(new_graph, stmt.clone()); + } + StatementChange::Remove => changed = true, + StatementChange::Statement(stmt) => { + changed = true; + fact = P::Transfer::stmt(&stmt, fact); + cfg.push(new_graph, stmt); + } + StatementChange::Statements(stmts) => { + changed = true; + for stmt in &stmts { fact = P::Transfer::stmt(stmt, fact); } + cfg[new_graph].statements.extend(stmts); + } + + } + } + // Swap the statements back in. + ::std::mem::replace(&mut cfg[bb].statements, old_statements); + + // Handle the terminator replacement and transfer. + let terminator = ::std::mem::replace(&mut cfg[bb].terminator, None).unwrap(); + let repl = P::Rewrite::term(&terminator, &fact, cfg); + match repl { + TerminatorChange::None => { + cfg[new_graph].terminator = Some(terminator.clone()); + } + TerminatorChange::Terminator(t) => { + changed = true; + cfg[new_graph].terminator = Some(t); + } + } + let new_facts = P::Transfer::term(cfg[new_graph].terminator(), fact); + ::std::mem::replace(&mut cfg[bb].terminator, Some(terminator)); + + (if changed { Some(new_graph) } else { None }, new_facts) + }, &mut queue, fs) +} + +// /// The implementation of forward dataflow. +// pub struct ForwardDataflow(::std::marker::PhantomData); +// +// impl ForwardDataflow { +// pub fn new() -> ForwardDataflow { +// ForwardDataflow(::std::marker::PhantomData) +// } +// } +// +// impl Pass for ForwardDataflow {} +// +// impl<'tcx, P> MirPass<'tcx> for ForwardDataflow

+// where P: DataflowPass<'tcx> { +// fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut mir::Mir<'tcx>) { +// let facts: Facts<

>::Lattice> = +// Facts::new(

>::input_fact()); +// let (new_cfg, _) = self.arf_body(&mir.cfg, facts, mir::START_BLOCK); +// mir.cfg = new_cfg; +// } +// } +// +// impl<'tcx, P> ForwardDataflow

+// where P: DataflowPass<'tcx> { +// +// +// /// The implementation of backward dataflow. +// pub struct BackwardDataflow(::std::marker::PhantomData); +// +// impl BackwardDataflow { +// pub fn new() -> BackwardDataflow { +// BackwardDataflow(::std::marker::PhantomData) +// } +// } +// +// impl Pass for BackwardDataflow {} +// +// impl<'tcx, P> MirPass<'tcx> for BackwardDataflow

+// where P: DataflowPass<'tcx> { +// fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut mir::Mir<'tcx>) { +// let mut facts: Facts<

>::Lattice> = +// Facts::new(

>::input_fact()); +// // The desired effect here is that we should begin flowing from the blocks which terminate +// // the control flow (return, resume, calls of diverging functions, non-terminating loops +// // etc), but finding them all is a pain, so we just get the list of graph nodes postorder +// // and inspect them all! Perhaps not very effective, but certainly correct. +// let start_at = postorder(mir).filter(|&(_, d)| !d.is_cleanup).map(|(bb, _)| { +// facts.put(bb,

>::Lattice::bottom()); +// (bb, ()) +// }).collect(); +// let (new_cfg, _) = self.arb_body(&mir.cfg, facts, start_at); +// mir.cfg = new_cfg; +// } +// } +// +// impl<'tcx, P> BackwardDataflow

+// where P: DataflowPass<'tcx> { +// fn arb_body(&self, cfg: &CFG<'tcx>, +// facts: Facts<

>::Lattice>, mut map: HashMap) +// -> (CFG<'tcx>, Facts<

>::Lattice>){ +// fixpoint(cfg, Direction::Backward, |bb, fact, cfg| { +// let new_graph = cfg.start_new_block(); +// let mut fact = fact.clone(); +// // This is a reverse thing so we inspect the terminator first and statements in reverse +// // order later. +// // +// // Handle the terminator replacement and transfer. +// let terminator = ::std::mem::replace(&mut cfg[bb].terminator, None).unwrap(); +// let repl = P::rewrite_term(&terminator, &fact, cfg); +// // TODO: this really needs to get factored out +// let mut new_facts = match repl { +// TerminatorReplacement::Terminator(t) => { +// cfg[new_graph].terminator = Some(t); +// P::transfer_term(cfg[new_graph].terminator(), fact) +// } +// TerminatorReplacement::Graph(from, _) => { +// // FIXME: a more optimal approach would be to copy the from to the tail of our +// // new_graph. (1 less extra block). However there’s a problem with inspecting +// // the statements of the merged block, because we just did the statements +// // for this block already. +// cfg.terminate(new_graph, terminator.scope, terminator.span, +// mir::TerminatorKind::Goto { target: from }); +// P::transfer_term(&cfg[new_graph].terminator(), fact) +// } +// }; +// // FIXME: this should just have a different API. +// assert!(new_facts.len() == 1, "transfer_term function is incorrect"); +// fact = new_facts.pop().unwrap().1; +// ::std::mem::replace(&mut cfg[bb].terminator, Some(terminator)); +// +// // Swap out the vector of old statements for a duration of statement inspection. +// let old_statements = ::std::mem::replace(&mut cfg[bb].statements, Vec::new()); +// for stmt in old_statements.iter().rev() { +// // Given a fact and statement produce a new fact and optionally a replacement +// // graph. +// let mut new_repl = P::rewrite_stmt(&stmt, &fact, cfg); +// new_repl.normalise(); +// match new_repl { +// StatementReplacement::None => {} +// StatementReplacement::Statement(nstmt) => { +// fact = P::transfer_stmt(&nstmt, fact); +// cfg.push(new_graph, nstmt) +// } +// StatementReplacement::Statements(stmts) => { +// for stmt in &stmts { +// fact = P::transfer_stmt(stmt, fact); +// } +// cfg[new_graph].statements.extend(stmts) +// } +// StatementReplacement::Graph(..) => unimplemented!(), +// // debug_assert!(cfg[replacement.1].terminator.is_none(), +// // "buggy pass: replacement tail has a terminator set!"); +// }; +// } +// // Reverse the statements, because we were analysing bottom-top but pusshing +// // top-bottom. +// cfg[new_graph].statements.reverse(); +// ::std::mem::replace(&mut cfg[bb].statements, old_statements); +// (Some(new_graph), vec![(mir::START_BLOCK, fact)]) +// }, &mut map, facts) +// } +// } + +enum Direction { + Forward, + Backward +} + +/// The fixpoint function is the engine of this whole thing. Important part of it is the `f: BF` +/// callback. This for each basic block and its facts has to produce a replacement graph and a +/// bunch of facts which are to be joined with the facts in the graph elsewhere. +fn fixpoint<'tcx, F: Lattice, BF>(cfg: &CFG<'tcx>, + direction: Direction, + f: BF, + to_visit: &mut BitVector, + mut init_facts: Facts) -> (CFG<'tcx>, Facts) +// TODO: we probably want to pass in a list of basicblocks as successors (predecessors in backward +// fixpoing) and let BF return just a list of F. +where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (Option, Vec), + // ^~ This function given a single block and fact before it optionally produces a replacement + // graph (if not, the original block is the “replacement graph”) for the block and a list of + // facts for arbitrary blocks (most likely for the blocks in the replacement graph and blocks + // into which data flows from the replacement graph) + // + // Invariant: + // * None of the already existing blocks in CFG may be modified; +{ + let mut cfg = cfg.clone(); + + while let Some(block) = to_visit.iter().next() { + to_visit.remove(block); + let block = BasicBlock::new(block); + + let (new_target, new_facts) = { + let fact = &mut init_facts[block]; + f(block, fact, &mut cfg) + }; + + // First of all, we merge in the replacement graph, if any. + if let Some(replacement_bb) = new_target { + cfg.swap(replacement_bb, block); + } + + // Then we record the facts in the correct direction. + if let Direction::Forward = direction { + for (f, &target) in new_facts.into_iter() + .zip(cfg[block].terminator().successors().iter()) { + let facts_changed = Lattice::join(&mut init_facts[target], &f); + if facts_changed { + to_visit.insert(target.index()); + } + } + } else { + unimplemented!() + // let mut new_facts = new_facts; + // let fact = new_facts.pop().unwrap().1; + // for pred in cfg.predecessors(block) { + // if init_facts.exists(pred) { + // let old_fact = &mut init_facts[pred]; + // let facts_changed = Lattice::join(old_fact, &fact); + // if facts_changed { + // to_visit.insert(pred, ()); + // } + // } else { + // init_facts.put(pred, fact.clone()); + // to_visit.insert(pred, ()); + // } + // } + } + } + (cfg, init_facts) +} diff --git a/src/librustc/mir/transform/lattice.rs b/src/librustc/mir/transform/lattice.rs new file mode 100644 index 0000000000000..129291b349b7f --- /dev/null +++ b/src/librustc/mir/transform/lattice.rs @@ -0,0 +1,115 @@ +use std::fmt::{Debug, Formatter}; +use std::collections::hash_map::Entry; +use std::collections::HashMap; + +pub trait Lattice: Clone { + fn bottom() -> Self; + fn join(&mut self, other: &Self) -> bool; +} + +/// Extend the type with a Top point. +#[derive(Clone, PartialEq)] +pub enum WTop { + Top, + Value(T) +} + +impl Lattice for WTop { + fn bottom() -> Self { + WTop::Value(::bottom()) + } + + /// V + V = join(v, v) + /// ⊤ + V = ⊤ (no change) + /// V + ⊤ = ⊤ + /// ⊤ + ⊤ = ⊤ (no change) + default fn join(&mut self, other: &Self) -> bool { + match (self, other) { + (&mut WTop::Value(ref mut this), &WTop::Value(ref o)) => ::join(this, o), + (&mut WTop::Top, _) => false, + (this, &WTop::Top) => { + *this = WTop::Top; + true + } + } + } +} + +impl Debug for WTop { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { + match *self { + WTop::Top => f.write_str("⊤"), + WTop::Value(ref t) => ::fmt(t, f) + } + } +} + +/// Extend the type with a bottom point +/// +/// This guarantees the bottom() of the underlying lattice won’t get called so it may be +/// implemented as a `panic!()` or something. +#[derive(Clone, PartialEq)] +pub enum WBottom { + Bottom, + Value(T) +} + +impl Lattice for WBottom { + fn bottom() -> Self { + WBottom::Bottom + } + + /// V + V = join(v, v) + /// ⊥ + V = V + /// V + ⊥ = V (no change) + /// ⊥ + ⊥ = ⊥ (no change) + fn join(&mut self, other: &Self) -> bool { + match (self, other) { + (&mut WBottom::Value(ref mut this), &WBottom::Value(ref o)) => + ::join(this, o), + (_, &WBottom::Bottom) => false, + (this, o) => { + *this = o.clone(); + true + } + } + } + +} + +impl Debug for WBottom { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { + match *self { + WBottom::Bottom => f.write_str("⊥"), + WBottom::Value(ref t) => ::fmt(t, f) + } + } +} + +/// Extend the type with both bottom and top points. +type WTopBottom = WTop>; + +impl Lattice for HashMap +where K: Clone + Eq + ::std::hash::Hash, + T: Lattice, + H: Clone + ::std::hash::BuildHasher + ::std::default::Default +{ + fn bottom() -> Self { + HashMap::default() + } + fn join(&mut self, other: &Self) -> bool { + let mut changed = false; + for (key, val) in other.iter() { + match self.entry(key.clone()) { + Entry::Vacant(e) => { + e.insert(val.clone()); + changed = true + } + Entry::Occupied(mut e) => changed |= e.get_mut().join(val) + } + } + changed + } +} + + From 060d84040e17aceefe5819f4a945b66c184fb382 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 14 May 2016 03:29:51 +0300 Subject: [PATCH 03/24] Implement a ACS propagation pass ACS here stands for Alias-Constant-Simplify. This propagation pass is a composition of three distinct dataflow passes: alias-propagation, constant-propagation and terminator simplification. This pass also serves to showcase the features of the framework. Most notably the terseness, composability and the capability to interleave analysis and transformations. --- src/librustc/mir/transform/lattice.rs | 3 +- src/librustc/mir/transform/mod.rs | 3 + src/librustc_driver/driver.rs | 3 +- src/librustc_mir/transform/acs_propagate.rs | 224 ++++++++++++++++++++ src/librustc_mir/transform/mod.rs | 1 + src/librustc_mir/transform/simplify_cfg.rs | 50 ----- 6 files changed, 232 insertions(+), 52 deletions(-) create mode 100644 src/librustc_mir/transform/acs_propagate.rs diff --git a/src/librustc/mir/transform/lattice.rs b/src/librustc/mir/transform/lattice.rs index 129291b349b7f..85a5081f167dc 100644 --- a/src/librustc/mir/transform/lattice.rs +++ b/src/librustc/mir/transform/lattice.rs @@ -23,7 +23,8 @@ impl Lattice for WTop { /// ⊤ + V = ⊤ (no change) /// V + ⊤ = ⊤ /// ⊤ + ⊤ = ⊤ (no change) - default fn join(&mut self, other: &Self) -> bool { +// default fn join(&mut self, other: &Self) -> bool { + fn join(&mut self, other: &Self) -> bool { match (self, other) { (&mut WTop::Value(ref mut this), &WTop::Value(ref o)) => ::join(this, o), (&mut WTop::Top, _) => false, diff --git a/src/librustc/mir/transform/mod.rs b/src/librustc/mir/transform/mod.rs index 828a48532a2fc..3c9257086bc42 100644 --- a/src/librustc/mir/transform/mod.rs +++ b/src/librustc/mir/transform/mod.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +pub mod lattice; +pub mod dataflow; + use dep_graph::DepNode; use hir; use hir::map::DefPathData; diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index b28d203ed8dc2..6f09d3cabf4d4 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -967,7 +967,8 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); passes.push_pass(box mir::transform::qualify_consts::QualifyAndPromoteConstants); passes.push_pass(box mir::transform::type_check::TypeckMir); - passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg); + passes.push_pass(box mir::transform::acs_propagate::ACSPropagate); + // passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg); passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); // And run everything. passes.run_passes(tcx, &mut mir_map); diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs new file mode 100644 index 0000000000000..38ce707f4edc5 --- /dev/null +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -0,0 +1,224 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! This is Alias-Constant-Simplify propagation pass. This is a composition of three distinct +//! dataflow passes: alias-propagation, constant-propagation and terminator simplification. +//! +//! All these are very similar in their nature: +//! +//! | Constant | Alias | Simplify | +//!|----------------|-----------|----------|-----------| +//!| Lattice Domain | Lvalue | Lvalue | Lvalue | +//!| Lattice Value | Constant | Lvalue | Constant | +//!| Transfer | x = const | x = lval | x = const | +//!| Rewrite | x → const | x → lval | T(x) → T' | +//!| Bottom | {} | {} | {} | +//! +//! For all of them we will be using a lattice of Hashmap from Lvalue to +//! WTop> +//! +//! My personal believ is that it should be possible to make a way to compose two hashmap lattices +//! into one, but I can’t seem to get it just right yet, so we do the composing and decomposing +//! manually here. + +use rustc_data_structures::fnv::FnvHashMap; +use rustc_data_structures::bitvec::BitVector; +use rustc::mir::repr::*; +use rustc::mir::visit::{MutVisitor, LvalueContext}; +use rustc::mir::transform::lattice::Lattice; +use rustc::mir::transform::dataflow::*; +use rustc::mir::transform::{Pass, MirPass, MirSource}; +use rustc::ty::TyCtxt; +use rustc::middle::const_val::ConstVal; +use pretty; + +#[derive(PartialEq, Debug, Eq, Clone)] +pub enum Either<'tcx> { + Top, + Lvalue(Lvalue<'tcx>), + Const(Constant<'tcx>), +} + +impl<'tcx> Lattice for Either<'tcx> { + fn bottom() -> Self { unimplemented!() } + fn join(&mut self, other: &Self) -> bool { + if self == other { + false + } else { + *self = Either::Top; + true + } + } +} + +pub type ACSLattice<'a> = FnvHashMap, Either<'a>>; + +pub struct ACSPropagate; + +impl Pass for ACSPropagate {} + +impl<'tcx> MirPass<'tcx> for ACSPropagate { + fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { + let mut q = BitVector::new(mir.cfg.len()); + q.insert(START_BLOCK.index()); + let ret = ar_forward::(&mut mir.cfg, Facts::new(), q); + mir.cfg = ret.0; + pretty::dump_mir(tcx, "acs_propagate", &0, src, mir, None); + } + +} + +impl<'tcx> DataflowPass<'tcx> for ACSPropagate { + type Lattice = ACSLattice<'tcx>; + type Rewrite = RewriteAndThen<'tcx, AliasRewrite, + RewriteAndThen<'tcx, ConstRewrite, SimplifyRewrite>>; + type Transfer = ACSPropagateTransfer; +} + +pub struct ACSPropagateTransfer; + +impl<'tcx> Transfer<'tcx, ACSLattice<'tcx>> for ACSPropagateTransfer { + type TerminatorOut = Vec>; + fn stmt(s: &Statement<'tcx>, mut lat: ACSLattice<'tcx>) -> ACSLattice<'tcx> { + let StatementKind::Assign(ref lval, ref rval) = s.kind; + match *rval { + Rvalue::Use(Operand::Consume(ref nlval)) => + lat.insert(lval.clone(), Either::Lvalue(nlval.clone())), + Rvalue::Use(Operand::Constant(ref c)) => + lat.insert(lval.clone(), Either::Const(c.clone())), + _ => lat.insert(lval.clone(), Either::Top) + }; + lat + } + fn term(t: &Terminator<'tcx>, lat: ACSLattice<'tcx>) -> Self::TerminatorOut { + // FIXME: this should inspect the terminators and set their known values to constants. Esp. + // for the if: in the truthy branch the operand is known to be true and in the falsy branch + // the operand is known to be false. Now we just ignore the potential here. + let mut ret = vec![]; + ret.resize(t.successors().len(), lat); + ret + } +} + +pub struct AliasRewrite; + +impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for AliasRewrite { + fn stmt(s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + -> StatementChange<'tcx> { + let mut ns = s.clone(); + let mut vis = RewriteAliasVisitor(&l, false); + vis.visit_statement(START_BLOCK, &mut ns); + if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } + } + fn term(t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + -> TerminatorChange<'tcx> { + let mut nt = t.clone(); + let mut vis = RewriteAliasVisitor(&l, false); + vis.visit_terminator(START_BLOCK, &mut nt); + if vis.1 { TerminatorChange::Terminator(nt) } else { TerminatorChange::None } + } +} + +struct RewriteAliasVisitor<'a, 'tcx: 'a>(pub &'a ACSLattice<'tcx>, pub bool); +impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { + fn visit_lvalue(&mut self, lvalue: &mut Lvalue<'tcx>, context: LvalueContext) { + match context { + LvalueContext::Store | LvalueContext::Call => {} + _ => { + let replacement = self.0.get(lvalue); + match replacement { + Some(&Either::Lvalue(ref nlval)) => { + self.1 = true; + *lvalue = nlval.clone(); + } + _ => {} + } + } + } + self.super_lvalue(lvalue, context); + } +} + +pub struct ConstRewrite; + +impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for ConstRewrite { + fn stmt(s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + -> StatementChange<'tcx> { + let mut ns = s.clone(); + let mut vis = RewriteConstVisitor(&l, false); + vis.visit_statement(START_BLOCK, &mut ns); + if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } + } + fn term(t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + -> TerminatorChange<'tcx> { + let mut nt = t.clone(); + let mut vis = RewriteConstVisitor(&l, false); + vis.visit_terminator(START_BLOCK, &mut nt); + if vis.1 { TerminatorChange::Terminator(nt) } else { TerminatorChange::None } + } +} + +struct RewriteConstVisitor<'a, 'tcx: 'a>(pub &'a ACSLattice<'tcx>, pub bool); +impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { + fn visit_operand(&mut self, op: &mut Operand<'tcx>) { + let repl = if let Operand::Consume(ref lval) = *op { + if let Some(&Either::Const(ref c)) = self.0.get(lval) { + Some(c.clone()) + } else { + None + } + } else { + None + }; + if let Some(c) = repl { + *op = Operand::Constant(c); + } + self.super_operand(op); + } +} + + +pub struct SimplifyRewrite; + +impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for SimplifyRewrite { + fn stmt(s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + -> StatementChange<'tcx> { + StatementChange::None + } + fn term(t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + -> TerminatorChange<'tcx> { + match t.kind { + TerminatorKind::If { ref targets, .. } if targets.0 == targets.1 => { + let mut nt = t.clone(); + nt.kind = TerminatorKind::Goto { target: targets.0 }; + TerminatorChange::Terminator(nt) + } + TerminatorKind::If { ref targets, cond: Operand::Constant(Constant { + literal: Literal::Value { + value: ConstVal::Bool(cond) + }, .. + }) } => { + let mut nt = t.clone(); + if cond { + nt.kind = TerminatorKind::Goto { target: targets.0 }; + } else { + nt.kind = TerminatorKind::Goto { target: targets.1 }; + } + TerminatorChange::Terminator(nt) + } + TerminatorKind::SwitchInt { ref targets, .. } if targets.len() == 1 => { + let mut nt = t.clone(); + nt.kind = TerminatorKind::Goto { target: targets[0] }; + TerminatorChange::Terminator(nt) + } + _ => TerminatorChange::None + } + } +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 339dcdec06080..56be0223be4f5 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -17,3 +17,4 @@ pub mod add_call_guards; pub mod promote_consts; pub mod qualify_consts; pub mod dump_mir; +pub mod acs_propagate; diff --git a/src/librustc_mir/transform/simplify_cfg.rs b/src/librustc_mir/transform/simplify_cfg.rs index b5fe091a3e9de..1d370648aead5 100644 --- a/src/librustc_mir/transform/simplify_cfg.rs +++ b/src/librustc_mir/transform/simplify_cfg.rs @@ -29,7 +29,6 @@ impl SimplifyCfg { impl<'tcx> MirPass<'tcx> for SimplifyCfg { fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { - simplify_branches(mir); RemoveDeadBlocks.run_pass(tcx, src, mir); merge_consecutive_blocks(mir); RemoveDeadBlocks.run_pass(tcx, src, mir); @@ -153,52 +152,3 @@ fn final_target(mir: &Mir, mut target: BasicBlock) -> Option { Some(target) } - -fn simplify_branches(mir: &mut Mir) { - loop { - let mut changed = false; - - for bb in mir.all_basic_blocks() { - let basic_block = mir.basic_block_data_mut(bb); - let mut terminator = basic_block.terminator_mut(); - terminator.kind = match terminator.kind { - TerminatorKind::If { ref targets, .. } if targets.0 == targets.1 => { - changed = true; - TerminatorKind::Goto { target: targets.0 } - } - - TerminatorKind::If { ref targets, cond: Operand::Constant(Constant { - literal: Literal::Value { - value: ConstVal::Bool(cond) - }, .. - }) } => { - changed = true; - if cond { - TerminatorKind::Goto { target: targets.0 } - } else { - TerminatorKind::Goto { target: targets.1 } - } - } - - TerminatorKind::Assert { target, cond: Operand::Constant(Constant { - literal: Literal::Value { - value: ConstVal::Bool(cond) - }, .. - }), expected, .. } if cond == expected => { - changed = true; - TerminatorKind::Goto { target: target } - } - - TerminatorKind::SwitchInt { ref targets, .. } if targets.len() == 1 => { - changed = true; - TerminatorKind::Goto { target: targets[0] } - } - _ => continue - } - } - - if !changed { - break; - } - } -} From e20d181e24c4a94918fe6bcb1b349d660e85d5ec Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 05:46:52 -0500 Subject: [PATCH 04/24] Remove use of specialization in Lattice to avoid use of unstable features --- src/librustc/mir/transform/lattice.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc/mir/transform/lattice.rs b/src/librustc/mir/transform/lattice.rs index 85a5081f167dc..9f6cda4abdcc9 100644 --- a/src/librustc/mir/transform/lattice.rs +++ b/src/librustc/mir/transform/lattice.rs @@ -23,7 +23,6 @@ impl Lattice for WTop { /// ⊤ + V = ⊤ (no change) /// V + ⊤ = ⊤ /// ⊤ + ⊤ = ⊤ (no change) -// default fn join(&mut self, other: &Self) -> bool { fn join(&mut self, other: &Self) -> bool { match (self, other) { (&mut WTop::Value(ref mut this), &WTop::Value(ref o)) => ::join(this, o), From ea689a06fab07275cbbe74e2248c645167da37a1 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:06:44 -0500 Subject: [PATCH 05/24] Remove DataflowPass --- src/librustc/mir/transform/dataflow.rs | 37 +++++++++------------ src/librustc_mir/transform/acs_propagate.rs | 18 ++++------ 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index 508f74e356a0f..f56fde71b52bb 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -5,13 +5,6 @@ use rustc_data_structures::bitvec::BitVector; use mir::transform::lattice::Lattice; - -pub trait DataflowPass<'tcx> { - type Lattice: Lattice; - type Rewrite: Rewrite<'tcx, Self::Lattice>; - type Transfer: Transfer<'tcx, Self::Lattice>; -} - pub trait Rewrite<'tcx, L: Lattice> { /// The rewrite function which given a statement optionally produces an alternative graph to be /// placed in place of the original statement. @@ -119,11 +112,12 @@ impl<'tcx> StatementChange<'tcx> { } } -pub trait Transfer<'tcx, L: Lattice> { - type TerminatorOut; +pub trait Transfer<'tcx> { + type Lattice: Lattice; + /// The transfer function which given a statement and a fact produces a fact which is true /// after the statement. - fn stmt(&mir::Statement<'tcx>, L) -> L; + fn stmt(&mir::Statement<'tcx>, Self::Lattice) -> Self::Lattice; /// The transfer function which given a terminator and a fact produces a fact for each /// successor of the terminator. @@ -131,7 +125,7 @@ pub trait Transfer<'tcx, L: Lattice> { /// Corectness precondtition: /// * The list of facts produced should only contain the facts for blocks which are successors /// of the terminator being transfered. - fn term(&mir::Terminator<'tcx>, L) -> Self::TerminatorOut; + fn term(&mir::Terminator<'tcx>, Self::Lattice) -> Vec; } @@ -168,11 +162,10 @@ impl ::std::ops::IndexMut for Facts { } /// Analyse and rewrite using dataflow in the forward direction -pub fn ar_forward<'tcx, T, P>(cfg: &CFG<'tcx>, fs: Facts, mut queue: BitVector) --> (CFG<'tcx>, Facts) -// FIXME: shouldn’t need that T generic. -where T: Transfer<'tcx, P::Lattice, TerminatorOut=Vec>, - P: DataflowPass<'tcx, Transfer=T> +pub fn ar_forward<'tcx, T, R>(cfg: &CFG<'tcx>, fs: Facts, mut queue: BitVector) +-> (CFG<'tcx>, Facts) +where T: Transfer<'tcx>, + R: Rewrite<'tcx, T::Lattice> { fixpoint(cfg, Direction::Forward, |bb, fact, cfg| { let new_graph = cfg.start_new_block(); @@ -183,22 +176,22 @@ where T: Transfer<'tcx, P::Lattice, TerminatorOut=Vec>, for stmt in &old_statements { // Given a fact and statement produce a new fact and optionally a replacement // graph. - let mut new_repl = P::Rewrite::stmt(&stmt, &fact, cfg); + let mut new_repl = R::stmt(&stmt, &fact, cfg); new_repl.normalise(); match new_repl { StatementChange::None => { - fact = P::Transfer::stmt(stmt, fact); + fact = T::stmt(stmt, fact); cfg.push(new_graph, stmt.clone()); } StatementChange::Remove => changed = true, StatementChange::Statement(stmt) => { changed = true; - fact = P::Transfer::stmt(&stmt, fact); + fact = T::stmt(&stmt, fact); cfg.push(new_graph, stmt); } StatementChange::Statements(stmts) => { changed = true; - for stmt in &stmts { fact = P::Transfer::stmt(stmt, fact); } + for stmt in &stmts { fact = T::stmt(stmt, fact); } cfg[new_graph].statements.extend(stmts); } @@ -209,7 +202,7 @@ where T: Transfer<'tcx, P::Lattice, TerminatorOut=Vec>, // Handle the terminator replacement and transfer. let terminator = ::std::mem::replace(&mut cfg[bb].terminator, None).unwrap(); - let repl = P::Rewrite::term(&terminator, &fact, cfg); + let repl = R::term(&terminator, &fact, cfg); match repl { TerminatorChange::None => { cfg[new_graph].terminator = Some(terminator.clone()); @@ -219,7 +212,7 @@ where T: Transfer<'tcx, P::Lattice, TerminatorOut=Vec>, cfg[new_graph].terminator = Some(t); } } - let new_facts = P::Transfer::term(cfg[new_graph].terminator(), fact); + let new_facts = T::term(cfg[new_graph].terminator(), fact); ::std::mem::replace(&mut cfg[bb].terminator, Some(terminator)); (if changed { Some(new_graph) } else { None }, new_facts) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index 38ce707f4edc5..aaae35efb3961 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -68,24 +68,19 @@ impl<'tcx> MirPass<'tcx> for ACSPropagate { fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { let mut q = BitVector::new(mir.cfg.len()); q.insert(START_BLOCK.index()); - let ret = ar_forward::(&mut mir.cfg, Facts::new(), q); + let ret = ar_forward::>>(&mut mir.cfg, Facts::new(), q); mir.cfg = ret.0; pretty::dump_mir(tcx, "acs_propagate", &0, src, mir, None); } } -impl<'tcx> DataflowPass<'tcx> for ACSPropagate { - type Lattice = ACSLattice<'tcx>; - type Rewrite = RewriteAndThen<'tcx, AliasRewrite, - RewriteAndThen<'tcx, ConstRewrite, SimplifyRewrite>>; - type Transfer = ACSPropagateTransfer; -} - pub struct ACSPropagateTransfer; -impl<'tcx> Transfer<'tcx, ACSLattice<'tcx>> for ACSPropagateTransfer { - type TerminatorOut = Vec>; +impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { + type Lattice = ACSLattice<'tcx>; + fn stmt(s: &Statement<'tcx>, mut lat: ACSLattice<'tcx>) -> ACSLattice<'tcx> { let StatementKind::Assign(ref lval, ref rval) = s.kind; match *rval { @@ -97,7 +92,8 @@ impl<'tcx> Transfer<'tcx, ACSLattice<'tcx>> for ACSPropagateTransfer { }; lat } - fn term(t: &Terminator<'tcx>, lat: ACSLattice<'tcx>) -> Self::TerminatorOut { + + fn term(t: &Terminator<'tcx>, lat: ACSLattice<'tcx>) -> Vec> { // FIXME: this should inspect the terminators and set their known values to constants. Esp. // for the if: in the truthy branch the operand is known to be true and in the falsy branch // the operand is known to be false. Now we just ignore the potential here. From c1bb0604112526600f0d26064fe03e9523751ba1 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:25:02 -0500 Subject: [PATCH 06/24] Add &self arguments to Transfer and Rewrite and start taking them by value instead of by type --- src/librustc/mir/transform/dataflow.rs | 48 +++++++++++---------- src/librustc_mir/transform/acs_propagate.rs | 27 +++++++----- 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index f56fde71b52bb..181d31e89de33 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -16,7 +16,7 @@ pub trait Rewrite<'tcx, L: Lattice> { /// that is, given some fact `fact` true before both the statement and relacement graph, and /// a fact `fact2` which is true after the statement, the same `fact2` must be true after the /// replacement graph too. - fn stmt(&mir::Statement<'tcx>, &L, &mut CFG<'tcx>) -> StatementChange<'tcx>; + fn stmt(&self, &mir::Statement<'tcx>, &L, &mut CFG<'tcx>) -> StatementChange<'tcx>; /// The rewrite function which given a terminator optionally produces an alternative graph to /// be placed in place of the original statement. @@ -28,7 +28,11 @@ pub trait Rewrite<'tcx, L: Lattice> { /// that is, given some fact `fact` true before both the terminator and relacement graph, and /// a fact `fact2` which is true after the statement, the same `fact2` must be true after the /// replacement graph too. - fn term(&mir::Terminator<'tcx>, &L, &mut CFG<'tcx>) -> TerminatorChange<'tcx>; + fn term(&self, &mir::Terminator<'tcx>, &L, &mut CFG<'tcx>) -> TerminatorChange<'tcx>; + + fn and_then(self, other: R2) -> RewriteAndThen where Self: Sized { + RewriteAndThen(self, other) + } } /// This combinator has the following behaviour: @@ -36,16 +40,16 @@ pub trait Rewrite<'tcx, L: Lattice> { /// * Rewrite the node with the first rewriter. /// * if the first rewriter replaced the node, 2nd rewriter is used to rewrite the replacement. /// * otherwise 2nd rewriter is used to rewrite the original node. -pub struct RewriteAndThen<'tcx, R1, R2>(::std::marker::PhantomData<(&'tcx (), R1, R2)>); -impl<'tcx, L, R1, R2> Rewrite<'tcx, L> for RewriteAndThen<'tcx, R1, R2> +pub struct RewriteAndThen(R1, R2); +impl<'tcx, L, R1, R2> Rewrite<'tcx, L> for RewriteAndThen where L: Lattice, R1: Rewrite<'tcx, L>, R2: Rewrite<'tcx, L> { - fn stmt(s: &mir::Statement<'tcx>, l: &L, c: &mut CFG<'tcx>) -> StatementChange<'tcx> { - let rs = >::stmt(s, l, c); + fn stmt(&self, s: &mir::Statement<'tcx>, l: &L, c: &mut CFG<'tcx>) -> StatementChange<'tcx> { + let rs = self.0.stmt(s, l, c); match rs { - StatementChange::None => >::stmt(s, l, c), + StatementChange::None => self.1.stmt(s, l, c), StatementChange::Remove => StatementChange::Remove, StatementChange::Statement(ns) => - match >::stmt(&ns, l, c) { + match self.1.stmt(&ns, l, c) { StatementChange::None => StatementChange::Statement(ns), x => x }, @@ -54,7 +58,7 @@ where L: Lattice, R1: Rewrite<'tcx, L>, R2: Rewrite<'tcx, L> { // replaced by other statements 1:1 let mut new_new_stmts = Vec::with_capacity(nss.len()); for s in nss { - match >::stmt(&s, l, c) { + match self.1.stmt(&s, l, c) { StatementChange::None => new_new_stmts.push(s), StatementChange::Remove => {}, StatementChange::Statement(ns) => new_new_stmts.push(ns), @@ -66,11 +70,11 @@ where L: Lattice, R1: Rewrite<'tcx, L>, R2: Rewrite<'tcx, L> { } } - fn term(t: &mir::Terminator<'tcx>, l: &L, c: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { - let rt = >::term(t, l, c); + fn term(&self, t: &mir::Terminator<'tcx>, l: &L, c: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { + let rt = self.0.term(t, l, c); match rt { - TerminatorChange::None => >::term(t, l, c), - TerminatorChange::Terminator(nt) => match >::term(&nt, l, c) { + TerminatorChange::None => self.1.term(t, l, c), + TerminatorChange::Terminator(nt) => match self.1.term(&nt, l, c) { TerminatorChange::None => TerminatorChange::Terminator(nt), x => x } @@ -117,7 +121,7 @@ pub trait Transfer<'tcx> { /// The transfer function which given a statement and a fact produces a fact which is true /// after the statement. - fn stmt(&mir::Statement<'tcx>, Self::Lattice) -> Self::Lattice; + fn stmt(&self, &mir::Statement<'tcx>, Self::Lattice) -> Self::Lattice; /// The transfer function which given a terminator and a fact produces a fact for each /// successor of the terminator. @@ -125,7 +129,7 @@ pub trait Transfer<'tcx> { /// Corectness precondtition: /// * The list of facts produced should only contain the facts for blocks which are successors /// of the terminator being transfered. - fn term(&mir::Terminator<'tcx>, Self::Lattice) -> Vec; + fn term(&self, &mir::Terminator<'tcx>, Self::Lattice) -> Vec; } @@ -162,7 +166,7 @@ impl ::std::ops::IndexMut for Facts { } /// Analyse and rewrite using dataflow in the forward direction -pub fn ar_forward<'tcx, T, R>(cfg: &CFG<'tcx>, fs: Facts, mut queue: BitVector) +pub fn ar_forward<'tcx, T, R>(cfg: &CFG<'tcx>, fs: Facts, mut queue: BitVector, transfer: T, rewrite: R) -> (CFG<'tcx>, Facts) where T: Transfer<'tcx>, R: Rewrite<'tcx, T::Lattice> @@ -176,22 +180,22 @@ where T: Transfer<'tcx>, for stmt in &old_statements { // Given a fact and statement produce a new fact and optionally a replacement // graph. - let mut new_repl = R::stmt(&stmt, &fact, cfg); + let mut new_repl = rewrite.stmt(&stmt, &fact, cfg); new_repl.normalise(); match new_repl { StatementChange::None => { - fact = T::stmt(stmt, fact); + fact = transfer.stmt(stmt, fact); cfg.push(new_graph, stmt.clone()); } StatementChange::Remove => changed = true, StatementChange::Statement(stmt) => { changed = true; - fact = T::stmt(&stmt, fact); + fact = transfer.stmt(&stmt, fact); cfg.push(new_graph, stmt); } StatementChange::Statements(stmts) => { changed = true; - for stmt in &stmts { fact = T::stmt(stmt, fact); } + for stmt in &stmts { fact = transfer.stmt(stmt, fact); } cfg[new_graph].statements.extend(stmts); } @@ -202,7 +206,7 @@ where T: Transfer<'tcx>, // Handle the terminator replacement and transfer. let terminator = ::std::mem::replace(&mut cfg[bb].terminator, None).unwrap(); - let repl = R::term(&terminator, &fact, cfg); + let repl = rewrite.term(&terminator, &fact, cfg); match repl { TerminatorChange::None => { cfg[new_graph].terminator = Some(terminator.clone()); @@ -212,7 +216,7 @@ where T: Transfer<'tcx>, cfg[new_graph].terminator = Some(t); } } - let new_facts = T::term(cfg[new_graph].terminator(), fact); + let new_facts = transfer.term(cfg[new_graph].terminator(), fact); ::std::mem::replace(&mut cfg[bb].terminator, Some(terminator)); (if changed { Some(new_graph) } else { None }, new_facts) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index aaae35efb3961..0aef37685acf2 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -68,8 +68,12 @@ impl<'tcx> MirPass<'tcx> for ACSPropagate { fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { let mut q = BitVector::new(mir.cfg.len()); q.insert(START_BLOCK.index()); - let ret = ar_forward::>>(&mut mir.cfg, Facts::new(), q); + let ret = ar_forward( + &mut mir.cfg, + Facts::new(), q, + ACSPropagateTransfer, + AliasRewrite.and_then(ConstRewrite).and_then(SimplifyRewrite) + ); mir.cfg = ret.0; pretty::dump_mir(tcx, "acs_propagate", &0, src, mir, None); } @@ -81,7 +85,7 @@ pub struct ACSPropagateTransfer; impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { type Lattice = ACSLattice<'tcx>; - fn stmt(s: &Statement<'tcx>, mut lat: ACSLattice<'tcx>) -> ACSLattice<'tcx> { + fn stmt(&self, s: &Statement<'tcx>, mut lat: ACSLattice<'tcx>) -> ACSLattice<'tcx> { let StatementKind::Assign(ref lval, ref rval) = s.kind; match *rval { Rvalue::Use(Operand::Consume(ref nlval)) => @@ -93,7 +97,7 @@ impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { lat } - fn term(t: &Terminator<'tcx>, lat: ACSLattice<'tcx>) -> Vec> { + fn term(&self, t: &Terminator<'tcx>, lat: ACSLattice<'tcx>) -> Vec> { // FIXME: this should inspect the terminators and set their known values to constants. Esp. // for the if: in the truthy branch the operand is known to be true and in the falsy branch // the operand is known to be false. Now we just ignore the potential here. @@ -106,14 +110,15 @@ impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { pub struct AliasRewrite; impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for AliasRewrite { - fn stmt(s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn stmt(&self, s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) -> StatementChange<'tcx> { let mut ns = s.clone(); let mut vis = RewriteAliasVisitor(&l, false); vis.visit_statement(START_BLOCK, &mut ns); if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } } - fn term(t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + + fn term(&self, t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { let mut nt = t.clone(); let mut vis = RewriteAliasVisitor(&l, false); @@ -145,14 +150,15 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { pub struct ConstRewrite; impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for ConstRewrite { - fn stmt(s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn stmt(&self, s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) -> StatementChange<'tcx> { let mut ns = s.clone(); let mut vis = RewriteConstVisitor(&l, false); vis.visit_statement(START_BLOCK, &mut ns); if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } } - fn term(t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + + fn term(&self, t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { let mut nt = t.clone(); let mut vis = RewriteConstVisitor(&l, false); @@ -184,11 +190,12 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { pub struct SimplifyRewrite; impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for SimplifyRewrite { - fn stmt(s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn stmt(&self, s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) -> StatementChange<'tcx> { StatementChange::None } - fn term(t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + + fn term(&self, t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { match t.kind { TerminatorKind::If { ref targets, .. } if targets.0 == targets.1 => { From 52eff47242ad8c2ca1f81a3e763bd8e87d34a321 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:29:59 -0500 Subject: [PATCH 07/24] Remove unused and buggy support for dataflow passes that introduce multiple statements --- src/librustc/mir/transform/dataflow.rs | 42 +------------------------- 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index 181d31e89de33..83fb62a4c04fd 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -52,21 +52,7 @@ where L: Lattice, R1: Rewrite<'tcx, L>, R2: Rewrite<'tcx, L> { match self.1.stmt(&ns, l, c) { StatementChange::None => StatementChange::Statement(ns), x => x - }, - StatementChange::Statements(nss) => { - // We expect the common case of all statements in this vector being replaced/not - // replaced by other statements 1:1 - let mut new_new_stmts = Vec::with_capacity(nss.len()); - for s in nss { - match self.1.stmt(&s, l, c) { - StatementChange::None => new_new_stmts.push(s), - StatementChange::Remove => {}, - StatementChange::Statement(ns) => new_new_stmts.push(ns), - StatementChange::Statements(nss) => new_new_stmts.extend(nss) - } } - StatementChange::Statements(new_new_stmts) - } } } @@ -96,24 +82,6 @@ pub enum StatementChange<'tcx> { Remove, /// Replace with another single statement Statement(mir::Statement<'tcx>), - /// Replace with a list of statements - Statements(Vec>), -} - -impl<'tcx> StatementChange<'tcx> { - fn normalise(&mut self) { - let old = ::std::mem::replace(self, StatementChange::None); - *self = match old { - StatementChange::Statements(mut stmts) => { - match stmts.len() { - 0 => StatementChange::Remove, - 1 => StatementChange::Statement(stmts.pop().unwrap()), - _ => StatementChange::Statements(stmts) - } - } - o => o - } - } } pub trait Transfer<'tcx> { @@ -180,9 +148,7 @@ where T: Transfer<'tcx>, for stmt in &old_statements { // Given a fact and statement produce a new fact and optionally a replacement // graph. - let mut new_repl = rewrite.stmt(&stmt, &fact, cfg); - new_repl.normalise(); - match new_repl { + match rewrite.stmt(&stmt, &fact, cfg) { StatementChange::None => { fact = transfer.stmt(stmt, fact); cfg.push(new_graph, stmt.clone()); @@ -193,12 +159,6 @@ where T: Transfer<'tcx>, fact = transfer.stmt(&stmt, fact); cfg.push(new_graph, stmt); } - StatementChange::Statements(stmts) => { - changed = true; - for stmt in &stmts { fact = transfer.stmt(stmt, fact); } - cfg[new_graph].statements.extend(stmts); - } - } } // Swap the statements back in. From 6413fe69a667a6350aa2b6c54a4913493f7986fb Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:40:33 -0500 Subject: [PATCH 08/24] Let ar_forward generate its own queue --- src/librustc/mir/transform/dataflow.rs | 7 +++++-- src/librustc_mir/transform/acs_propagate.rs | 5 +---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index 83fb62a4c04fd..7e5b7c2fcb4bf 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -1,6 +1,6 @@ use mir::repr as mir; use mir::cfg::CFG; -use mir::repr::BasicBlock; +use mir::repr::{BasicBlock, START_BLOCK}; use rustc_data_structures::bitvec::BitVector; use mir::transform::lattice::Lattice; @@ -134,11 +134,14 @@ impl ::std::ops::IndexMut for Facts { } /// Analyse and rewrite using dataflow in the forward direction -pub fn ar_forward<'tcx, T, R>(cfg: &CFG<'tcx>, fs: Facts, mut queue: BitVector, transfer: T, rewrite: R) +pub fn ar_forward<'tcx, T, R>(cfg: &CFG<'tcx>, fs: Facts, transfer: T, rewrite: R) -> (CFG<'tcx>, Facts) where T: Transfer<'tcx>, R: Rewrite<'tcx, T::Lattice> { + let mut queue = BitVector::new(cfg.len()); + queue.insert(START_BLOCK.index()); + fixpoint(cfg, Direction::Forward, |bb, fact, cfg| { let new_graph = cfg.start_new_block(); let mut fact = fact.clone(); diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index 0aef37685acf2..8ac5cadb2cb3c 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -29,7 +29,6 @@ //! manually here. use rustc_data_structures::fnv::FnvHashMap; -use rustc_data_structures::bitvec::BitVector; use rustc::mir::repr::*; use rustc::mir::visit::{MutVisitor, LvalueContext}; use rustc::mir::transform::lattice::Lattice; @@ -66,11 +65,9 @@ impl Pass for ACSPropagate {} impl<'tcx> MirPass<'tcx> for ACSPropagate { fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { - let mut q = BitVector::new(mir.cfg.len()); - q.insert(START_BLOCK.index()); let ret = ar_forward( &mut mir.cfg, - Facts::new(), q, + Facts::new(), ACSPropagateTransfer, AliasRewrite.and_then(ConstRewrite).and_then(SimplifyRewrite) ); From 0756739ed5d69e767a8864db03571e71ca8f3361 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:58:27 -0500 Subject: [PATCH 09/24] Change the fully capitalized ACS to the partially capitalized Acs, matching the rest of rustc --- src/librustc_driver/driver.rs | 2 +- src/librustc_mir/transform/acs_propagate.rs | 42 ++++++++++----------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 6f09d3cabf4d4..13b18da1e96cd 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -967,7 +967,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); passes.push_pass(box mir::transform::qualify_consts::QualifyAndPromoteConstants); passes.push_pass(box mir::transform::type_check::TypeckMir); - passes.push_pass(box mir::transform::acs_propagate::ACSPropagate); + passes.push_pass(box mir::transform::acs_propagate::AcsPropagate); // passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg); passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); // And run everything. diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index 8ac5cadb2cb3c..4dea693488e47 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -57,18 +57,18 @@ impl<'tcx> Lattice for Either<'tcx> { } } -pub type ACSLattice<'a> = FnvHashMap, Either<'a>>; +pub type AcsLattice<'a> = FnvHashMap, Either<'a>>; -pub struct ACSPropagate; +pub struct AcsPropagate; -impl Pass for ACSPropagate {} +impl Pass for AcsPropagate {} -impl<'tcx> MirPass<'tcx> for ACSPropagate { +impl<'tcx> MirPass<'tcx> for AcsPropagate { fn run_pass<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource, mir: &mut Mir<'tcx>) { let ret = ar_forward( &mut mir.cfg, Facts::new(), - ACSPropagateTransfer, + AcsPropagateTransfer, AliasRewrite.and_then(ConstRewrite).and_then(SimplifyRewrite) ); mir.cfg = ret.0; @@ -77,12 +77,12 @@ impl<'tcx> MirPass<'tcx> for ACSPropagate { } -pub struct ACSPropagateTransfer; +pub struct AcsPropagateTransfer; -impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { - type Lattice = ACSLattice<'tcx>; +impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { + type Lattice = AcsLattice<'tcx>; - fn stmt(&self, s: &Statement<'tcx>, mut lat: ACSLattice<'tcx>) -> ACSLattice<'tcx> { + fn stmt(&self, s: &Statement<'tcx>, mut lat: AcsLattice<'tcx>) -> AcsLattice<'tcx> { let StatementKind::Assign(ref lval, ref rval) = s.kind; match *rval { Rvalue::Use(Operand::Consume(ref nlval)) => @@ -94,7 +94,7 @@ impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { lat } - fn term(&self, t: &Terminator<'tcx>, lat: ACSLattice<'tcx>) -> Vec> { + fn term(&self, t: &Terminator<'tcx>, lat: AcsLattice<'tcx>) -> Vec> { // FIXME: this should inspect the terminators and set their known values to constants. Esp. // for the if: in the truthy branch the operand is known to be true and in the falsy branch // the operand is known to be false. Now we just ignore the potential here. @@ -106,8 +106,8 @@ impl<'tcx> Transfer<'tcx> for ACSPropagateTransfer { pub struct AliasRewrite; -impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for AliasRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) +impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { + fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) -> StatementChange<'tcx> { let mut ns = s.clone(); let mut vis = RewriteAliasVisitor(&l, false); @@ -115,7 +115,7 @@ impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for AliasRewrite { if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } } - fn term(&self, t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { let mut nt = t.clone(); let mut vis = RewriteAliasVisitor(&l, false); @@ -124,7 +124,7 @@ impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for AliasRewrite { } } -struct RewriteAliasVisitor<'a, 'tcx: 'a>(pub &'a ACSLattice<'tcx>, pub bool); +struct RewriteAliasVisitor<'a, 'tcx: 'a>(pub &'a AcsLattice<'tcx>, pub bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { fn visit_lvalue(&mut self, lvalue: &mut Lvalue<'tcx>, context: LvalueContext) { match context { @@ -146,8 +146,8 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { pub struct ConstRewrite; -impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for ConstRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) +impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { + fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) -> StatementChange<'tcx> { let mut ns = s.clone(); let mut vis = RewriteConstVisitor(&l, false); @@ -155,7 +155,7 @@ impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for ConstRewrite { if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } } - fn term(&self, t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { let mut nt = t.clone(); let mut vis = RewriteConstVisitor(&l, false); @@ -164,7 +164,7 @@ impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for ConstRewrite { } } -struct RewriteConstVisitor<'a, 'tcx: 'a>(pub &'a ACSLattice<'tcx>, pub bool); +struct RewriteConstVisitor<'a, 'tcx: 'a>(pub &'a AcsLattice<'tcx>, pub bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { fn visit_operand(&mut self, op: &mut Operand<'tcx>) { let repl = if let Operand::Consume(ref lval) = *op { @@ -186,13 +186,13 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { pub struct SimplifyRewrite; -impl<'tcx> Rewrite<'tcx, ACSLattice<'tcx>> for SimplifyRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) +impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for SimplifyRewrite { + fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) -> StatementChange<'tcx> { StatementChange::None } - fn term(&self, t: &Terminator<'tcx>, l: &ACSLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { match t.kind { TerminatorKind::If { ref targets, .. } if targets.0 == targets.1 => { From a0eebd32f34eb6dccf11638d0bf7e62b762e8191 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Sun, 22 May 2016 13:52:43 -0500 Subject: [PATCH 10/24] Fix various nits in MIR Dataflow --- src/librustc/mir/transform/dataflow.rs | 8 +++--- src/librustc_mir/transform/acs_propagate.rs | 28 ++++++++++----------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index 7e5b7c2fcb4bf..a712c90dc5240 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -126,7 +126,7 @@ impl ::std::ops::Index for Facts { impl ::std::ops::IndexMut for Facts { fn index_mut(&mut self, index: BasicBlock) -> &mut F { - if let None = self.0.get_mut(index.index()) { + if self.0.get(index.index()).is_none() { self.put(index, ::bottom()); } self.0.get_mut(index.index()).unwrap() @@ -165,10 +165,10 @@ where T: Transfer<'tcx>, } } // Swap the statements back in. - ::std::mem::replace(&mut cfg[bb].statements, old_statements); + cfg[bb].statements = old_statements; // Handle the terminator replacement and transfer. - let terminator = ::std::mem::replace(&mut cfg[bb].terminator, None).unwrap(); + let terminator = cfg[bb].terminator.take().unwrap(); let repl = rewrite.term(&terminator, &fact, cfg); match repl { TerminatorChange::None => { @@ -180,7 +180,7 @@ where T: Transfer<'tcx>, } } let new_facts = transfer.term(cfg[new_graph].terminator(), fact); - ::std::mem::replace(&mut cfg[bb].terminator, Some(terminator)); + cfg[bb].terminator = Some(terminator); (if changed { Some(new_graph) } else { None }, new_facts) }, &mut queue, fs) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index 4dea693488e47..b7bbdae539c4a 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -24,7 +24,7 @@ //! For all of them we will be using a lattice of Hashmap from Lvalue to //! WTop> //! -//! My personal believ is that it should be possible to make a way to compose two hashmap lattices +//! My personal belief is that it should be possible to make a way to compose two hashmap lattices //! into one, but I can’t seem to get it just right yet, so we do the composing and decomposing //! manually here. @@ -107,7 +107,7 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { pub struct AliasRewrite; impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { let mut ns = s.clone(); let mut vis = RewriteAliasVisitor(&l, false); @@ -115,7 +115,7 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } } - fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { let mut nt = t.clone(); let mut vis = RewriteAliasVisitor(&l, false); @@ -130,13 +130,9 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { match context { LvalueContext::Store | LvalueContext::Call => {} _ => { - let replacement = self.0.get(lvalue); - match replacement { - Some(&Either::Lvalue(ref nlval)) => { - self.1 = true; - *lvalue = nlval.clone(); - } - _ => {} + if let Some(&Either::Lvalue(ref nlval)) = self.0.get(lvalue) { + self.1 = true; + *lvalue = nlval.clone(); } } } @@ -147,7 +143,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { pub struct ConstRewrite; impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { let mut ns = s.clone(); let mut vis = RewriteConstVisitor(&l, false); @@ -155,7 +151,7 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } } - fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { let mut nt = t.clone(); let mut vis = RewriteConstVisitor(&l, false); @@ -167,6 +163,7 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { struct RewriteConstVisitor<'a, 'tcx: 'a>(pub &'a AcsLattice<'tcx>, pub bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { fn visit_operand(&mut self, op: &mut Operand<'tcx>) { + // To satisy borrow checker, modify `op` after inspecting it let repl = if let Operand::Consume(ref lval) = *op { if let Some(&Either::Const(ref c)) = self.0.get(lval) { Some(c.clone()) @@ -179,6 +176,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { if let Some(c) = repl { *op = Operand::Constant(c); } + self.super_operand(op); } } @@ -186,13 +184,13 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { pub struct SimplifyRewrite; -impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for SimplifyRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) +impl<'tcx, L: Lattice> Rewrite<'tcx, L> for SimplifyRewrite { + fn stmt(&self, _: &Statement<'tcx>, _: &L, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { StatementChange::None } - fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, cfg: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, _: &L, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { match t.kind { TerminatorKind::If { ref targets, .. } if targets.0 == targets.1 => { From 7cbcb4b0b548e33d60bb05c60e10d0052f2d4914 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Sun, 22 May 2016 13:54:25 -0500 Subject: [PATCH 11/24] Actually rewrite constants in AcsPropagate --- src/librustc_mir/transform/acs_propagate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index b7bbdae539c4a..79e46e4d86b29 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -174,6 +174,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { None }; if let Some(c) = repl { + self.1 = true; *op = Operand::Constant(c); } From 4af1473b5b861dbda4f5acfc818e95713fce2cfa Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Tue, 24 May 2016 19:52:16 -0500 Subject: [PATCH 12/24] Remove some unnecessary `pub`s in AcsPropagate --- src/librustc_mir/transform/acs_propagate.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index 79e46e4d86b29..ebd35cf44bb5f 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -39,7 +39,7 @@ use rustc::middle::const_val::ConstVal; use pretty; #[derive(PartialEq, Debug, Eq, Clone)] -pub enum Either<'tcx> { +enum Either<'tcx> { Top, Lvalue(Lvalue<'tcx>), Const(Constant<'tcx>), @@ -57,7 +57,7 @@ impl<'tcx> Lattice for Either<'tcx> { } } -pub type AcsLattice<'a> = FnvHashMap, Either<'a>>; +type AcsLattice<'a> = FnvHashMap, Either<'a>>; pub struct AcsPropagate; @@ -77,7 +77,7 @@ impl<'tcx> MirPass<'tcx> for AcsPropagate { } -pub struct AcsPropagateTransfer; +struct AcsPropagateTransfer; impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { type Lattice = AcsLattice<'tcx>; @@ -104,7 +104,7 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { } } -pub struct AliasRewrite; +struct AliasRewrite; impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) @@ -124,7 +124,7 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { } } -struct RewriteAliasVisitor<'a, 'tcx: 'a>(pub &'a AcsLattice<'tcx>, pub bool); +struct RewriteAliasVisitor<'a, 'tcx: 'a>(&'a AcsLattice<'tcx>, bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { fn visit_lvalue(&mut self, lvalue: &mut Lvalue<'tcx>, context: LvalueContext) { match context { @@ -140,7 +140,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { } } -pub struct ConstRewrite; +struct ConstRewrite; impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) @@ -160,7 +160,7 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { } } -struct RewriteConstVisitor<'a, 'tcx: 'a>(pub &'a AcsLattice<'tcx>, pub bool); +struct RewriteConstVisitor<'a, 'tcx: 'a>(&'a AcsLattice<'tcx>, bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { fn visit_operand(&mut self, op: &mut Operand<'tcx>) { // To satisy borrow checker, modify `op` after inspecting it @@ -183,7 +183,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { } -pub struct SimplifyRewrite; +struct SimplifyRewrite; impl<'tcx, L: Lattice> Rewrite<'tcx, L> for SimplifyRewrite { fn stmt(&self, _: &Statement<'tcx>, _: &L, _: &mut CFG<'tcx>) From ad3753390d05077429a0c2255591eb8a6d9aa14a Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Tue, 24 May 2016 23:45:44 -0500 Subject: [PATCH 13/24] Invalidate values in AcsLattice that are overwritten, either by a write to a projected lvalue, or through the destination of a call. Projected lvalues are now entirely not tracked. With --enable-orbit, everything bootstraps except for denied warnings. --- src/librustc_mir/transform/acs_propagate.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index ebd35cf44bb5f..e9adde359e169 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -84,6 +84,15 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { fn stmt(&self, s: &Statement<'tcx>, mut lat: AcsLattice<'tcx>) -> AcsLattice<'tcx> { let StatementKind::Assign(ref lval, ref rval) = s.kind; + if let &Lvalue::Projection(_) = lval { + let mut base = lval; + while let &Lvalue::Projection(ref proj) = base { + base = &proj.base; + } + lat.insert(base.clone(), Either::Top); + return lat; + } + match *rval { Rvalue::Use(Operand::Consume(ref nlval)) => lat.insert(lval.clone(), Either::Lvalue(nlval.clone())), @@ -94,7 +103,11 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { lat } - fn term(&self, t: &Terminator<'tcx>, lat: AcsLattice<'tcx>) -> Vec> { + fn term(&self, t: &Terminator<'tcx>, mut lat: AcsLattice<'tcx>) -> Vec> { + if let TerminatorKind::Call { destination: Some((ref dest, _)), .. } = t.kind { + lat.insert(dest.clone(), Either::Top); + } + // FIXME: this should inspect the terminators and set their known values to constants. Esp. // for the if: in the truthy branch the operand is known to be true and in the falsy branch // the operand is known to be false. Now we just ignore the potential here. From 9b3ecda395bb169c3f8da26c68bfcb8f8d869aa3 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Tue, 24 May 2016 23:49:51 -0500 Subject: [PATCH 14/24] Temporarily completely disable Backward dataflow to fix unused variant warning --- src/librustc/mir/transform/dataflow.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index a712c90dc5240..0886a63d1b734 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -310,7 +310,7 @@ where T: Transfer<'tcx>, enum Direction { Forward, - Backward +// Backward } /// The fixpoint function is the engine of this whole thing. Important part of it is the `f: BF` @@ -349,16 +349,17 @@ where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (Option, Vec), } // Then we record the facts in the correct direction. - if let Direction::Forward = direction { - for (f, &target) in new_facts.into_iter() - .zip(cfg[block].terminator().successors().iter()) { - let facts_changed = Lattice::join(&mut init_facts[target], &f); - if facts_changed { - to_visit.insert(target.index()); + match direction { + Direction::Forward => { + for (f, &target) in new_facts.into_iter() + .zip(cfg[block].terminator().successors().iter()) { + let facts_changed = Lattice::join(&mut init_facts[target], &f); + if facts_changed { + to_visit.insert(target.index()); + } } } - } else { - unimplemented!() + // Direction::Backward => unimplemented!() // let mut new_facts = new_facts; // let fact = new_facts.pop().unwrap().1; // for pred in cfg.predecessors(block) { From b0fbbaea6b51f1bbfcebcf3251ad6aae85845ff6 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Thu, 26 May 2016 19:20:53 -0500 Subject: [PATCH 15/24] Rewrite AcsLattice to be more correct --- src/librustc_mir/transform/acs_propagate.rs | 141 ++++++++++++++------ 1 file changed, 100 insertions(+), 41 deletions(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index e9adde359e169..68d13adbaf215 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -28,6 +28,8 @@ //! into one, but I can’t seem to get it just right yet, so we do the composing and decomposing //! manually here. +use self::AcsLattice::*; + use rustc_data_structures::fnv::FnvHashMap; use rustc::mir::repr::*; use rustc::mir::visit::{MutVisitor, LvalueContext}; @@ -40,25 +42,49 @@ use pretty; #[derive(PartialEq, Debug, Eq, Clone)] enum Either<'tcx> { - Top, Lvalue(Lvalue<'tcx>), Const(Constant<'tcx>), } -impl<'tcx> Lattice for Either<'tcx> { - fn bottom() -> Self { unimplemented!() } +#[derive(Debug, Clone)] +enum AcsLattice<'tcx> { + Bottom, + Wrap(FnvHashMap, Either<'tcx>>) +} + +impl<'tcx> Lattice for AcsLattice<'tcx> { + fn bottom() -> Self { Bottom } fn join(&mut self, other: &Self) -> bool { - if self == other { - false - } else { - *self = Either::Top; - true + let other_map = match *other { + Bottom => return false, + Wrap(ref map) => map + }; + let self_map = match *self { + Bottom => { + *self = Wrap(other_map.clone()); + return true; + }, + Wrap(ref mut map) => map + }; + + let mut changed = false; + + for (k, v) in other_map { + let should_remove = if let Some(cur_v) = self_map.get(k) { + cur_v != v + } else { + false + }; + if should_remove { + self_map.remove(k); + changed = true; + } } + + changed } } -type AcsLattice<'a> = FnvHashMap, Either<'a>>; - pub struct AcsPropagate; impl Pass for AcsPropagate {} @@ -79,33 +105,46 @@ impl<'tcx> MirPass<'tcx> for AcsPropagate { struct AcsPropagateTransfer; +fn base_lvalue<'a, 'tcx>(mut lval: &'a Lvalue<'tcx>) -> &'a Lvalue<'tcx> { + while let &Lvalue::Projection(ref proj) = lval { + lval = &proj.base; + } + lval +} + impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { type Lattice = AcsLattice<'tcx>; - fn stmt(&self, s: &Statement<'tcx>, mut lat: AcsLattice<'tcx>) -> AcsLattice<'tcx> { + fn stmt(&self, s: &Statement<'tcx>, lat: AcsLattice<'tcx>) -> AcsLattice<'tcx> { + let mut lat_map = match lat { + Bottom => FnvHashMap::default(), + Wrap(map) => map + }; + let StatementKind::Assign(ref lval, ref rval) = s.kind; if let &Lvalue::Projection(_) = lval { - let mut base = lval; - while let &Lvalue::Projection(ref proj) = base { - base = &proj.base; - } - lat.insert(base.clone(), Either::Top); - return lat; + lat_map.remove(base_lvalue(lval)); + return Wrap(lat_map); } match *rval { Rvalue::Use(Operand::Consume(ref nlval)) => - lat.insert(lval.clone(), Either::Lvalue(nlval.clone())), + lat_map.insert(lval.clone(), Either::Lvalue(nlval.clone())), Rvalue::Use(Operand::Constant(ref c)) => - lat.insert(lval.clone(), Either::Const(c.clone())), - _ => lat.insert(lval.clone(), Either::Top) + lat_map.insert(lval.clone(), Either::Const(c.clone())), + _ => lat_map.remove(lval) }; - lat + Wrap(lat_map) } fn term(&self, t: &Terminator<'tcx>, mut lat: AcsLattice<'tcx>) -> Vec> { - if let TerminatorKind::Call { destination: Some((ref dest, _)), .. } = t.kind { - lat.insert(dest.clone(), Either::Top); + match t.kind { + TerminatorKind::Call { .. } | + TerminatorKind::Drop { .. } => { + // FIXME: Be smarter here by using an alias analysis + lat = Wrap(FnvHashMap::default()); + }, + _ => { } } // FIXME: this should inspect the terminators and set their known values to constants. Esp. @@ -122,22 +161,32 @@ struct AliasRewrite; impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { - let mut ns = s.clone(); - let mut vis = RewriteAliasVisitor(&l, false); - vis.visit_statement(START_BLOCK, &mut ns); - if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } + if let Wrap(ref map) = *l { + let mut ns = s.clone(); + let mut vis = RewriteAliasVisitor(map, false); + vis.visit_statement(START_BLOCK, &mut ns); + if vis.1 { + return StatementChange::Statement(ns); + } + } + StatementChange::None } fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { - let mut nt = t.clone(); - let mut vis = RewriteAliasVisitor(&l, false); - vis.visit_terminator(START_BLOCK, &mut nt); - if vis.1 { TerminatorChange::Terminator(nt) } else { TerminatorChange::None } + if let Wrap(ref map) = *l { + let mut nt = t.clone(); + let mut vis = RewriteAliasVisitor(map, false); + vis.visit_terminator(START_BLOCK, &mut nt); + if vis.1 { + return TerminatorChange::Terminator(nt); + } + } + TerminatorChange::None } } -struct RewriteAliasVisitor<'a, 'tcx: 'a>(&'a AcsLattice<'tcx>, bool); +struct RewriteAliasVisitor<'a, 'tcx: 'a>(&'a FnvHashMap, Either<'tcx>>, bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { fn visit_lvalue(&mut self, lvalue: &mut Lvalue<'tcx>, context: LvalueContext) { match context { @@ -158,22 +207,32 @@ struct ConstRewrite; impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { - let mut ns = s.clone(); - let mut vis = RewriteConstVisitor(&l, false); - vis.visit_statement(START_BLOCK, &mut ns); - if vis.1 { StatementChange::Statement(ns) } else { StatementChange::None } + if let Wrap(ref map) = *l { + let mut ns = s.clone(); + let mut vis = RewriteConstVisitor(map, false); + vis.visit_statement(START_BLOCK, &mut ns); + if vis.1 { + return StatementChange::Statement(ns); + } + } + StatementChange::None } fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { - let mut nt = t.clone(); - let mut vis = RewriteConstVisitor(&l, false); - vis.visit_terminator(START_BLOCK, &mut nt); - if vis.1 { TerminatorChange::Terminator(nt) } else { TerminatorChange::None } + if let Wrap(ref map) = *l { + let mut nt = t.clone(); + let mut vis = RewriteConstVisitor(map, false); + vis.visit_terminator(START_BLOCK, &mut nt); + if vis.1 { + return TerminatorChange::Terminator(nt); + } + } + TerminatorChange::None } } -struct RewriteConstVisitor<'a, 'tcx: 'a>(&'a AcsLattice<'tcx>, bool); +struct RewriteConstVisitor<'a, 'tcx: 'a>(&'a FnvHashMap, Either<'tcx>>, bool); impl<'a, 'tcx> MutVisitor<'tcx> for RewriteConstVisitor<'a, 'tcx> { fn visit_operand(&mut self, op: &mut Operand<'tcx>) { // To satisy borrow checker, modify `op` after inspecting it From 7f1d4c8e01c86d8022f49d083be8c05e09611207 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 30 May 2016 22:08:54 -0500 Subject: [PATCH 16/24] Further correctness improvements to AcsRewrite --- src/librustc_mir/transform/acs_propagate.rs | 46 +++++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index 68d13adbaf215..e81004f87305e 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -112,6 +112,31 @@ fn base_lvalue<'a, 'tcx>(mut lval: &'a Lvalue<'tcx>) -> &'a Lvalue<'tcx> { lval } +fn invalidate<'tcx>(map: &mut FnvHashMap, Either<'tcx>>, lval: &Lvalue<'tcx>) { + map.remove(lval); + + let mut repl = None; + + for (k, v) in &mut *map { + if let Either::Lvalue(ref mut nlval) = *v { + if nlval == lval { + match repl { + None => { + repl = Some(k.clone()) + }, + Some(ref r) => { + *nlval = r.clone(); + } + } + } + } + } + + if let Some(repl) = repl { + map.remove(&repl); + } +} + impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { type Lattice = AcsLattice<'tcx>; @@ -122,17 +147,20 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { }; let StatementKind::Assign(ref lval, ref rval) = s.kind; + invalidate(&mut lat_map, base_lvalue(lval)); + if let &Lvalue::Projection(_) = lval { - lat_map.remove(base_lvalue(lval)); return Wrap(lat_map); } match *rval { - Rvalue::Use(Operand::Consume(ref nlval)) => - lat_map.insert(lval.clone(), Either::Lvalue(nlval.clone())), - Rvalue::Use(Operand::Constant(ref c)) => - lat_map.insert(lval.clone(), Either::Const(c.clone())), - _ => lat_map.remove(lval) + Rvalue::Use(Operand::Consume(ref nlval)) => { + lat_map.insert(lval.clone(), Either::Lvalue(nlval.clone())); + }, + Rvalue::Use(Operand::Constant(ref c)) => { + lat_map.insert(lval.clone(), Either::Const(c.clone())); + }, + _ => { } }; Wrap(lat_map) } @@ -190,13 +218,13 @@ struct RewriteAliasVisitor<'a, 'tcx: 'a>(&'a FnvHashMap, Either<'tc impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { fn visit_lvalue(&mut self, lvalue: &mut Lvalue<'tcx>, context: LvalueContext) { match context { - LvalueContext::Store | LvalueContext::Call => {} - _ => { + LvalueContext::Consume => { if let Some(&Either::Lvalue(ref nlval)) = self.0.get(lvalue) { self.1 = true; *lvalue = nlval.clone(); } - } + }, + _ => { } } self.super_lvalue(lvalue, context); } From cfcf7cd38b1f5aaac3958bf7dcd13ca2dc0466d4 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 6 Jun 2016 22:45:33 -0500 Subject: [PATCH 17/24] Remove unused import left over from rebase --- src/librustc_mir/transform/simplify_cfg.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_mir/transform/simplify_cfg.rs b/src/librustc_mir/transform/simplify_cfg.rs index 1d370648aead5..e07b01678fc17 100644 --- a/src/librustc_mir/transform/simplify_cfg.rs +++ b/src/librustc_mir/transform/simplify_cfg.rs @@ -9,7 +9,6 @@ // except according to those terms. use rustc_data_structures::bitvec::BitVector; -use rustc::middle::const_val::ConstVal; use rustc::ty::TyCtxt; use rustc::mir::repr::*; use rustc::mir::transform::{MirPass, MirSource, Pass}; From ae0c91c59d586ca319c46e95d05de0cef76d8b18 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 11:29:38 -0500 Subject: [PATCH 18/24] Properly reset basic blocks optimized based on overspeculative information --- src/librustc/mir/transform/dataflow.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index 0886a63d1b734..9757a8bff26b3 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -316,7 +316,7 @@ enum Direction { /// The fixpoint function is the engine of this whole thing. Important part of it is the `f: BF` /// callback. This for each basic block and its facts has to produce a replacement graph and a /// bunch of facts which are to be joined with the facts in the graph elsewhere. -fn fixpoint<'tcx, F: Lattice, BF>(cfg: &CFG<'tcx>, +fn fixpoint<'tcx, F: Lattice, BF>(original_cfg: &CFG<'tcx>, direction: Direction, f: BF, to_visit: &mut BitVector, @@ -332,7 +332,7 @@ where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (Option, Vec), // Invariant: // * None of the already existing blocks in CFG may be modified; { - let mut cfg = cfg.clone(); + let mut cfg = original_cfg.clone(); while let Some(block) = to_visit.iter().next() { to_visit.remove(block); @@ -351,13 +351,19 @@ where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (Option, Vec), // Then we record the facts in the correct direction. match direction { Direction::Forward => { + let mut changed_targets = vec![]; + for (f, &target) in new_facts.into_iter() .zip(cfg[block].terminator().successors().iter()) { - let facts_changed = Lattice::join(&mut init_facts[target], &f); - if facts_changed { - to_visit.insert(target.index()); + if Lattice::join(&mut init_facts[target], &f) { + changed_targets.push(target); } } + + for target in changed_targets { + to_visit.insert(target.index()); + cfg[target].clone_from(&original_cfg[target]); + } } // Direction::Backward => unimplemented!() // let mut new_facts = new_facts; From ffc240622b0e3cf1d73ce83ae27d52b465d4657f Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 11:45:03 -0500 Subject: [PATCH 19/24] Implement MIR's CFG::swap correctly --- src/librustc/mir/cfg.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/cfg.rs b/src/librustc/mir/cfg.rs index 3b6df96ec4fe6..631d553327dbd 100644 --- a/src/librustc/mir/cfg.rs +++ b/src/librustc/mir/cfg.rs @@ -59,8 +59,19 @@ impl<'tcx> CFG<'tcx> { pub fn swap(&mut self, b1: BasicBlock, b2: BasicBlock) { - // TODO: find all the branches to b2 from subgraph starting at b2 and replace them with b1. - self.basic_blocks.swap(b1.index(), b2.index()); + if b1 != b2 { + for idx in 0..self.len() { + let bb = BasicBlock::new(idx); + for target in self.successors_mut(bb) { + if *target == b1 { + *target = b2; + } else if *target == b2 { + *target = b1; + } + } + } + self.basic_blocks.swap(b1.index(), b2.index()); + } } pub fn start_new_block(&mut self) -> BasicBlock { From 3bd906cf51f0f25b5548fc8613677df78186d31b Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 13:01:59 -0500 Subject: [PATCH 20/24] Mutably update blocks when analyzing instead of creating new ones. rustc bootstraps! --- src/librustc/mir/transform/dataflow.rs | 51 +++++++++++++------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index 9757a8bff26b3..e91c889401b80 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -143,46 +143,52 @@ where T: Transfer<'tcx>, queue.insert(START_BLOCK.index()); fixpoint(cfg, Direction::Forward, |bb, fact, cfg| { - let new_graph = cfg.start_new_block(); let mut fact = fact.clone(); let mut changed = false; // Swap out the vector of old statements for a duration of statement inspection. - let old_statements = ::std::mem::replace(&mut cfg[bb].statements, Vec::new()); - for stmt in &old_statements { + let mut statements = ::std::mem::replace(&mut cfg[bb].statements, Vec::new()); + + // FIXME(#25477): This could be implemented much more cleanly with `retain_mut` + let num_statements = statements.len(); + let mut num_removed = 0; + for stmt_idx in 0..num_statements { // Given a fact and statement produce a new fact and optionally a replacement // graph. - match rewrite.stmt(&stmt, &fact, cfg) { - StatementChange::None => { - fact = transfer.stmt(stmt, fact); - cfg.push(new_graph, stmt.clone()); + match rewrite.stmt(&statements[stmt_idx], &fact, cfg) { + StatementChange::None => {} + StatementChange::Remove => { + changed = true; + num_removed += 1; + continue; } - StatementChange::Remove => changed = true, - StatementChange::Statement(stmt) => { + StatementChange::Statement(new_stmt) => { changed = true; - fact = transfer.stmt(&stmt, fact); - cfg.push(new_graph, stmt); + fact = transfer.stmt(&new_stmt, fact); + statements[stmt_idx] = new_stmt; } } + statements.swap(stmt_idx - num_removed, stmt_idx); } + statements.truncate(num_statements - num_removed); + // Swap the statements back in. - cfg[bb].statements = old_statements; + cfg[bb].statements = statements; // Handle the terminator replacement and transfer. let terminator = cfg[bb].terminator.take().unwrap(); let repl = rewrite.term(&terminator, &fact, cfg); match repl { TerminatorChange::None => { - cfg[new_graph].terminator = Some(terminator.clone()); + cfg[bb].terminator = Some(terminator) } - TerminatorChange::Terminator(t) => { + TerminatorChange::Terminator(new_terminator) => { changed = true; - cfg[new_graph].terminator = Some(t); + cfg[bb].terminator = Some(new_terminator); } } - let new_facts = transfer.term(cfg[new_graph].terminator(), fact); - cfg[bb].terminator = Some(terminator); + let new_facts = transfer.term(cfg[bb].terminator(), fact); - (if changed { Some(new_graph) } else { None }, new_facts) + (changed, new_facts) }, &mut queue, fs) } @@ -323,7 +329,7 @@ fn fixpoint<'tcx, F: Lattice, BF>(original_cfg: &CFG<'tcx>, mut init_facts: Facts) -> (CFG<'tcx>, Facts) // TODO: we probably want to pass in a list of basicblocks as successors (predecessors in backward // fixpoing) and let BF return just a list of F. -where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (Option, Vec), +where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (bool, Vec), // ^~ This function given a single block and fact before it optionally produces a replacement // graph (if not, the original block is the “replacement graph”) for the block and a list of // facts for arbitrary blocks (most likely for the blocks in the replacement graph and blocks @@ -338,16 +344,11 @@ where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (Option, Vec), to_visit.remove(block); let block = BasicBlock::new(block); - let (new_target, new_facts) = { + let (_changed, new_facts) = { let fact = &mut init_facts[block]; f(block, fact, &mut cfg) }; - // First of all, we merge in the replacement graph, if any. - if let Some(replacement_bb) = new_target { - cfg.swap(replacement_bb, block); - } - // Then we record the facts in the correct direction. match direction { Direction::Forward => { From 07d073e73da6ed966d713ed78204f2e8f9242743 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 13:22:57 -0500 Subject: [PATCH 21/24] Fix make tidy --- src/librustc/mir/cfg.rs | 10 +++++ src/librustc/mir/transform/dataflow.rs | 51 ++++++++++++++++++-------- src/librustc/mir/transform/lattice.rs | 10 +++++ 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/librustc/mir/cfg.rs b/src/librustc/mir/cfg.rs index 631d553327dbd..65d8e832ec010 100644 --- a/src/librustc/mir/cfg.rs +++ b/src/librustc/mir/cfg.rs @@ -1,3 +1,13 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + use mir::repr::*; use std::ops::{Index, IndexMut}; diff --git a/src/librustc/mir/transform/dataflow.rs b/src/librustc/mir/transform/dataflow.rs index e91c889401b80..6f12fba5b126b 100644 --- a/src/librustc/mir/transform/dataflow.rs +++ b/src/librustc/mir/transform/dataflow.rs @@ -1,3 +1,13 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + use mir::repr as mir; use mir::cfg::CFG; use mir::repr::{BasicBlock, START_BLOCK}; @@ -205,7 +215,10 @@ where T: Transfer<'tcx>, // // impl<'tcx, P> MirPass<'tcx> for ForwardDataflow

// where P: DataflowPass<'tcx> { -// fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut mir::Mir<'tcx>) { +// fn run_pass<'a>(&mut self, +// _: TyCtxt<'a, 'tcx, 'tcx>, +// _: MirSource, +// mir: &mut mir::Mir<'tcx>) { // let facts: Facts<

>::Lattice> = // Facts::new(

>::input_fact()); // let (new_cfg, _) = self.arf_body(&mir.cfg, facts, mir::START_BLOCK); @@ -230,12 +243,17 @@ where T: Transfer<'tcx>, // // impl<'tcx, P> MirPass<'tcx> for BackwardDataflow

// where P: DataflowPass<'tcx> { -// fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut mir::Mir<'tcx>) { +// fn run_pass<'a>(&mut self, +// _: TyCtxt<'a, 'tcx, 'tcx>, +// _: MirSource, +// mir: &mut mir::Mir<'tcx>) { // let mut facts: Facts<

>::Lattice> = // Facts::new(

>::input_fact()); -// // The desired effect here is that we should begin flowing from the blocks which terminate +// // The desired effect here is that we should begin flowing from the blocks which +// // terminate // // the control flow (return, resume, calls of diverging functions, non-terminating loops -// // etc), but finding them all is a pain, so we just get the list of graph nodes postorder +// // etc), but finding them all is a pain, so we just get the list of graph nodes +// // postorder // // and inspect them all! Perhaps not very effective, but certainly correct. // let start_at = postorder(mir).filter(|&(_, d)| !d.is_cleanup).map(|(bb, _)| { // facts.put(bb,

>::Lattice::bottom()); @@ -249,26 +267,29 @@ where T: Transfer<'tcx>, // impl<'tcx, P> BackwardDataflow

// where P: DataflowPass<'tcx> { // fn arb_body(&self, cfg: &CFG<'tcx>, -// facts: Facts<

>::Lattice>, mut map: HashMap) +// facts: Facts<

>::Lattice>, +// mut map: HashMap) // -> (CFG<'tcx>, Facts<

>::Lattice>){ // fixpoint(cfg, Direction::Backward, |bb, fact, cfg| { // let new_graph = cfg.start_new_block(); // let mut fact = fact.clone(); -// // This is a reverse thing so we inspect the terminator first and statements in reverse -// // order later. +// // This is a reverse thing so we inspect the terminator first and statements +// // in reverse order later. // // // // Handle the terminator replacement and transfer. // let terminator = ::std::mem::replace(&mut cfg[bb].terminator, None).unwrap(); // let repl = P::rewrite_term(&terminator, &fact, cfg); -// // TODO: this really needs to get factored out +// // FIXME: this really needs to get factored out // let mut new_facts = match repl { // TerminatorReplacement::Terminator(t) => { // cfg[new_graph].terminator = Some(t); // P::transfer_term(cfg[new_graph].terminator(), fact) // } // TerminatorReplacement::Graph(from, _) => { -// // FIXME: a more optimal approach would be to copy the from to the tail of our -// // new_graph. (1 less extra block). However there’s a problem with inspecting +// // FIXME: a more optimal approach would be to copy the from to the tail of +// // our +// // new_graph. (1 less extra block). However there’s a problem with +// // inspecting // // the statements of the merged block, because we just did the statements // // for this block already. // cfg.terminate(new_graph, terminator.scope, terminator.span, @@ -327,13 +348,13 @@ fn fixpoint<'tcx, F: Lattice, BF>(original_cfg: &CFG<'tcx>, f: BF, to_visit: &mut BitVector, mut init_facts: Facts) -> (CFG<'tcx>, Facts) -// TODO: we probably want to pass in a list of basicblocks as successors (predecessors in backward -// fixpoing) and let BF return just a list of F. +// FIXME: we probably want to pass in a list of basicblocks as successors (predecessors in +// backward fixpoint) and let BF return just a list of F. where BF: Fn(BasicBlock, &F, &mut CFG<'tcx>) -> (bool, Vec), // ^~ This function given a single block and fact before it optionally produces a replacement - // graph (if not, the original block is the “replacement graph”) for the block and a list of - // facts for arbitrary blocks (most likely for the blocks in the replacement graph and blocks - // into which data flows from the replacement graph) + // graph (if not, the original block is the “replacement graph”) for the block and a + // list of facts for arbitrary blocks (most likely for the blocks in the replacement graph + // and blocks into which data flows from the replacement graph) // // Invariant: // * None of the already existing blocks in CFG may be modified; diff --git a/src/librustc/mir/transform/lattice.rs b/src/librustc/mir/transform/lattice.rs index 9f6cda4abdcc9..e0c6f43164c6a 100644 --- a/src/librustc/mir/transform/lattice.rs +++ b/src/librustc/mir/transform/lattice.rs @@ -1,3 +1,13 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + use std::fmt::{Debug, Formatter}; use std::collections::hash_map::Entry; use std::collections::HashMap; From 282d51d62ec6ab3bdf00ff974071a937f2bc3bab Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 16:47:50 -0500 Subject: [PATCH 22/24] Also clear the Acs cache on DropAndReplace --- src/librustc_mir/transform/acs_propagate.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index e81004f87305e..e69290470f883 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -168,7 +168,8 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { fn term(&self, t: &Terminator<'tcx>, mut lat: AcsLattice<'tcx>) -> Vec> { match t.kind { TerminatorKind::Call { .. } | - TerminatorKind::Drop { .. } => { + TerminatorKind::Drop { .. } | + TerminatorKind::DropAndReplace { .. } => { // FIXME: Be smarter here by using an alias analysis lat = Wrap(FnvHashMap::default()); }, From f4452c888ec5372e5a150192c620502bf01d9e44 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 17:43:15 -0500 Subject: [PATCH 23/24] Use the generic WBottom for AcsLattice --- src/librustc_mir/transform/acs_propagate.rs | 80 ++++++++++----------- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index e69290470f883..c60ebf0c93e88 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -28,12 +28,10 @@ //! into one, but I can’t seem to get it just right yet, so we do the composing and decomposing //! manually here. -use self::AcsLattice::*; - use rustc_data_structures::fnv::FnvHashMap; use rustc::mir::repr::*; use rustc::mir::visit::{MutVisitor, LvalueContext}; -use rustc::mir::transform::lattice::Lattice; +use rustc::mir::transform::lattice::{Lattice, WBottom}; use rustc::mir::transform::dataflow::*; use rustc::mir::transform::{Pass, MirPass, MirSource}; use rustc::ty::TyCtxt; @@ -47,36 +45,23 @@ enum Either<'tcx> { } #[derive(Debug, Clone)] -enum AcsLattice<'tcx> { - Bottom, - Wrap(FnvHashMap, Either<'tcx>>) +struct AcsLattice<'tcx> { + known_values: FnvHashMap, Either<'tcx>> } impl<'tcx> Lattice for AcsLattice<'tcx> { - fn bottom() -> Self { Bottom } + fn bottom() -> Self { unimplemented!() } fn join(&mut self, other: &Self) -> bool { - let other_map = match *other { - Bottom => return false, - Wrap(ref map) => map - }; - let self_map = match *self { - Bottom => { - *self = Wrap(other_map.clone()); - return true; - }, - Wrap(ref mut map) => map - }; - let mut changed = false; - for (k, v) in other_map { - let should_remove = if let Some(cur_v) = self_map.get(k) { + for (k, v) in &other.known_values { + let should_remove = if let Some(cur_v) = self.known_values.get(k) { cur_v != v } else { false }; if should_remove { - self_map.remove(k); + self.known_values.remove(k); changed = true; } } @@ -138,19 +123,21 @@ fn invalidate<'tcx>(map: &mut FnvHashMap, Either<'tcx>>, lval: &Lva } impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { - type Lattice = AcsLattice<'tcx>; + type Lattice = WBottom>; - fn stmt(&self, s: &Statement<'tcx>, lat: AcsLattice<'tcx>) -> AcsLattice<'tcx> { + fn stmt(&self, s: &Statement<'tcx>, lat: WBottom>) -> WBottom> { let mut lat_map = match lat { - Bottom => FnvHashMap::default(), - Wrap(map) => map + WBottom::Bottom => FnvHashMap::default(), + WBottom::Value(lat) => lat.known_values }; let StatementKind::Assign(ref lval, ref rval) = s.kind; invalidate(&mut lat_map, base_lvalue(lval)); if let &Lvalue::Projection(_) = lval { - return Wrap(lat_map); + return WBottom::Value(AcsLattice { + known_values: lat_map + }); } match *rval { @@ -162,16 +149,21 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { }, _ => { } }; - Wrap(lat_map) + + WBottom::Value(AcsLattice { + known_values: lat_map + }) } - fn term(&self, t: &Terminator<'tcx>, mut lat: AcsLattice<'tcx>) -> Vec> { + fn term(&self, t: &Terminator<'tcx>, mut lat: WBottom>) -> Vec>> { match t.kind { TerminatorKind::Call { .. } | TerminatorKind::Drop { .. } | TerminatorKind::DropAndReplace { .. } => { // FIXME: Be smarter here by using an alias analysis - lat = Wrap(FnvHashMap::default()); + lat = WBottom::Value(AcsLattice { + known_values: FnvHashMap::default() + }); }, _ => { } } @@ -187,12 +179,12 @@ impl<'tcx> Transfer<'tcx> for AcsPropagateTransfer { struct AliasRewrite; -impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) +impl<'tcx> Rewrite<'tcx, WBottom>> for AliasRewrite { + fn stmt(&self, s: &Statement<'tcx>, l: &WBottom>, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { - if let Wrap(ref map) = *l { + if let &WBottom::Value(ref lat) = l { let mut ns = s.clone(); - let mut vis = RewriteAliasVisitor(map, false); + let mut vis = RewriteAliasVisitor(&lat.known_values, false); vis.visit_statement(START_BLOCK, &mut ns); if vis.1 { return StatementChange::Statement(ns); @@ -201,11 +193,11 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for AliasRewrite { StatementChange::None } - fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &WBottom>, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { - if let Wrap(ref map) = *l { + if let &WBottom::Value(ref lat) = l { let mut nt = t.clone(); - let mut vis = RewriteAliasVisitor(map, false); + let mut vis = RewriteAliasVisitor(&lat.known_values, false); vis.visit_terminator(START_BLOCK, &mut nt); if vis.1 { return TerminatorChange::Terminator(nt); @@ -233,12 +225,12 @@ impl<'a, 'tcx> MutVisitor<'tcx> for RewriteAliasVisitor<'a, 'tcx> { struct ConstRewrite; -impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { - fn stmt(&self, s: &Statement<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) +impl<'tcx> Rewrite<'tcx, WBottom>> for ConstRewrite { + fn stmt(&self, s: &Statement<'tcx>, l: &WBottom>, _: &mut CFG<'tcx>) -> StatementChange<'tcx> { - if let Wrap(ref map) = *l { + if let &WBottom::Value(ref lat) = l { let mut ns = s.clone(); - let mut vis = RewriteConstVisitor(map, false); + let mut vis = RewriteConstVisitor(&lat.known_values, false); vis.visit_statement(START_BLOCK, &mut ns); if vis.1 { return StatementChange::Statement(ns); @@ -247,11 +239,11 @@ impl<'tcx> Rewrite<'tcx, AcsLattice<'tcx>> for ConstRewrite { StatementChange::None } - fn term(&self, t: &Terminator<'tcx>, l: &AcsLattice<'tcx>, _: &mut CFG<'tcx>) + fn term(&self, t: &Terminator<'tcx>, l: &WBottom>, _: &mut CFG<'tcx>) -> TerminatorChange<'tcx> { - if let Wrap(ref map) = *l { + if let &WBottom::Value(ref lat) = l { let mut nt = t.clone(); - let mut vis = RewriteConstVisitor(map, false); + let mut vis = RewriteConstVisitor(&lat.known_values, false); vis.visit_terminator(START_BLOCK, &mut nt); if vis.1 { return TerminatorChange::Terminator(nt); From c045fa409884f129c4f6f1584f7309d17cd4b1b7 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Wed, 8 Jun 2016 19:03:57 -0500 Subject: [PATCH 24/24] Properly implement intersection in AcsLattice::join --- src/librustc_mir/transform/acs_propagate.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/transform/acs_propagate.rs b/src/librustc_mir/transform/acs_propagate.rs index c60ebf0c93e88..6cbde390a1517 100644 --- a/src/librustc_mir/transform/acs_propagate.rs +++ b/src/librustc_mir/transform/acs_propagate.rs @@ -52,21 +52,19 @@ struct AcsLattice<'tcx> { impl<'tcx> Lattice for AcsLattice<'tcx> { fn bottom() -> Self { unimplemented!() } fn join(&mut self, other: &Self) -> bool { - let mut changed = false; + let mut to_remove = vec![]; - for (k, v) in &other.known_values { - let should_remove = if let Some(cur_v) = self.known_values.get(k) { - cur_v != v - } else { - false - }; - if should_remove { - self.known_values.remove(k); - changed = true; + for (k, v) in &self.known_values { + if other.known_values.get(k).map_or(true, |other_v| other_v != v) { + to_remove.push(k.clone()); } } - changed + for k in &to_remove { + self.known_values.remove(k); + } + + !to_remove.is_empty() } }