From d8ed2e7ed4118a2e590489514ad21b21afc9d1b0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 28 Jun 2019 18:41:53 -0700 Subject: [PATCH] Use RequiresStorage to determine which locals can overlap --- .../dataflow/impls/storage_liveness.rs | 4 --- src/librustc_mir/transform/generator.rs | 27 ++++++++++--------- .../run-pass/generator/size-moved-locals.rs | 24 +++++++++++++---- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index 0a3ae252be0..ad8b3f5fdbe 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -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: diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index c5f0f36b8b9..e16477cf2fd 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -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; @@ -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, @@ -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 { assert_eq!(body.local_decls.len(), ignored.0.domain_size()); assert_eq!(body.local_decls.len(), stored_locals.domain_size()); @@ -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; @@ -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, + _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 @@ -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 { diff --git a/src/test/run-pass/generator/size-moved-locals.rs b/src/test/run-pass/generator/size-moved-locals.rs index 9aa7ad02f3e..37e2e0cfdcc 100644 --- a/src/test/run-pass/generator/size-moved-locals.rs +++ b/src/test/run-pass/generator/size-moved-locals.rs @@ -21,7 +21,7 @@ impl Drop for Foo { fn drop(&mut self) {} } -fn simple() -> impl Generator { +fn move_before_yield() -> impl Generator { static || { let first = Foo([0; FOO_SIZE]); let _second = first; @@ -32,7 +32,7 @@ fn simple() -> impl Generator { fn noop() {} -fn complex() -> impl Generator { +fn move_before_yield_with_noop() -> impl Generator { static || { let first = Foo([0; FOO_SIZE]); noop(); @@ -42,7 +42,21 @@ fn complex() -> impl Generator { } } -fn main() { - assert_eq!(1028, std::mem::size_of_val(&simple())); - assert_eq!(1032, std::mem::size_of_val(&complex())); +// 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 { + 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(&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())); }