From d4b132a7dbe6c611acf946933d762697eae28eb1 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 05:46:52 -0500 Subject: [PATCH 01/14] Remove use of specialization in Lattice to avoid use of unstable features --- src/librustc/mir/transform/lattice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/transform/lattice.rs b/src/librustc/mir/transform/lattice.rs index 129291b349b7..9f6cda4abdcc 100644 --- a/src/librustc/mir/transform/lattice.rs +++ b/src/librustc/mir/transform/lattice.rs @@ -23,7 +23,7 @@ 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), (&mut WTop::Top, _) => false, From 8fb7a1c6590090e3b5595dd4d54c2a00473089ac Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:06:44 -0500 Subject: [PATCH 02/14] 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 508f74e356a0..f56fde71b52b 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 38ce707f4edc..aaae35efb396 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 9a46cd7eb832c234574a0167a420a4e6bc77deef Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:25:02 -0500 Subject: [PATCH 03/14] 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 f56fde71b52b..181d31e89de3 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 aaae35efb396..0aef37685acf 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 ca08fdf51aecd10fe2a992226f0f466748880160 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:29:59 -0500 Subject: [PATCH 04/14] 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 181d31e89de3..83fb62a4c04f 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 c36dc329c561bc7fc35b95ed10349bb17793c39c Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:40:33 -0500 Subject: [PATCH 05/14] 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 83fb62a4c04f..7e5b7c2fcb4b 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 0aef37685acf..8ac5cadb2cb3 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 ca5258657a9e9f952cd9d0a380d4480a2f879477 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 16 May 2016 06:58:27 -0500 Subject: [PATCH 06/14] 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 d74eb959af8e..71b24958f898 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -939,7 +939,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 8ac5cadb2cb3..4dea693488e4 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 a1cf39a6a22d34c15c29caa7f70a0c4442db55c4 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Sun, 22 May 2016 10:29:30 -0500 Subject: [PATCH 07/14] Fix infinite loop in SimplifyCfg and re-enable it --- src/librustc_driver/driver.rs | 2 +- src/librustc_mir/transform/simplify_cfg.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 71b24958f898..95759efae3b5 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -940,7 +940,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, 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::simplify_cfg::SimplifyCfg); + 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/simplify_cfg.rs b/src/librustc_mir/transform/simplify_cfg.rs index 228fca054c23..76f5c9ec9feb 100644 --- a/src/librustc_mir/transform/simplify_cfg.rs +++ b/src/librustc_mir/transform/simplify_cfg.rs @@ -83,7 +83,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyCfg { while changed { pretty::dump_mir(tcx, "simplify_cfg", &counter, src, mir, None); counter += 1; - changed |= self.remove_goto_chains(mir); + changed = self.remove_goto_chains(mir); RemoveDeadBlocks.run_pass(tcx, src, mir); } // FIXME: Should probably be moved into some kind of pass manager From e2d48c5c96466afea5479f0d428ad4e7472548c5 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Sun, 22 May 2016 13:52:43 -0500 Subject: [PATCH 08/14] 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 7e5b7c2fcb4b..a712c90dc524 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 4dea693488e4..b7bbdae539c4 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 50697a50f9fb4e05871aef0f466e37d4d5d5145e Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Sun, 22 May 2016 13:54:25 -0500 Subject: [PATCH 09/14] 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 b7bbdae539c4..79e46e4d86b2 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 a40d9c13f5ef82327d7d63217af8241c32845aa3 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Tue, 24 May 2016 19:52:16 -0500 Subject: [PATCH 10/14] 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 79e46e4d86b2..ebd35cf44bb5 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 52d150f5b22ba9954e243f9903ba06b81ab5ed59 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Tue, 24 May 2016 23:45:44 -0500 Subject: [PATCH 11/14] 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 ebd35cf44bb5..e9adde359e16 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 92628f79cc7c27054e10c5f74564e22cf047ecee Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Tue, 24 May 2016 23:49:51 -0500 Subject: [PATCH 12/14] 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 a712c90dc524..0886a63d1b73 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 ab1d316baca658240471b1980706ba87e5a8121a Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Thu, 26 May 2016 19:20:53 -0500 Subject: [PATCH 13/14] 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 e9adde359e16..68d13adbaf21 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 d5c74f8c281caad4d92e17fd13e2b5f7be803c7d Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 30 May 2016 22:08:54 -0500 Subject: [PATCH 14/14] 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 68d13adbaf21..e81004f87305 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); }