Skip to content

Commit

Permalink
Use RequiresStorage to determine which locals can overlap
Browse files Browse the repository at this point in the history
  • Loading branch information
tmandry committed Jun 29, 2019
1 parent aee1357 commit d8ed2e7
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 20 deletions.
4 changes: 0 additions & 4 deletions src/librustc_mir/dataflow/impls/storage_liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ impl<'a, 'tcx> BottomValue for MaybeStorageLive<'a, 'tcx> {

/// Dataflow analysis that determines whether each local requires storage at a
/// given location; i.e. whether its storage can go away without being observed.
///
/// In the case of a movable generator, borrowed_locals can be `None` and we
/// will not consider borrows in this pass. This relies on the fact that we only
/// use this pass at yield points for these generators.
pub struct RequiresStorage<'mir, 'tcx, 'b> {
body: &'mir Body<'tcx>,
borrowed_locals:
Expand Down
27 changes: 15 additions & 12 deletions src/librustc_mir/transform/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc_data_structures::bit_set::{BitSet, BitMatrix};
use std::borrow::Cow;
use std::iter;
use std::marker::PhantomData;
use std::mem;
use crate::transform::{MirPass, MirSource};
use crate::transform::simplify;
Expand Down Expand Up @@ -539,8 +540,8 @@ fn locals_live_across_suspend_points(
body,
&live_locals,
&ignored,
storage_live,
storage_live_analysis);
requires_storage,
requires_storage_analysis);

LivenessInfo {
live_locals,
Expand Down Expand Up @@ -577,8 +578,8 @@ fn compute_storage_conflicts(
body: &'mir Body<'tcx>,
stored_locals: &liveness::LiveVarSet,
ignored: &StorageIgnored,
storage_live: DataflowResults<'tcx, MaybeStorageLive<'mir, 'tcx>>,
_storage_live_analysis: MaybeStorageLive<'mir, 'tcx>,
requires_storage: DataflowResults<'tcx, RequiresStorage<'mir, 'tcx, '_>>,
_requires_storage_analysis: RequiresStorage<'mir, 'tcx, '_>,
) -> BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal> {
assert_eq!(body.local_decls.len(), ignored.0.domain_size());
assert_eq!(body.local_decls.len(), stored_locals.domain_size());
Expand All @@ -594,9 +595,10 @@ fn compute_storage_conflicts(
let mut visitor = StorageConflictVisitor {
body,
stored_locals: &stored_locals,
local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len())
local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len()),
_phantom: PhantomData::default(),
};
let mut state = FlowAtLocation::new(storage_live);
let mut state = FlowAtLocation::new(requires_storage);
visitor.analyze_results(&mut state);
let local_conflicts = visitor.local_conflicts;

Expand Down Expand Up @@ -626,18 +628,19 @@ fn compute_storage_conflicts(
storage_conflicts
}

struct StorageConflictVisitor<'body, 'tcx, 's> {
struct StorageConflictVisitor<'body: 'b, 'tcx, 's, 'b> {
body: &'body Body<'tcx>,
stored_locals: &'s liveness::LiveVarSet,
// FIXME(tmandry): Consider using sparse bitsets here once we have good
// benchmarks for generators.
local_conflicts: BitMatrix<Local, Local>,
_phantom: PhantomData<&'b ()>,
}

impl<'body, 'tcx, 's> DataflowResultsConsumer<'body, 'tcx>
for StorageConflictVisitor<'body, 'tcx, 's>
impl<'body, 'tcx, 's, 'b> DataflowResultsConsumer<'body, 'tcx>
for StorageConflictVisitor<'body, 'tcx, 's, 'b>
{
type FlowState = FlowAtLocation<'tcx, MaybeStorageLive<'body, 'tcx>>;
type FlowState = FlowAtLocation<'tcx, RequiresStorage<'body, 'tcx, 'b>>;

fn body(&self) -> &'body Body<'tcx> {
self.body
Expand Down Expand Up @@ -665,9 +668,9 @@ impl<'body, 'tcx, 's> DataflowResultsConsumer<'body, 'tcx>
}
}

impl<'body, 'tcx, 's> StorageConflictVisitor<'body, 'tcx, 's> {
impl<'body, 'tcx, 's, 'b> StorageConflictVisitor<'body, 'tcx, 's, 'b> {
fn apply_state(&mut self,
flow_state: &FlowAtLocation<'tcx, MaybeStorageLive<'body, 'tcx>>,
flow_state: &FlowAtLocation<'tcx, RequiresStorage<'body, 'tcx, 'b>>,
loc: Location) {
// Ignore unreachable blocks.
match self.body.basic_blocks()[loc.block].terminator().kind {
Expand Down
22 changes: 18 additions & 4 deletions src/test/run-pass/generator/size-moved-locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl Drop for Foo {
fn drop(&mut self) {}
}

fn simple() -> impl Generator<Yield = (), Return = ()> {
fn move_before_yield() -> impl Generator<Yield = (), Return = ()> {
static || {
let first = Foo([0; FOO_SIZE]);
let _second = first;
Expand All @@ -32,7 +32,7 @@ fn simple() -> impl Generator<Yield = (), Return = ()> {

fn noop() {}

fn complex() -> impl Generator<Yield = (), Return = ()> {
fn move_before_yield_with_noop() -> impl Generator<Yield = (), Return = ()> {
static || {
let first = Foo([0; FOO_SIZE]);
noop();
Expand All @@ -42,7 +42,21 @@ fn complex() -> impl Generator<Yield = (), Return = ()> {
}
}

// Today we don't have NRVO (we allocate space for both `first` and `second`,)
// but we can overlap `first` with `_third`.
fn overlap_move_points() -> impl Generator<Yield = (), Return = ()> {
static || {
let first = Foo([0; FOO_SIZE]);
yield;
let second = first;
yield;
let _third = second;
yield;
}
}

fn main() {
assert_eq!(1028, std::mem::size_of_val(&simple()));
assert_eq!(1032, std::mem::size_of_val(&complex()));
assert_eq!(1028, std::mem::size_of_val(&move_before_yield()));
assert_eq!(1032, std::mem::size_of_val(&move_before_yield_with_noop()));
assert_eq!(2056, std::mem::size_of_val(&overlap_move_points()));
}

0 comments on commit d8ed2e7

Please sign in to comment.