Skip to content

Commit

Permalink
avoid computing liveness when a variable doesn't need it
Browse files Browse the repository at this point in the history
In particular, we skip computing liveness for a variable X if all the
regions in its type are known to outlive free regions.
  • Loading branch information
nikomatsakis committed Aug 7, 2018
1 parent 887296e commit 3b7989d
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 59 deletions.
2 changes: 2 additions & 0 deletions src/librustc_mir/borrow_check/nll/constraints/graph.rs
Expand Up @@ -24,6 +24,8 @@ crate struct ConstraintGraph<D: ConstraintGraphDirecton> {

crate type NormalConstraintGraph = ConstraintGraph<Normal>;

crate type ReverseConstraintGraph = ConstraintGraph<Reverse>;

/// Marker trait that controls whether a `R1: R2` constraint
/// represents an edge `R1 -> R2` or `R2 -> R1`.
crate trait ConstraintGraphDirecton: Copy + 'static {
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/borrow_check/nll/constraints/mod.rs
Expand Up @@ -46,6 +46,12 @@ impl ConstraintSet {
graph::ConstraintGraph::new(graph::Normal, self, num_region_vars)
}

/// Like `graph`, but constraints a reverse graph where `R1: R2`
/// represents an edge `R2 -> R1`.
crate fn reverse_graph(&self, num_region_vars: usize) -> graph::ReverseConstraintGraph {
graph::ConstraintGraph::new(graph::Reverse, self, num_region_vars)
}

/// Compute cycles (SCCs) in the graph of regions. In particular,
/// find all regions R1, R2 such that R1: R2 and R2: R1 and group
/// them into an SCC, and find the relationships between SCCs.
Expand Down
Expand Up @@ -16,21 +16,25 @@
//! liveness code so that it only operates over variables with regions in their
//! types, instead of all variables.

use borrow_check::nll::ToRegionVid;
use rustc::mir::{Local, Mir};
use rustc::ty::TypeFoldable;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc::ty::{RegionVid, TyCtxt};
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use util::liveness::LiveVariableMap;

use rustc_data_structures::indexed_vec::Idx;

/// Map between Local and LocalWithRegion indices: this map is supplied to the
/// liveness code so that it will only analyze those variables whose types
/// contain regions.
/// Map between Local and LocalWithRegion indices: the purpose of this
/// map is to define the subset of local variables for which we need
/// to do a liveness computation. We only need to compute whether a
/// variable `X` is live if that variable contains some region `R` in
/// its type where `R` is not known to outlive a free region (i.e.,
/// where `R` may be valid for just a subset of the fn body).
crate struct NllLivenessMap {
/// For each local variable, contains either None (if the type has no regions)
/// or Some(i) with a suitable index.
/// For each local variable, contains `Some(i)` if liveness is
/// needed for this variable.
pub from_local: IndexVec<Local, Option<LocalWithRegion>>,
/// For each LocalWithRegion, maps back to the original Local index.

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

Expand All @@ -51,21 +55,32 @@ 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(
tcx: TyCtxt<'_, '_, 'tcx>,
free_regions: &FxHashSet<RegionVid>,
mir: &Mir<'tcx>,
) -> Self {
let mut to_local = IndexVec::default();
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))
} else {
if tcx.all_free_regions_meet(&local_decl.ty, |r| {
free_regions.contains(&r.to_region_vid())
}) {
// If all the regions in the type are free regions
// (or there are no regions), then we don't need
// to track liveness for this variable.
None
} else {
Some(to_local.push(local))
}
})
.collect();

debug!("{} total variables", mir.local_decls.len());
debug!("{} variables need liveness", to_local.len());
debug!("{} regions outlive free regions", free_regions.len());

Self {
from_local,
to_local,
Expand Down
69 changes: 62 additions & 7 deletions src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs
Expand Up @@ -8,8 +8,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::nll::{NllLivenessMap, LocalWithRegion};
use borrow_check::nll::constraints::ConstraintSet;
use borrow_check::nll::type_check::AtLocation;
use borrow_check::nll::{LocalWithRegion, NllLivenessMap};
use borrow_check::nll::universal_regions::UniversalRegions;
use dataflow::move_paths::{HasMoveData, MoveData};
use dataflow::MaybeInitializedPlaces;
use dataflow::{FlowAtLocation, FlowsAtLocation};
Expand All @@ -18,10 +20,10 @@ use rustc::mir::{BasicBlock, Location, Mir};
use rustc::traits::query::dropck_outlives::DropckOutlivesResult;
use rustc::traits::query::type_op::outlives::DropckOutlives;
use rustc::traits::query::type_op::TypeOp;
use rustc::ty::{Ty, TypeFoldable};
use rustc_data_structures::fx::FxHashMap;
use rustc::ty::{RegionVid, Ty, TypeFoldable};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use std::rc::Rc;
use util::liveness::{LivenessResults, LiveVariableMap };
use util::liveness::{LiveVariableMap, LivenessResults};

use super::TypeChecker;

Expand All @@ -41,9 +43,18 @@ pub(super) fn generate<'gcx, 'tcx>(
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
) -> (LivenessResults<LocalWithRegion>, NllLivenessMap) {
let liveness_map = NllLivenessMap::compute(&mir);
let free_regions = {
let borrowck_context = cx.borrowck_context.as_ref().unwrap();
regions_that_outlive_free_regions(
cx.infcx.num_region_vars(),
&borrowck_context.universal_regions,
&borrowck_context.constraints.outlives_constraints,
)
};
let liveness_map = NllLivenessMap::compute(cx.tcx(), &free_regions, mir);
let liveness = LivenessResults::compute(mir, &liveness_map);

// For everything else, it is only live where it is actually used.
{
let mut generator = TypeLivenessGenerator {
cx,
Expand All @@ -63,6 +74,45 @@ pub(super) fn generate<'gcx, 'tcx>(
(liveness, liveness_map)
}

/// Compute all regions that are (currently) known to outlive free
/// regions. For these regions, we do not need to compute
/// liveness, since the outlives constraints will ensure that they
/// are live over the whole fn body anyhow.
fn regions_that_outlive_free_regions(
num_region_vars: usize,
universal_regions: &UniversalRegions<'tcx>,
constraint_set: &ConstraintSet,
) -> FxHashSet<RegionVid> {
// Build a graph of the outlives constraints thus far. This is
// a reverse graph, so for each constraint `R1: R2` we have an
// edge `R2 -> R1`. Therefore, if we find all regions
// reachable from each free region, we will have all the
// regions that are forced to outlive some free region.
let rev_constraint_graph = constraint_set.reverse_graph(num_region_vars);
let rev_region_graph = rev_constraint_graph.region_graph(constraint_set);

// Stack for the depth-first search. Start out with all the free regions.
let mut stack: Vec<_> = universal_regions.universal_regions().collect();

// Set of all free regions, plus anything that outlives them. Initially
// just contains the free regions.
let mut outlives_free_region: FxHashSet<_> = stack.iter().cloned().collect();

// Do the DFS -- for each thing in the stack, find all things
// that outlive it and add them to the set. If they are not,
// push them onto the stack for later.
while let Some(sub_region) = stack.pop() {
stack.extend(
rev_region_graph
.outgoing_regions(sub_region)
.filter(|&r| outlives_free_region.insert(r)),
);
}

// Return the final set of things we visited.
outlives_free_region
}

struct TypeLivenessGenerator<'gen, 'typeck, 'flow, 'gcx, 'tcx>
where
'typeck: 'gen,
Expand Down Expand Up @@ -182,8 +232,13 @@ impl<'gen, 'typeck, 'flow, 'gcx, 'tcx> TypeLivenessGenerator<'gen, 'typeck, 'flo

cx.tcx().for_each_free_region(&value, |live_region| {
if let Some(ref mut borrowck_context) = cx.borrowck_context {
let region_vid = borrowck_context.universal_regions.to_region_vid(live_region);
borrowck_context.constraints.liveness_constraints.add_element(region_vid, location);
let region_vid = borrowck_context
.universal_regions
.to_region_vid(live_region);
borrowck_context
.constraints
.liveness_constraints
.add_element(region_vid, location);

if let Some(all_facts) = borrowck_context.all_facts {
let start_index = borrowck_context.location_table.start_index(location);
Expand Down
34 changes: 14 additions & 20 deletions src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.nll.stderr
@@ -1,30 +1,24 @@
error[E0597]: borrowed value does not live long enough
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:15:21
|
LL | fn gimme_static_mut_let() -> &'static mut u32 {
| _______________________________________________-
LL | | let ref mut x = 1234543; //~ ERROR
| | ^^^^^^^ temporary value does not live long enough
LL | | x
LL | | }
| | -
| | |
| |_temporary value only lives until here
| borrow later used here
LL | let ref mut x = 1234543; //~ ERROR
| ^^^^^^^ temporary value does not live long enough
LL | x
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

error[E0597]: borrowed value does not live long enough
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:20:25
|
LL | fn gimme_static_mut_let_nested() -> &'static mut u32 {
| ______________________________________________________-
LL | | let (ref mut x, ) = (1234543, ); //~ ERROR
| | ^^^^^^^^^^^ temporary value does not live long enough
LL | | x
LL | | }
| | -
| | |
| |_temporary value only lives until here
| borrow later used here
LL | let (ref mut x, ) = (1234543, ); //~ ERROR
| ^^^^^^^^^^^ temporary value does not live long enough
LL | x
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

error[E0597]: borrowed value does not live long enough
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:25:11
Expand Down
15 changes: 12 additions & 3 deletions src/test/ui/nll/get_default.nll.stderr
Expand Up @@ -63,9 +63,18 @@ LL | match map.get() {
LL | Some(v) => {
LL | map.set(String::new()); // Both AST and MIR error here
| ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
...
LL | return v;
| - borrow later used here
|
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 41:1...
--> $DIR/get_default.rs:41:1
|
LL | / fn err(map: &mut Map) -> &String {
LL | | loop {
LL | | match map.get() {
LL | | Some(v) => {
... |
LL | | }
LL | | }
| |_^

error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir)
--> $DIR/get_default.rs:51:17
Expand Down
15 changes: 12 additions & 3 deletions src/test/ui/nll/get_default.stderr
Expand Up @@ -63,9 +63,18 @@ LL | match map.get() {
LL | Some(v) => {
LL | map.set(String::new()); // Both AST and MIR error here
| ^^^ mutable borrow occurs here
...
LL | return v;
| - borrow later used here
|
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 41:1...
--> $DIR/get_default.rs:41:1
|
LL | / fn err(map: &mut Map) -> &String {
LL | | loop {
LL | | match map.get() {
LL | | Some(v) => {
... |
LL | | }
LL | | }
| |_^

error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir)
--> $DIR/get_default.rs:51:17
Expand Down
17 changes: 7 additions & 10 deletions src/test/ui/nll/return-ref-mut-issue-46557.stderr
@@ -1,16 +1,13 @@
error[E0597]: borrowed value does not live long enough
--> $DIR/return-ref-mut-issue-46557.rs:17:21
|
LL | fn gimme_static_mut() -> &'static mut u32 {
| ___________________________________________-
LL | | let ref mut x = 1234543; //~ ERROR borrowed value does not live long enough [E0597]
| | ^^^^^^^ temporary value does not live long enough
LL | | x
LL | | }
| | -
| | |
| |_temporary value only lives until here
| borrow later used here
LL | let ref mut x = 1234543; //~ ERROR borrowed value does not live long enough [E0597]
| ^^^^^^^ temporary value does not live long enough
LL | x
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

error: aborting due to previous error

Expand Down

0 comments on commit 3b7989d

Please sign in to comment.