From e87ab56e79ee50cd54e57c7e019c747710234876 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 28 Nov 2017 08:24:38 +0200 Subject: [PATCH] rustc_mir: implement an "lvalue reuse" optimization (aka destination propagation aka NRVO). --- src/librustc_mir/transform/mod.rs | 8 +- src/librustc_mir/transform/reuse_lvalues.rs | 288 ++++++++++++++++++ src/test/codegen/align-struct.rs | 3 + src/test/codegen/zip.rs | 2 +- src/test/mir-opt/copy_propagation.rs | 9 +- .../mir-opt/inline-closure-borrows-arg.rs | 8 +- src/test/mir-opt/inline-closure.rs | 2 +- 7 files changed, 306 insertions(+), 14 deletions(-) create mode 100644 src/librustc_mir/transform/reuse_lvalues.rs diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 418d3d220581e..6a7a4beea90a9 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -44,6 +44,7 @@ pub mod generator; pub mod inline; pub mod nll; pub mod lower_128bit; +pub mod reuse_lvalues; pub(crate) fn provide(providers: &mut Providers) { self::qualify_consts::provide(providers); @@ -252,12 +253,17 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx // Optimizations begin. inline::Inline, + + // Lowering generator control-flow and variables + // has to happen before we do anything else to them. + generator::StateTransform, + instcombine::InstCombine, deaggregator::Deaggregator, + reuse_lvalues::ReuseLvalues, copy_prop::CopyPropagation, simplify::SimplifyLocals, - generator::StateTransform, add_call_guards::CriticalCallEdges, dump_mir::Marker("PreTrans"), ]; diff --git a/src/librustc_mir/transform/reuse_lvalues.rs b/src/librustc_mir/transform/reuse_lvalues.rs new file mode 100644 index 0000000000000..8d9ff1b674e05 --- /dev/null +++ b/src/librustc_mir/transform/reuse_lvalues.rs @@ -0,0 +1,288 @@ +// Copyright 2017 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. + +//! A pass that reuses "final destinations" of values, +//! propagating the lvalue back through a chain of moves. + +use rustc::hir; +use rustc::mir::*; +use rustc::mir::visit::{LvalueContext, MutVisitor, Visitor}; +use rustc::session::config::FullDebugInfo; +use rustc::ty::TyCtxt; +use transform::{MirPass, MirSource}; + +use rustc_data_structures::indexed_vec::IndexVec; + +pub struct ReuseLvalues; + +impl MirPass for ReuseLvalues { + fn run_pass<'a, 'tcx>(&self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + source: MirSource, + mir: &mut Mir<'tcx>) { + // Don't run on constant MIR, because trans might not be able to + // evaluate the modified MIR. + // FIXME(eddyb) Remove check after miri is merged. + let id = tcx.hir.as_local_node_id(source.def_id).unwrap(); + match (tcx.hir.body_owner_kind(id), source.promoted) { + (_, Some(_)) | + (hir::BodyOwnerKind::Const, _) | + (hir::BodyOwnerKind::Static(_), _) => return, + + (hir::BodyOwnerKind::Fn, _) => { + if tcx.is_const_fn(source.def_id) { + // Don't run on const functions, as, again, trans might not be able to evaluate + // the optimized IR. + return + } + } + } + + // FIXME(eddyb) We should allow multiple user variables + // per local for debuginfo instead of not optimizing. + if tcx.sess.opts.debuginfo == FullDebugInfo { + return; + } + + // Collect the def and move information for all locals. + let mut collector = DefMoveCollector { + defs_moves: IndexVec::from_elem((Def::Never, Move::Never), &mir.local_decls), + }; + for arg in mir.args_iter() { + // Arguments are special because they're initialized + // in callers, and the collector doesn't know this. + collector.defs_moves[arg].0 = Def::Other; + } + collector.visit_mir(mir); + let mut defs_moves = collector.defs_moves; + + // Keep only locals with a clear initialization and move, + // as they are guaranteed to have all accesses in between. + // Also, the destination local of the move has to also have + // a single initialization (the move itself), otherwise + // there could be accesses that overlap the move chain. + for local in mir.local_decls.indices() { + let (def, mov) = defs_moves[local]; + if let Move::Into(dest) = mov { + if let Def::Once { .. } = def { + if let (Def::Once { ref mut reused }, _) = defs_moves[dest] { + *reused = true; + continue; + } + } + + // Failed to confirm the destination. + defs_moves[local].1 = Move::Other; + } + } + + // Apply the appropriate replacements. + let mut replacer = Replacer { + defs_moves + }; + replacer.visit_mir(mir); + } +} + +/// How (many times) was a local written? +/// Note that borrows are completely ignored, +/// as they get invalidated by moves regardless. +#[derive(Copy, Clone, Debug)] +enum Def { + /// No writes to this local. + Never, + + /// Only one direct initialization, from `Assign` or `Call`. + /// + /// If `reused` is `true`, this local is a destination + /// in a chain of moves and should have all of its + /// `StorageLive` statements removed. + Once { + reused: bool + }, + + /// Any other number or kind of mutating accesses. + Other, +} + +/// How (many times) was a local moved? +#[derive(Copy, Clone, Debug)] +enum Move { + /// No moves of this local. + Never, + + /// Only one move, assigning another local. + /// + /// Ends up replaced by its destination and should + /// have all of its `StorageDead` statements removed. + Into(Local), + + /// Any other number or kind of moves. + Other, +} + + +struct DefMoveCollector { + defs_moves: IndexVec, +} + +impl<'tcx> Visitor<'tcx> for DefMoveCollector { + fn visit_local(&mut self, + &local: &Local, + context: LvalueContext<'tcx>, + _location: Location) { + let (ref mut def, ref mut mov) = self.defs_moves[local]; + match context { + // We specifically want the first direct initialization. + LvalueContext::Store | + LvalueContext::Call => { + if let Def::Never = *def { + *def = Def::Once { reused: false }; + } else { + *def = Def::Other; + } + } + + // Initialization of a field or similar. + LvalueContext::Projection(Mutability::Mut) => { + *def = Def::Other; + } + + // Both of these count as moved, and not the kind + // we want for `Move::Into` (see `visit_assign`). + LvalueContext::Drop | + LvalueContext::Move => *mov = Move::Other, + + // We can ignore everything else. + LvalueContext::Inspect | + LvalueContext::Copy | + LvalueContext::Projection(Mutability::Not) | + LvalueContext::Borrow { .. } | + LvalueContext::StorageLive | + LvalueContext::StorageDead | + LvalueContext::Validate => {} + } + } + + fn visit_projection(&mut self, + proj: &LvalueProjection<'tcx>, + context: LvalueContext<'tcx>, + location: Location) { + // Pretend derefs copy the underlying pointer, as we don't + // need to treat the base local as being mutated or moved. + let context = if let ProjectionElem::Deref = proj.elem { + LvalueContext::Copy + } else { + match context { + // Pass down the kinds of contexts for which we don't + // need to disambigutate between direct and projected. + LvalueContext::Borrow { .. } | + LvalueContext::Copy | + LvalueContext::Move | + LvalueContext::Drop => context, + + _ if context.is_mutating_use() => { + LvalueContext::Projection(Mutability::Mut) + } + _ => { + LvalueContext::Projection(Mutability::Not) + } + } + }; + self.visit_lvalue(&proj.base, context, location); + self.visit_projection_elem(&proj.elem, context, location); + } + + fn visit_assign(&mut self, + _: BasicBlock, + lvalue: &Lvalue<'tcx>, + rvalue: &Rvalue<'tcx>, + location: Location) { + self.visit_lvalue(lvalue, LvalueContext::Store, location); + + // Handle `dest = move src`, and skip the `visit_local` + // for `src`, which would always set it to `Move::Other`. + match (lvalue, rvalue) { + (&Lvalue::Local(dest), &Rvalue::Use(Operand::Move(Lvalue::Local(src)))) => { + let (_, ref mut mov) = self.defs_moves[src]; + // We specifically want the first whole move into another local. + if let Move::Never = *mov { + *mov = Move::Into(dest); + } else { + *mov = Move::Other; + } + } + _ => { + self.visit_rvalue(rvalue, location); + } + } + } +} + +struct Replacer { + defs_moves: IndexVec, +} + +impl<'a, 'tcx> MutVisitor<'tcx> for Replacer { + fn visit_local(&mut self, + local: &mut Local, + context: LvalueContext<'tcx>, + location: Location) { + let original = *local; + if let (_, Move::Into(dest)) = self.defs_moves[original] { + *local = dest; + + // Keep going, in case the move chain doesn't stop here. + self.visit_local(local, context, location); + + // Cache the final result, in a similar way to union-find. + let final_dest = *local; + if final_dest != dest { + self.defs_moves[original].1 = Move::Into(final_dest); + } + } + } + + fn visit_statement(&mut self, + block: BasicBlock, + statement: &mut Statement<'tcx>, + location: Location) { + // Fuse storage liveness ranges of move chains, by removing + // `StorageLive` of destinations and `StorageDead` of sources. + match statement.kind { + StatementKind::StorageLive(local) | + StatementKind::StorageDead(local) => { + // FIXME(eddyb) We also have to remove `StorageLive` of + // sources and `StorageDead` of destinations to avoid + // creating invalid storage liveness (half-)ranges. + // The proper solution might involve recomputing them. + match self.defs_moves[local] { + (Def::Once { reused: true }, _) | + (_, Move::Into(_)) => { + statement.kind = StatementKind::Nop; + } + _ => {} + } + } + _ => {} + } + + self.super_statement(block, statement, location); + + // Remove self-assignments resulting from replaced move chains. + match statement.kind { + StatementKind::Assign(Lvalue::Local(dest), + Rvalue::Use(Operand::Move(Lvalue::Local(src)))) if dest == src => { + statement.kind = StatementKind::Nop; + } + _ => {} + } + } +} diff --git a/src/test/codegen/align-struct.rs b/src/test/codegen/align-struct.rs index ba81e2d6046e8..b743b55788533 100644 --- a/src/test/codegen/align-struct.rs +++ b/src/test/codegen/align-struct.rs @@ -15,8 +15,10 @@ #![feature(repr_align)] #[repr(align(64))] +#[derive(Copy, Clone)] pub struct Align64(i32); +#[derive(Copy, Clone)] pub struct Nested64 { a: Align64, b: i32, @@ -24,6 +26,7 @@ pub struct Nested64 { d: i8, } +#[derive(Copy, Clone)] pub enum Enum64 { A(Align64), B(i32), diff --git a/src/test/codegen/zip.rs b/src/test/codegen/zip.rs index d0051c5165fe1..e9088cb0d53cf 100644 --- a/src/test/codegen/zip.rs +++ b/src/test/codegen/zip.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags: -C no-prepopulate-passes -O +// compile-flags: -O #![crate_type = "lib"] diff --git a/src/test/mir-opt/copy_propagation.rs b/src/test/mir-opt/copy_propagation.rs index 50d8a5154c449..067a937b0b30c 100644 --- a/src/test/mir-opt/copy_propagation.rs +++ b/src/test/mir-opt/copy_propagation.rs @@ -22,12 +22,9 @@ fn main() { // START rustc.test.CopyPropagation.before.mir // bb0: { // ... -// _3 = _1; +// _2 = _1; // ... -// _2 = move _3; -// ... -// _4 = _2; -// _0 = move _4; +// _0 = _2; // ... // return; // } @@ -35,7 +32,7 @@ fn main() { // START rustc.test.CopyPropagation.after.mir // bb0: { // ... -// _0 = move _1; +// _0 = _1; // ... // return; // } diff --git a/src/test/mir-opt/inline-closure-borrows-arg.rs b/src/test/mir-opt/inline-closure-borrows-arg.rs index 3fb54f90984dd..6ce51be3ec5db 100644 --- a/src/test/mir-opt/inline-closure-borrows-arg.rs +++ b/src/test/mir-opt/inline-closure-borrows-arg.rs @@ -38,11 +38,9 @@ fn foo(_t: T, q: &i32) -> i32 { // ... // _7 = &(*_2); // _5 = (move _6, move _7); -// _9 = move (_5.0: &i32); -// _10 = move (_5.1: &i32); -// StorageLive(_8); -// _8 = (*_9); -// _0 = move _8; +// _8 = move (_5.0: &i32); +// _9 = move (_5.1: &i32); +// _0 = (*_8); // ... // return; // } diff --git a/src/test/mir-opt/inline-closure.rs b/src/test/mir-opt/inline-closure.rs index dc8ff13c03a88..22e7de31e90cf 100644 --- a/src/test/mir-opt/inline-closure.rs +++ b/src/test/mir-opt/inline-closure.rs @@ -36,7 +36,7 @@ fn foo(_t: T, q: i32) -> i32 { // _5 = (move _6, move _7); // _8 = move (_5.0: i32); // _9 = move (_5.1: i32); -// _0 = move _8; +// _0 = _8; // ... // return; // }