Skip to content

Commit

Permalink
Auto merge of rust-lang#52991 - nikomatsakis:nll-escaping-into-return…
Browse files Browse the repository at this point in the history
…, r=pnkfelix

avoid computing liveness for locals that escape into statics

Fixes rust-lang#52713

I poked at this on the plane and I think it's working -- but I want to do a bit more investigation and double check. The idea is to identify those local variables where the entire value will "escape" into the return -- for them, we don't need to compute liveness, since we know that the outlives relations from the return type will force those regions to be equal to free regions. This should help with html5ever in particular.

- [x] test performance
- [x] verify correctness
- [x] add comments

r? @pnkfelix
cc @lqd
  • Loading branch information
bors committed Aug 5, 2018
2 parents ddcf17e + 2e2ea26 commit b47c314
Show file tree
Hide file tree
Showing 9 changed files with 331 additions and 65 deletions.
229 changes: 229 additions & 0 deletions src/librustc_mir/borrow_check/nll/escaping_locals.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! Identify those variables whose entire value will eventually be
//! returned from the fn via the RETURN_PLACE. As an optimization, we
//! can skip computing liveness results for those variables. The idea
//! is that the return type of the fn only ever contains free
//! regions. Therefore, the types of those variables are going to
//! ultimately be contrained to outlive those free regions -- since
//! free regions are always live for the entire body, this implies
//! that the liveness results are not important for those regions.
//! This is most important in the "fns" that we create to represent static
//! values, since those are often really quite large, and all regions in them
//! will ultimately be constrained to be `'static`. Two examples:
//!
//! ```
//! fn foo() -> &'static [u32] { &[] }
//! static FOO: &[u32] = &[];
//! ```
//!
//! In both these cases, the return value will only have static lifetime.
//!
//! NB: The simple logic here relies on the fact that outlives
//! relations in our analysis don't have locations. Otherwise, we
//! would have to restrict ourselves to values that are
//! *unconditionally* returned (which would still cover the "big
//! static value" case).
//!
//! The way that this code works is to use union-find -- we iterate
//! over the MIR and union together two variables X and Y if all
//! regions in the value of Y are going to be stored into X -- that
//! is, if `typeof(X): 'a` requires that `typeof(Y): 'a`. This means
//! that e.g. we can union together `x` and `y` if we have something
//! like `x = (y, 22)`, but not something like `x = y.f` (since there
//! may be regions in the type of `y` that do not appear in the field
//! `f`).
use rustc::mir::visit::Visitor;
use rustc::mir::*;

use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::unify as ut;

crate struct EscapingLocals {
unification_table: ut::UnificationTable<ut::InPlace<AssignedLocal>>,
}

impl EscapingLocals {
crate fn compute(mir: &Mir<'tcx>) -> Self {
let mut visitor = GatherAssignedLocalsVisitor::new();
visitor.visit_mir(mir);

EscapingLocals {
unification_table: visitor.unification_table,
}
}

/// True if `local` is known to escape into static
/// memory.
crate fn escapes_into_return(&mut self, local: Local) -> bool {
let return_place = AssignedLocal::from(RETURN_PLACE);
let other_place = AssignedLocal::from(local);
self.unification_table.unioned(return_place, other_place)
}
}

/// The MIR visitor gathering the union-find of the locals used in
/// assignments.
struct GatherAssignedLocalsVisitor {
unification_table: ut::UnificationTable<ut::InPlace<AssignedLocal>>,
}

#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
struct AssignedLocal(u32);

impl ut::UnifyKey for AssignedLocal {
type Value = ();

fn index(&self) -> u32 {
self.0
}

fn from_index(i: u32) -> AssignedLocal {
AssignedLocal(i)
}

fn tag() -> &'static str {
"AssignedLocal"
}
}

impl From<Local> for AssignedLocal {
fn from(item: Local) -> Self {
// newtype_indexes use usize but are u32s.
assert!(item.index() < ::std::u32::MAX as usize);
AssignedLocal(item.index() as u32)
}
}

impl GatherAssignedLocalsVisitor {
fn new() -> Self {
Self {
unification_table: ut::UnificationTable::new(),
}
}

fn union_locals_if_needed(&mut self, lvalue: Option<Local>, rvalue: Option<Local>) {
if let Some(lvalue) = lvalue {
if let Some(rvalue) = rvalue {
if lvalue != rvalue {
debug!("EscapingLocals: union {:?} and {:?}", lvalue, rvalue);
self.unification_table
.union(AssignedLocal::from(lvalue), AssignedLocal::from(rvalue));
}
}
}
}
}

// Returns the potential `Local` associated to this `Place` or `PlaceProjection`
fn find_local_in_place(place: &Place) -> Option<Local> {
match place {
Place::Local(local) => Some(*local),

// If you do e.g. `x = a.f` then only *part* of the type of
// `a` escapes into `x` (the part contained in `f`); if `a`'s
// type has regions that don't appear in `f`, those might not
// escape.
Place::Projection(..) => None,

Place::Static { .. } | Place::Promoted { .. } => None,
}
}

// Returns the potential `Local` in this `Operand`.
fn find_local_in_operand(op: &Operand) -> Option<Local> {
// Conservatively check a subset of `Operand`s we know our
// benchmarks track, for example `html5ever`.
match op {
Operand::Copy(place) | Operand::Move(place) => find_local_in_place(place),
Operand::Constant(_) => None,
}
}

impl Visitor<'tcx> for GatherAssignedLocalsVisitor {
fn visit_mir(&mut self, mir: &Mir<'tcx>) {
// We need as many union-find keys as there are locals
for _ in 0..mir.local_decls.len() {
self.unification_table.new_key(());
}

self.super_mir(mir);
}

fn visit_assign(
&mut self,
block: BasicBlock,
place: &Place<'tcx>,
rvalue: &Rvalue<'tcx>,
location: Location,
) {
let local = find_local_in_place(place);

// Find those cases where there is a `Place` consumed by
// `rvalue` and we know that all regions in its type will be
// incorporated into `place`, the `Place` we are assigning to.
match rvalue {
// `x = y` is the simplest possible case.
Rvalue::Use(op) => self.union_locals_if_needed(local, find_local_in_operand(op)),

// `X = &'r P` -- the type of `X` will be `&'r T_P`, where
// `T_P` is the type of `P`.
Rvalue::Ref(_, _, place) => {
// Special case: if you have `X = &*Y` (or `X = &**Y`
// etc), then the outlives relationships will ensure
// that all regions in `Y` are constrained by regions
// in `X` -- this is because the lifetimes of the
// references we deref through are required to outlive
// the borrow lifetime `'r` (which appears in `X`).
//
// (We don't actually need to check the type of `Y`:
// since `ProjectionElem::Deref` represents a built-in
// deref and not an overloaded deref, if the thing we
// deref through is not a reference, then it must be a
// `Box` or `*const`, in which case it contains no
// references.)
let mut place_ref = place;
while let Place::Projection(proj) = place_ref {
if let ProjectionElem::Deref = proj.elem {
place_ref = &proj.base;
} else {
break;
}
}

self.union_locals_if_needed(local, find_local_in_place(place_ref))
}

Rvalue::Cast(kind, op, _) => match kind {
CastKind::Unsize => {
// Casting a `&[T; N]` to `&[T]` or `&Foo` to `&Trait` --
// in both cases, no regions are "lost".
self.union_locals_if_needed(local, find_local_in_operand(op))
}
_ => (),
},

// Constructing an aggregate like `(x,)` or `Foo { x }`
// includes the full type of `x`.
Rvalue::Aggregate(_, ops) => {
for rvalue in ops.iter().map(find_local_in_operand) {
self.union_locals_if_needed(local, rvalue);
}
}

// For other things, be conservative and do not union.
_ => (),
};

self.super_assign(block, place, rvalue, location);
}
}
48 changes: 35 additions & 13 deletions src/librustc_mir/borrow_check/nll/liveness_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
//! liveness code so that it only operates over variables with regions in their
//! types, instead of all variables.
use borrow_check::nll::escaping_locals::EscapingLocals;
use rustc::mir::{Local, Mir};
use rustc::ty::TypeFoldable;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc::mir::{Mir, Local};
use util::liveness::LiveVariableMap;

use rustc_data_structures::indexed_vec::Idx;
Expand All @@ -29,14 +30,13 @@ use rustc_data_structures::indexed_vec::Idx;
crate struct NllLivenessMap {
/// For each local variable, contains either None (if the type has no regions)
/// or Some(i) with a suitable index.
pub from_local: IndexVec<Local, Option<LocalWithRegion>>,
/// For each LocalWithRegion, maps back to the original Local index.
pub to_local: IndexVec<LocalWithRegion, Local>,
from_local: IndexVec<Local, Option<LocalWithRegion>>,

/// For each LocalWithRegion, maps back to the original Local index.
to_local: IndexVec<LocalWithRegion, Local>,
}

impl LiveVariableMap for NllLivenessMap {

fn from_local(&self, local: Local) -> Option<Self::LiveVar> {
self.from_local[local]
}
Expand All @@ -55,21 +55,43 @@ impl LiveVariableMap for NllLivenessMap {
impl NllLivenessMap {
/// Iterates over the variables in Mir and assigns each Local whose type contains
/// regions a LocalWithRegion index. Returns a map for converting back and forth.
pub fn compute(mir: &Mir) -> Self {
crate fn compute(mir: &Mir<'tcx>) -> Self {
let mut escaping_locals = EscapingLocals::compute(mir);

let mut to_local = IndexVec::default();
let from_local: IndexVec<Local,Option<_>> = mir
let mut escapes_into_return = 0;
let mut no_regions = 0;
let from_local: IndexVec<Local, Option<_>> = mir
.local_decls
.iter_enumerated()
.map(|(local, local_decl)| {
if local_decl.ty.has_free_regions() {
Some(to_local.push(local))
if escaping_locals.escapes_into_return(local) {
// If the local escapes into the return value,
// then the return value will force all of the
// regions in its type to outlive free regions
// (e.g., `'static`) and hence liveness is not
// needed. This is particularly important for big
// statics.
escapes_into_return += 1;
None
} else if local_decl.ty.has_free_regions() {
let l = to_local.push(local);
debug!("liveness_map: {:?} = {:?}", local, l);
Some(l)
} else {
no_regions += 1;
None
}
else {
None
}
}).collect();

Self { from_local, to_local }
debug!("liveness_map: {} variables need liveness", to_local.len());
debug!("liveness_map: {} escapes into return", escapes_into_return);
debug!("liveness_map: {} no regions", no_regions);

Self {
from_local,
to_local,
}
}
}

Expand Down
13 changes: 7 additions & 6 deletions src/librustc_mir/borrow_check/nll/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ use polonius_engine::{Algorithm, Output};
use util as mir_util;
use util::pretty::{self, ALIGN};

mod constraints;
mod constraint_generation;
mod escaping_locals;
pub mod explain_borrow;
mod facts;
mod invalidation;
Expand All @@ -49,8 +51,6 @@ crate mod type_check;
mod universal_regions;
crate mod liveness_map;

mod constraints;

use self::facts::AllFacts;
use self::region_infer::RegionInferenceContext;
use self::universal_regions::UniversalRegions;
Expand Down Expand Up @@ -120,6 +120,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
location_table,
borrow_set,
&liveness,
&liveness_map,
&mut all_facts,
flow_inits,
move_data,
Expand Down Expand Up @@ -205,6 +206,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
dump_mir_results(
infcx,
&liveness,
&liveness_map,
MirSource::item(def_id),
&mir,
&regioncx,
Expand All @@ -221,6 +223,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
fn dump_mir_results<'a, 'gcx, 'tcx>(
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
liveness: &LivenessResults<LocalWithRegion>,
liveness_map: &NllLivenessMap,
source: MirSource,
mir: &Mir<'tcx>,
regioncx: &RegionInferenceContext,
Expand All @@ -230,16 +233,14 @@ fn dump_mir_results<'a, 'gcx, 'tcx>(
return;
}

let map = &NllLivenessMap::compute(mir);

let regular_liveness_per_location: FxHashMap<_, _> = mir
.basic_blocks()
.indices()
.flat_map(|bb| {
let mut results = vec![];
liveness
.regular
.simulate_block(&mir, bb, map, |location, local_set| {
.simulate_block(&mir, bb, liveness_map, |location, local_set| {
results.push((location, local_set.clone()));
});
results
Expand All @@ -253,7 +254,7 @@ fn dump_mir_results<'a, 'gcx, 'tcx>(
let mut results = vec![];
liveness
.drop
.simulate_block(&mir, bb, map, |location, local_set| {
.simulate_block(&mir, bb, liveness_map, |location, local_set| {
results.push((location, local_set.clone()));
});
results
Expand Down
Loading

0 comments on commit b47c314

Please sign in to comment.