From 68b9f0be83e72c8502640d41fba75c6eec13d0b1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Dec 2017 10:17:21 -0500 Subject: [PATCH 1/8] use `'_` to make clear that `BlockSets` contain references --- src/librustc_mir/dataflow/impls/mod.rs | 28 +++++++++---------- .../dataflow/impls/storage_liveness.rs | 2 +- src/librustc_mir/dataflow/mod.rs | 4 +-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 106a88e703c79..fe063b2a4b69b 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -292,7 +292,7 @@ impl<'a, 'gcx, 'tcx> HasMoveData<'tcx> for EverInitializedLvals<'a, 'gcx, 'tcx> impl<'a, 'gcx, 'tcx> MaybeInitializedLvals<'a, 'gcx, 'tcx> { - fn update_bits(sets: &mut BlockSets, path: MovePathIndex, + fn update_bits(sets: &mut BlockSets<'_, MovePathIndex>, path: MovePathIndex, state: DropFlagState) { match state { @@ -303,7 +303,7 @@ impl<'a, 'gcx, 'tcx> MaybeInitializedLvals<'a, 'gcx, 'tcx> { } impl<'a, 'gcx, 'tcx> MaybeUninitializedLvals<'a, 'gcx, 'tcx> { - fn update_bits(sets: &mut BlockSets, path: MovePathIndex, + fn update_bits(sets: &mut BlockSets<'_, MovePathIndex>, path: MovePathIndex, state: DropFlagState) { match state { @@ -314,7 +314,7 @@ impl<'a, 'gcx, 'tcx> MaybeUninitializedLvals<'a, 'gcx, 'tcx> { } impl<'a, 'gcx, 'tcx> DefinitelyInitializedLvals<'a, 'gcx, 'tcx> { - fn update_bits(sets: &mut BlockSets, path: MovePathIndex, + fn update_bits(sets: &mut BlockSets<'_, MovePathIndex>, path: MovePathIndex, state: DropFlagState) { match state { @@ -341,7 +341,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedLvals<'a, 'gcx, 'tcx> { } fn statement_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MovePathIndex>, location: Location) { drop_flag_effects_for_location( @@ -352,7 +352,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedLvals<'a, 'gcx, 'tcx> { } fn terminator_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MovePathIndex>, location: Location) { drop_flag_effects_for_location( @@ -396,7 +396,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedLvals<'a, 'gcx, 'tcx> { } fn statement_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MovePathIndex>, location: Location) { drop_flag_effects_for_location( @@ -407,7 +407,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedLvals<'a, 'gcx, 'tcx> { } fn terminator_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MovePathIndex>, location: Location) { drop_flag_effects_for_location( @@ -450,7 +450,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'gcx, 'tcx } fn statement_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MovePathIndex>, location: Location) { drop_flag_effects_for_location( @@ -461,7 +461,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'gcx, 'tcx } fn terminator_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MovePathIndex>, location: Location) { drop_flag_effects_for_location( @@ -497,7 +497,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { } fn statement_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MoveOutIndex>, location: Location) { let (tcx, mir, move_data) = (self.tcx, self.mir, self.move_data()); let stmt = &mir[location.block].statements[location.statement_index]; @@ -525,7 +525,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { } fn terminator_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, MoveOutIndex>, location: Location) { let (tcx, mir, move_data) = (self.tcx, self.mir, self.move_data()); @@ -542,7 +542,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { } fn propagate_call_return(&self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, MoveOutIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, dest_place: &mir::Place) { @@ -575,7 +575,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { } fn statement_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, InitIndex>, location: Location) { let (_, mir, move_data) = (self.tcx, self.mir, self.move_data()); let stmt = &mir[location.block].statements[location.statement_index]; @@ -622,7 +622,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { } fn terminator_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, InitIndex>, location: Location) { let (mir, move_data) = (self.mir, self.move_data()); diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index dea61542ac4e2..0a6e5c7f3e97e 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -59,7 +59,7 @@ impl<'a, 'tcx> BitDenotation for MaybeStorageLive<'a, 'tcx> { } fn propagate_call_return(&self, - _in_out: &mut IdxSet, + _sets: &mut BlockSets<'_, Local>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, _dest_place: &mir::Place) { diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index b18fb7c7b9cce..b2bc41286648d 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -674,7 +674,7 @@ pub trait BitDenotation: BitwiseOperator { /// `bb_data` is the sequence of statements identified by `bb` in /// the MIR. fn statement_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, Self::Idx>, location: Location); /// Similar to `terminator_effect`, except it applies @@ -703,7 +703,7 @@ pub trait BitDenotation: BitwiseOperator { /// The effects applied here cannot depend on which branch the /// terminator took. fn terminator_effect(&self, - sets: &mut BlockSets, + sets: &mut BlockSets<'_, Self::Idx>, location: Location); /// Mutates the block-sets according to the (flow-dependent) From 3f768fdd23f148c43c2bc875a2aae9e40fac6ec5 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Dec 2017 10:17:43 -0500 Subject: [PATCH 2/8] document that `sets.on_entry` is not *always* block entry --- src/librustc_mir/dataflow/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index b2bc41286648d..5108b1a6d057a 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -612,11 +612,10 @@ pub trait BitDenotation: BitwiseOperator { /// `sets.on_entry` to that local clone into `statement_effect` and /// `terminator_effect`). /// - /// When its false, no local clone is constucted; instead a - /// reference directly into `on_entry` is passed along via - /// `sets.on_entry` instead, which represents the flow state at - /// the block's start, not necessarily the state immediately prior - /// to the statement/terminator under analysis. + /// When its false, no local clone is constucted; instead it is + /// undefined what `on_entry` points to (in practice, it will + /// frequently be a reference the flow state at the block's start, + /// but you should not rely on that). /// /// In either case, the passed reference is mutable; but this is a /// wart from using the `BlockSets` type in the API; the intention From af62153719445ac534ca52a9c312472ae86adf81 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Dec 2017 10:18:04 -0500 Subject: [PATCH 3/8] convert `propagate_call_return` to take `sets` and be pure function It used to take an `in_out` but that made it hard to determine whether the call had any effect or not. --- src/librustc_mir/dataflow/impls/borrows.rs | 4 +- src/librustc_mir/dataflow/impls/mod.rs | 58 ++++++++++++++-------- src/librustc_mir/dataflow/mod.rs | 46 +++++++++++------ 3 files changed, 69 insertions(+), 39 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index f543a33b130b6..adfde9261c301 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -678,7 +678,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Reservations<'a, 'gcx, 'tcx> { } fn propagate_call_return(&self, - _in_out: &mut IdxSet, + _sets: &mut BlockSets<'_, ReserveOrActivateIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, _dest_place: &mir::Place) { @@ -739,7 +739,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for ActiveBorrows<'a, 'gcx, 'tcx> { } fn propagate_call_return(&self, - _in_out: &mut IdxSet, + _sets: &mut BlockSets<'_, ReserveOrActivateIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, _dest_place: &mir::Place) { diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index fe063b2a4b69b..7eb1cae406fea 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -363,15 +363,19 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedLvals<'a, 'gcx, 'tcx> { } fn propagate_call_return(&self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, MovePathIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, dest_place: &mir::Place) { // when a call returns successfully, that means we need to set // the bits for that dest_place to 1 (initialized). - on_lookup_result_bits(self.tcx, self.mir, self.move_data(), - self.move_data().rev_lookup.find(dest_place), - |mpi| { in_out.add(&mpi); }); + on_lookup_result_bits( + self.tcx, + self.mir, + self.move_data(), + self.move_data().rev_lookup.find(dest_place), + |mpi| sets.gen(&mpi), + ); } } @@ -418,15 +422,19 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedLvals<'a, 'gcx, 'tcx> { } fn propagate_call_return(&self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, MovePathIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, dest_place: &mir::Place) { // when a call returns successfully, that means we need to set // the bits for that dest_place to 0 (initialized). - on_lookup_result_bits(self.tcx, self.mir, self.move_data(), - self.move_data().rev_lookup.find(dest_place), - |mpi| { in_out.remove(&mpi); }); + on_lookup_result_bits( + self.tcx, + self.mir, + self.move_data(), + self.move_data().rev_lookup.find(dest_place), + |mpi| sets.kill(&mpi), + ); } } @@ -472,15 +480,19 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'gcx, 'tcx } fn propagate_call_return(&self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, MovePathIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, dest_place: &mir::Place) { // when a call returns successfully, that means we need to set // the bits for that dest_place to 1 (initialized). - on_lookup_result_bits(self.tcx, self.mir, self.move_data(), - self.move_data().rev_lookup.find(dest_place), - |mpi| { in_out.add(&mpi); }); + on_lookup_result_bits( + self.tcx, + self.mir, + self.move_data(), + self.move_data().rev_lookup.find(dest_place), + |mpi| sets.gen(&mpi), + ); } } @@ -550,14 +562,16 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { let bits_per_block = self.bits_per_block(); let path_map = &move_data.path_map; - on_lookup_result_bits(self.tcx, - self.mir, - move_data, - move_data.rev_lookup.find(dest_place), - |mpi| for moi in &path_map[mpi] { - assert!(moi.index() < bits_per_block); - in_out.remove(&moi); - }); + on_lookup_result_bits( + self.tcx, + self.mir, + move_data, + move_data.rev_lookup.find(dest_place), + |mpi| for moi in &path_map[mpi] { + assert!(moi.index() < bits_per_block); + sets.kill(&moi); + }, + ); } } @@ -638,7 +652,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { } fn propagate_call_return(&self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, InitIndex>, call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, _dest_place: &mir::Place) { @@ -652,7 +666,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { }; for init_index in &init_loc_map[call_loc] { assert!(init_index.index() < bits_per_block); - in_out.add(init_index); + sets.gen(init_index); } } } diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 5108b1a6d057a..db17f925d2a9d 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -237,6 +237,8 @@ impl<'b, 'a: 'b, 'tcx: 'a, BD> PropagationContext<'b, 'a, 'tcx, BD> where BD: Bi { fn walk_cfg(&mut self, in_out: &mut IdxSet) { let mir = self.builder.mir; + let mut temp_gens = IdxSetBuf::new_empty(self.builder.flow_state.sets.bits_per_block); + let mut temp_kills = IdxSetBuf::new_empty(self.builder.flow_state.sets.bits_per_block); for (bb_idx, bb_data) in mir.basic_blocks().iter().enumerate() { let builder = &mut self.builder; { @@ -246,8 +248,15 @@ impl<'b, 'a: 'b, 'tcx: 'a, BD> PropagationContext<'b, 'a, 'tcx, BD> where BD: Bi in_out.union(sets.gen_set); in_out.subtract(sets.kill_set); } + + let sets = &mut BlockSets { + on_entry: in_out, + gen_set: &mut temp_gens, + kill_set: &mut temp_kills, + }; + builder.propagate_bits_into_graph_successors_of( - in_out, &mut self.changed, (mir::BasicBlock::new(bb_idx), bb_data)); + sets, &mut self.changed, (mir::BasicBlock::new(bb_idx), bb_data)); } } } @@ -555,11 +564,16 @@ impl<'a, E:Idx> BlockSets<'a, E> { self.on_entry.union(&self.gen_set); self.on_entry.subtract(&self.kill_set); } + + fn clear_local_effect(&mut self) { + self.gen_set.clear(); + self.kill_set.clear(); + } } impl AllSets { pub fn bits_per_block(&self) -> usize { self.bits_per_block } - pub fn for_block(&mut self, block_idx: usize) -> BlockSets { + pub fn for_block(&mut self, block_idx: usize) -> BlockSets<'_, E> { let offset = self.words_per_block * block_idx; let range = E::new(offset)..E::new(offset + self.words_per_block); BlockSets { @@ -725,7 +739,7 @@ pub trait BitDenotation: BitwiseOperator { /// kill-sets associated with each edge coming out of the basic /// block. fn propagate_call_return(&self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, Self::Idx>, call_bb: mir::BasicBlock, dest_bb: mir::BasicBlock, dest_place: &mir::Place); @@ -804,7 +818,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation /// unwind target). fn propagate_bits_into_graph_successors_of( &mut self, - in_out: &mut IdxSet, + sets: &mut BlockSets<'_, D::Idx>, changed: &mut bool, (bb, bb_data): (mir::BasicBlock, &mir::BasicBlockData)) { @@ -821,45 +835,47 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation mir::TerminatorKind::DropAndReplace { ref target, value: _, location: _, unwind: None } => { - self.propagate_bits_into_entry_set_for(in_out, changed, target); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); } mir::TerminatorKind::Yield { resume: ref target, drop: Some(ref drop), .. } => { - self.propagate_bits_into_entry_set_for(in_out, changed, target); - self.propagate_bits_into_entry_set_for(in_out, changed, drop); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, drop); } mir::TerminatorKind::Assert { ref target, cleanup: Some(ref unwind), .. } | mir::TerminatorKind::Drop { ref target, location: _, unwind: Some(ref unwind) } | mir::TerminatorKind::DropAndReplace { ref target, value: _, location: _, unwind: Some(ref unwind) } => { - self.propagate_bits_into_entry_set_for(in_out, changed, target); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); if !self.dead_unwinds.contains(&bb) { - self.propagate_bits_into_entry_set_for(in_out, changed, unwind); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, unwind); } } mir::TerminatorKind::SwitchInt { ref targets, .. } => { for target in targets { - self.propagate_bits_into_entry_set_for(in_out, changed, target); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); } } mir::TerminatorKind::Call { ref cleanup, ref destination, func: _, args: _ } => { if let Some(ref unwind) = *cleanup { if !self.dead_unwinds.contains(&bb) { - self.propagate_bits_into_entry_set_for(in_out, changed, unwind); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, unwind); } } if let Some((ref dest_place, ref dest_bb)) = *destination { // N.B.: This must be done *last*, after all other // propagation, as documented in comment above. + sets.clear_local_effect(); self.flow_state.operator.propagate_call_return( - in_out, bb, *dest_bb, dest_place); - self.propagate_bits_into_entry_set_for(in_out, changed, dest_bb); + sets, bb, *dest_bb, dest_place); + sets.apply_local_effect(); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, dest_bb); } } mir::TerminatorKind::FalseEdges { ref real_target, ref imaginary_targets } => { - self.propagate_bits_into_entry_set_for(in_out, changed, real_target); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, real_target); for target in imaginary_targets { - self.propagate_bits_into_entry_set_for(in_out, changed, target); + self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); } } } From 305686163ff0fc6bdf1acca2c8a3e4766db86b0c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Dec 2017 10:29:35 -0500 Subject: [PATCH 4/8] s/&mut sets.on_entry/&sets.on_entry/ --- src/librustc_mir/dataflow/mod.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index db17f925d2a9d..8c6284ac15fa0 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -835,31 +835,31 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation mir::TerminatorKind::DropAndReplace { ref target, value: _, location: _, unwind: None } => { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); } mir::TerminatorKind::Yield { resume: ref target, drop: Some(ref drop), .. } => { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, drop); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, drop); } mir::TerminatorKind::Assert { ref target, cleanup: Some(ref unwind), .. } | mir::TerminatorKind::Drop { ref target, location: _, unwind: Some(ref unwind) } | mir::TerminatorKind::DropAndReplace { ref target, value: _, location: _, unwind: Some(ref unwind) } => { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); if !self.dead_unwinds.contains(&bb) { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, unwind); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, unwind); } } mir::TerminatorKind::SwitchInt { ref targets, .. } => { for target in targets { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); } } mir::TerminatorKind::Call { ref cleanup, ref destination, func: _, args: _ } => { if let Some(ref unwind) = *cleanup { if !self.dead_unwinds.contains(&bb) { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, unwind); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, unwind); } } if let Some((ref dest_place, ref dest_bb)) = *destination { @@ -869,13 +869,13 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation self.flow_state.operator.propagate_call_return( sets, bb, *dest_bb, dest_place); sets.apply_local_effect(); - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, dest_bb); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, dest_bb); } } mir::TerminatorKind::FalseEdges { ref real_target, ref imaginary_targets } => { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, real_target); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, real_target); for target in imaginary_targets { - self.propagate_bits_into_entry_set_for(&mut sets.on_entry, changed, target); + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); } } } From 7619704d5246cab020f725582f9c50575d73dbff Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 31 Dec 2017 07:01:05 -0500 Subject: [PATCH 5/8] introduce (unused) scratch buffer --- src/librustc_mir/dataflow/mod.rs | 16 ++++++++++++---- src/librustc_mir/lib.rs | 1 + 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 8c6284ac15fa0..471d80b0e389e 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -20,6 +20,7 @@ use rustc::session::Session; use std::borrow::{Borrow, Cow}; use std::fmt; +use std::iter; use std::io; use std::mem; use std::path::PathBuf; @@ -237,8 +238,10 @@ impl<'b, 'a: 'b, 'tcx: 'a, BD> PropagationContext<'b, 'a, 'tcx, BD> where BD: Bi { fn walk_cfg(&mut self, in_out: &mut IdxSet) { let mir = self.builder.mir; - let mut temp_gens = IdxSetBuf::new_empty(self.builder.flow_state.sets.bits_per_block); - let mut temp_kills = IdxSetBuf::new_empty(self.builder.flow_state.sets.bits_per_block); + let bits_per_block = self.builder.flow_state.sets.bits_per_block; + let mut temp_gens = IdxSetBuf::new_empty(bits_per_block); + let mut temp_kills = IdxSetBuf::new_empty(bits_per_block); + let mut scratch_buf = IdxSetBuf::new_empty(bits_per_block); for (bb_idx, bb_data) in mir.basic_blocks().iter().enumerate() { let builder = &mut self.builder; { @@ -256,7 +259,11 @@ impl<'b, 'a: 'b, 'tcx: 'a, BD> PropagationContext<'b, 'a, 'tcx, BD> where BD: Bi }; builder.propagate_bits_into_graph_successors_of( - sets, &mut self.changed, (mir::BasicBlock::new(bb_idx), bb_data)); + sets, + &mut scratch_buf, + &mut self.changed, + (mir::BasicBlock::new(bb_idx), bb_data), + ); } } } @@ -819,8 +826,9 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation fn propagate_bits_into_graph_successors_of( &mut self, sets: &mut BlockSets<'_, D::Idx>, + scratch_buf: &mut IdxSet, changed: &mut bool, - (bb, bb_data): (mir::BasicBlock, &mir::BasicBlockData)) + (bb, bb_data): (mir::BasicBlock, &mir::BasicBlockData<'tcx>)) { match bb_data.terminator().kind { mir::TerminatorKind::Return | diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 1699ad0f19cf6..46651a411818f 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -37,6 +37,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![feature(collection_placement)] #![feature(nonzero)] #![feature(underscore_lifetimes)] +#![feature(universal_impl_trait)] #[macro_use] extern crate bitflags; From 02854722fa1fb45d7cfa18ecbc1166b28f54f20b Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 31 Dec 2017 07:01:46 -0500 Subject: [PATCH 6/8] introduce helper for propagating bits across a list of edges Don't use for calls yet. --- src/librustc_data_structures/indexed_set.rs | 5 ++ src/librustc_mir/dataflow/mod.rs | 99 +++++++++++++++------ 2 files changed, 76 insertions(+), 28 deletions(-) diff --git a/src/librustc_data_structures/indexed_set.rs b/src/librustc_data_structures/indexed_set.rs index 223e08de826ce..7266c1f064bf1 100644 --- a/src/librustc_data_structures/indexed_set.rs +++ b/src/librustc_data_structures/indexed_set.rs @@ -161,6 +161,11 @@ impl IdxSet { } } + /// True if there are no elements + pub fn is_empty(&self) -> bool { + self.bits.iter().all(|&b| b == 0) + } + /// Removes all elements pub fn clear(&mut self) { for b in &mut self.bits { diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 471d80b0e389e..40a0487025bb9 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -830,47 +830,72 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation changed: &mut bool, (bb, bb_data): (mir::BasicBlock, &mir::BasicBlockData<'tcx>)) { - match bb_data.terminator().kind { + let terminator = bb_data.terminator(); + match terminator.kind { mir::TerminatorKind::Return | mir::TerminatorKind::Resume | mir::TerminatorKind::Abort | mir::TerminatorKind::GeneratorDrop | mir::TerminatorKind::Unreachable => {} - mir::TerminatorKind::Goto { ref target } | - mir::TerminatorKind::Assert { ref target, cleanup: None, .. } | - mir::TerminatorKind::Yield { resume: ref target, drop: None, .. } | - mir::TerminatorKind::Drop { ref target, location: _, unwind: None } | + mir::TerminatorKind::Goto { target } | + mir::TerminatorKind::Assert { target, cleanup: None, .. } | + mir::TerminatorKind::Yield { resume: target, drop: None, .. } | + mir::TerminatorKind::Drop { target, location: _, unwind: None } | mir::TerminatorKind::DropAndReplace { - ref target, value: _, location: _, unwind: None + target, value: _, location: _, unwind: None } => { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); + self.propagate_bits_across_edges( + sets, + scratch_buf, + changed, + bb, + terminator, + &[target], + ) } - mir::TerminatorKind::Yield { resume: ref target, drop: Some(ref drop), .. } => { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, drop); + mir::TerminatorKind::Yield { resume: target, drop: Some(drop), .. } => { + self.propagate_bits_across_edges( + sets, + scratch_buf, + changed, + bb, + terminator, + &[target, drop], + ) } - mir::TerminatorKind::Assert { ref target, cleanup: Some(ref unwind), .. } | - mir::TerminatorKind::Drop { ref target, location: _, unwind: Some(ref unwind) } | + mir::TerminatorKind::Assert { target, cleanup: Some(unwind), .. } | + mir::TerminatorKind::Drop { target, location: _, unwind: Some(unwind) } | mir::TerminatorKind::DropAndReplace { - ref target, value: _, location: _, unwind: Some(ref unwind) + target, value: _, location: _, unwind: Some(unwind) } => { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); - if !self.dead_unwinds.contains(&bb) { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, unwind); - } + let all_targets = [target, unwind]; + let unwind_is_dead = self.dead_unwinds.contains(&bb); + self.propagate_bits_across_edges( + sets, + scratch_buf, + changed, + bb, + terminator, + if unwind_is_dead { &all_targets[..1] } else { &all_targets[..] }, + ) } mir::TerminatorKind::SwitchInt { ref targets, .. } => { - for target in targets { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); - } + self.propagate_bits_across_edges( + sets, + scratch_buf, + changed, + bb, + terminator, + targets, + ) } - mir::TerminatorKind::Call { ref cleanup, ref destination, func: _, args: _ } => { - if let Some(ref unwind) = *cleanup { + mir::TerminatorKind::Call { cleanup, ref destination, func: _, args: _ } => { + if let Some(ref unwind) = cleanup { if !self.dead_unwinds.contains(&bb) { self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, unwind); } } - if let Some((ref dest_place, ref dest_bb)) = *destination { + if let Some((dest_place, dest_bb)) = destination { // N.B.: This must be done *last*, after all other // propagation, as documented in comment above. sets.clear_local_effect(); @@ -880,15 +905,33 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, dest_bb); } } - mir::TerminatorKind::FalseEdges { ref real_target, ref imaginary_targets } => { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, real_target); - for target in imaginary_targets { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); - } + mir::TerminatorKind::FalseEdges { real_target, ref imaginary_targets } => { + self.propagate_bits_across_edges( + sets, + scratch_buf, + changed, + bb, + terminator, + iter::once(&real_target).chain(imaginary_targets), + ) } } } + fn propagate_bits_across_edges<'bb>( + &mut self, + sets: &mut BlockSets<'_, D::Idx>, + _scratch_buf: &mut IdxSet, + changed: &mut bool, + _source: mir::BasicBlock, + _terminator: &mir::Terminator<'tcx>, + targets: impl IntoIterator, + ) { + for target in targets { + self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); + } + } + fn propagate_bits_into_entry_set_for(&mut self, in_out: &IdxSet, changed: &mut bool, From 5fe40f6a44f45bdde9cce8fa5bf523331729aab0 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 31 Dec 2017 11:49:25 -0500 Subject: [PATCH 7/8] generalize `propagate_call_return` to `edge_effect` No functional change intended. --- src/librustc_mir/dataflow/impls/borrows.rs | 30 +-- src/librustc_mir/dataflow/impls/mod.rs | 197 +++++++++++------- .../dataflow/impls/storage_liveness.rs | 14 +- src/librustc_mir/dataflow/mod.rs | 173 ++++++++++----- 4 files changed, 268 insertions(+), 146 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index adfde9261c301..59d988e9d5c9e 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -23,7 +23,7 @@ use rustc_data_structures::bitslice::{BitwiseOperator}; use rustc_data_structures::indexed_set::{IdxSet}; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; -use dataflow::{BitDenotation, BlockSets, InitialFlow}; +use dataflow::{BitDenotation, BlockSets, EdgeKind, InitialFlow}; pub use dataflow::indexes::{BorrowIndex, ReserveOrActivateIndex}; use borrow_check::nll::region_infer::RegionInferenceContext; use borrow_check::nll::ToRegionVid; @@ -677,12 +677,14 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Reservations<'a, 'gcx, 'tcx> { self.0.terminator_effect_on_borrows(sets, location, false); } - fn propagate_call_return(&self, - _sets: &mut BlockSets<'_, ReserveOrActivateIndex>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - _dest_place: &mir::Place) { - // there are no effects on borrows from method call return... + fn edge_effect( + &self, + _sets: &mut BlockSets, + _source_block: mir::BasicBlock, + _edge_kind: EdgeKind<'_>, + _target_terminator: mir::BasicBlock, + ) { + // there are no effects on borrows from edges... // // ... but if overwriting a place can affect flow state, then // latter is not true; see NOTE on Assign case in @@ -738,12 +740,14 @@ impl<'a, 'gcx, 'tcx> BitDenotation for ActiveBorrows<'a, 'gcx, 'tcx> { self.0.terminator_effect_on_borrows(sets, location, true); } - fn propagate_call_return(&self, - _sets: &mut BlockSets<'_, ReserveOrActivateIndex>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - _dest_place: &mir::Place) { - // there are no effects on borrows from method call return... + fn edge_effect( + &self, + _sets: &mut BlockSets, + _source_block: mir::BasicBlock, + _edge_kind: EdgeKind<'_>, + _target_terminator: mir::BasicBlock, + ) { + // there are no effects on borrows from edges... // // ... but If overwriting a place can affect flow state, then // latter is not true; see NOTE on Assign case in diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 7eb1cae406fea..b955d9faf9e2d 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -23,7 +23,7 @@ use util::elaborate_drops::DropFlagState; use super::move_paths::{HasMoveData, MoveData, MoveOutIndex, MovePathIndex, InitIndex}; use super::move_paths::{LookupResult, InitKind}; -use super::{BitDenotation, BlockSets, InitialFlow}; +use super::{BitDenotation, BlockSets, EdgeKind, InitialFlow}; use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; @@ -362,20 +362,29 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedLvals<'a, 'gcx, 'tcx> { ) } - fn propagate_call_return(&self, - sets: &mut BlockSets<'_, MovePathIndex>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - dest_place: &mir::Place) { - // when a call returns successfully, that means we need to set - // the bits for that dest_place to 1 (initialized). - on_lookup_result_bits( - self.tcx, - self.mir, - self.move_data(), - self.move_data().rev_lookup.find(dest_place), - |mpi| sets.gen(&mpi), - ); + fn edge_effect( + &self, + sets: &mut BlockSets, + _source_block: mir::BasicBlock, + edge_kind: EdgeKind<'_>, + _target_block: mir::BasicBlock, + ) { + match edge_kind { + EdgeKind::CallReturn(dest_place) => { + // when a call returns successfully, that means we need to set + // the bits for that dest_place to 1 (initialized). + on_lookup_result_bits( + self.tcx, + self.mir, + self.move_data(), + self.move_data().rev_lookup.find(dest_place), + |mpi| sets.gen(&mpi), + ); + } + + EdgeKind::Noop | EdgeKind::FalseEdge => { + } + } } } @@ -421,20 +430,29 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedLvals<'a, 'gcx, 'tcx> { ) } - fn propagate_call_return(&self, - sets: &mut BlockSets<'_, MovePathIndex>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - dest_place: &mir::Place) { - // when a call returns successfully, that means we need to set - // the bits for that dest_place to 0 (initialized). - on_lookup_result_bits( - self.tcx, - self.mir, - self.move_data(), - self.move_data().rev_lookup.find(dest_place), - |mpi| sets.kill(&mpi), - ); + fn edge_effect( + &self, + sets: &mut BlockSets, + _source_block: mir::BasicBlock, + edge_kind: EdgeKind<'_>, + _target_block: mir::BasicBlock, + ) { + match edge_kind { + EdgeKind::CallReturn(dest_place) => { + // when a call returns successfully, that means we need to set + // the bits for that dest_place to 0 (initialized). + on_lookup_result_bits( + self.tcx, + self.mir, + self.move_data(), + self.move_data().rev_lookup.find(dest_place), + |mpi| sets.kill(&mpi), + ); + } + + EdgeKind::Noop | EdgeKind::FalseEdge => { + } + } } } @@ -479,20 +497,29 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'gcx, 'tcx ) } - fn propagate_call_return(&self, - sets: &mut BlockSets<'_, MovePathIndex>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - dest_place: &mir::Place) { - // when a call returns successfully, that means we need to set - // the bits for that dest_place to 1 (initialized). - on_lookup_result_bits( - self.tcx, - self.mir, - self.move_data(), - self.move_data().rev_lookup.find(dest_place), - |mpi| sets.gen(&mpi), - ); + fn edge_effect( + &self, + sets: &mut BlockSets, + _source_block: mir::BasicBlock, + edge_kind: EdgeKind<'_>, + _target_block: mir::BasicBlock, + ) { + match edge_kind { + EdgeKind::CallReturn(dest_place) => { + // when a call returns successfully, that means we need to set + // the bits for that dest_place to 1 (initialized). + on_lookup_result_bits( + self.tcx, + self.mir, + self.move_data(), + self.move_data().rev_lookup.find(dest_place), + |mpi| sets.gen(&mpi), + ); + } + + EdgeKind::Noop | EdgeKind::FalseEdge => { + } + } } } @@ -553,25 +580,34 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { |mpi| sets.kill_all(&path_map[mpi])); } - fn propagate_call_return(&self, - sets: &mut BlockSets<'_, MoveOutIndex>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - dest_place: &mir::Place) { - let move_data = self.move_data(); - let bits_per_block = self.bits_per_block(); + fn edge_effect( + &self, + sets: &mut BlockSets, + _source_block: mir::BasicBlock, + edge_kind: EdgeKind<'_>, + _target_terminator: mir::BasicBlock, + ) { + match edge_kind { + EdgeKind::CallReturn(dest_place) => { + let move_data = self.move_data(); + let bits_per_block = self.bits_per_block(); + + let path_map = &move_data.path_map; + on_lookup_result_bits( + self.tcx, + self.mir, + move_data, + move_data.rev_lookup.find(dest_place), + |mpi| for moi in &path_map[mpi] { + assert!(moi.index() < bits_per_block); + sets.kill(&moi); + }, + ); + } - let path_map = &move_data.path_map; - on_lookup_result_bits( - self.tcx, - self.mir, - move_data, - move_data.rev_lookup.find(dest_place), - |mpi| for moi in &path_map[mpi] { - assert!(moi.index() < bits_per_block); - sets.kill(&moi); - }, - ); + EdgeKind::Noop | EdgeKind::FalseEdge => { + } + } } } @@ -651,22 +687,31 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { ); } - fn propagate_call_return(&self, - sets: &mut BlockSets<'_, InitIndex>, - call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - _dest_place: &mir::Place) { - let move_data = self.move_data(); - let bits_per_block = self.bits_per_block(); - let init_loc_map = &move_data.init_loc_map; + fn edge_effect( + &self, + sets: &mut BlockSets, + source_block: mir::BasicBlock, + edge_kind: EdgeKind<'_>, + _target_block: mir::BasicBlock, + ) { + match edge_kind { + EdgeKind::CallReturn(_) => { + let move_data = self.move_data(); + let bits_per_block = self.bits_per_block(); + let init_loc_map = &move_data.init_loc_map; + + let call_loc = Location { + block: source_block, + statement_index: self.mir[source_block].statements.len(), + }; + for init_index in &init_loc_map[call_loc] { + assert!(init_index.index() < bits_per_block); + sets.gen(init_index); + } + } - let call_loc = Location { - block: call_bb, - statement_index: self.mir[call_bb].statements.len(), - }; - for init_index in &init_loc_map[call_loc] { - assert!(init_index.index() < bits_per_block); - sets.gen(init_index); + EdgeKind::Noop | EdgeKind::FalseEdge => { + } } } } diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index 0a6e5c7f3e97e..bb400037cda65 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -58,12 +58,14 @@ impl<'a, 'tcx> BitDenotation for MaybeStorageLive<'a, 'tcx> { // Terminators have no effect } - fn propagate_call_return(&self, - _sets: &mut BlockSets<'_, Local>, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, - _dest_place: &mir::Place) { - // Nothing to do when a call returns successfully + fn edge_effect( + &self, + _sets: &mut BlockSets, + _source_block: mir::BasicBlock, + _edge_kind: EdgeKind<'_>, + _target_terminator: mir::BasicBlock, + ) { + // No special effects on edges } } diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 40a0487025bb9..da583f89af321 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -15,7 +15,7 @@ use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::bitslice::{bitwise, BitwiseOperator}; use rustc::ty::{self, TyCtxt}; -use rustc::mir::{self, Mir, BasicBlock, BasicBlockData, Location, Statement, Terminator}; +use rustc::mir::{self, BasicBlock, BasicBlockData, Location, Mir, Place, Statement, Terminator}; use rustc::session::Session; use std::borrow::{Borrow, Cow}; @@ -576,6 +576,10 @@ impl<'a, E:Idx> BlockSets<'a, E> { self.gen_set.clear(); self.kill_set.clear(); } + + fn has_empty_local_effect(&mut self) -> bool { + self.gen_set.is_empty() && self.kill_set.is_empty() + } } impl AllSets { @@ -721,35 +725,59 @@ pub trait BitDenotation: BitwiseOperator { /// block, represented via GEN and KILL sets. /// /// The effects applied here cannot depend on which branch the - /// terminator took. + /// terminator took. Hence they are best understood as the effects + /// up to -- but not including -- the branches. fn terminator_effect(&self, sets: &mut BlockSets<'_, Self::Idx>, location: Location); /// Mutates the block-sets according to the (flow-dependent) - /// effect of a successful return from a Call terminator. + /// effect of a particular outgoing edge from a terminator. For + /// many terminators/operators, this is a no-op, since the effect + /// of the terminator is not dependent on which branch is taken + /// and hence can be accumulated via `terminator_effect`. + /// + /// One example where this callback is needed involves Call + /// terminators. In the case of a call terminator: /// - /// If basic-block BB_x ends with a call-instruction that, upon - /// successful return, flows to BB_y, then this method will be - /// called on the exit flow-state of BB_x in order to set up the - /// entry flow-state of BB_y. + /// tmp0 = call foo(...) /// - /// This is used, in particular, as a special case during the - /// "propagate" loop where all of the basic blocks are repeatedly - /// visited. Since the effects of a Call terminator are - /// flow-dependent, the current MIR cannot encode them via just - /// GEN and KILL sets attached to the block, and so instead we add - /// this extra machinery to represent the flow-dependent effect. + /// the assignment to `tmp0` only occurs if the call returns + /// normally (without unwinding). Therefore, we wish to apply the + /// effect of considering `tmp0` to be initialized only on the one + /// edge. /// - /// FIXME: Right now this is a bit of a wart in the API. It might - /// be better to represent this as an additional gen- and - /// kill-sets associated with each edge coming out of the basic - /// block. - fn propagate_call_return(&self, - sets: &mut BlockSets<'_, Self::Idx>, - call_bb: mir::BasicBlock, - dest_bb: mir::BasicBlock, - dest_place: &mir::Place); + /// The `edge_kind` parameter can be used to determine what sort + /// of terminator this is (you may need to add variants, though, + /// as the current set is somewhat minimal). + /// + /// Note that, during propagation, edge-specific effects are not + /// accumulated into the overall gen-kill sets for a block, and + /// hence this function will be called repeatedly as we iterate to + /// a fixed point. But so long as you define this callback (and + /// the rest) as a "pure function", this need not concern you. + fn edge_effect( + &self, + sets: &mut BlockSets, + source_block: mir::BasicBlock, + edge_kind: EdgeKind<'_>, + target_terminator: mir::BasicBlock, + ); +} + +#[derive(Copy, Clone)] +pub enum EdgeKind<'mir> { + /// A standard edge -- one where the terminator does not + /// perform any action along the edge. The edge may be a normal + /// or an unwinding edge. + Noop, + + /// An edge that doesn't really execute at runtime. + FalseEdge, + + /// A "call return" edge, where the return value of a call (a call + /// that did not unwind) is stored into its destination. + CallReturn(&'mir Place<'mir>), } impl<'a, 'tcx, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation @@ -830,8 +858,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation changed: &mut bool, (bb, bb_data): (mir::BasicBlock, &mir::BasicBlockData<'tcx>)) { - let terminator = bb_data.terminator(); - match terminator.kind { + match bb_data.terminator().kind { mir::TerminatorKind::Return | mir::TerminatorKind::Resume | mir::TerminatorKind::Abort | @@ -849,8 +876,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation scratch_buf, changed, bb, - terminator, - &[target], + iter::once((target, EdgeKind::Noop)), ) } mir::TerminatorKind::Yield { resume: target, drop: Some(drop), .. } => { @@ -859,8 +885,8 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation scratch_buf, changed, bb, - terminator, - &[target, drop], + iter::once((target, EdgeKind::Noop)) + .chain(iter::once((drop, EdgeKind::Noop))), ) } mir::TerminatorKind::Assert { target, cleanup: Some(unwind), .. } | @@ -868,15 +894,15 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation mir::TerminatorKind::DropAndReplace { target, value: _, location: _, unwind: Some(unwind) } => { - let all_targets = [target, unwind]; + let all_targets = [(target, EdgeKind::Noop), (unwind, EdgeKind::Noop)]; let unwind_is_dead = self.dead_unwinds.contains(&bb); + let targets = if unwind_is_dead { &all_targets[..1] } else { &all_targets[..] }; self.propagate_bits_across_edges( sets, scratch_buf, changed, bb, - terminator, - if unwind_is_dead { &all_targets[..1] } else { &all_targets[..] }, + targets.iter().cloned(), ) } mir::TerminatorKind::SwitchInt { ref targets, .. } => { @@ -885,25 +911,30 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation scratch_buf, changed, bb, - terminator, - targets, + targets.into_iter().map(|&target| (target, EdgeKind::Noop)), ) } mir::TerminatorKind::Call { cleanup, ref destination, func: _, args: _ } => { - if let Some(ref unwind) = cleanup { + let mut unwind_edge = None; + let mut normal_edge = None; + + if let Some(unwind) = cleanup { if !self.dead_unwinds.contains(&bb) { - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, unwind); + unwind_edge = Some((unwind, EdgeKind::Noop)); } } + if let Some((dest_place, dest_bb)) = destination { - // N.B.: This must be done *last*, after all other - // propagation, as documented in comment above. - sets.clear_local_effect(); - self.flow_state.operator.propagate_call_return( - sets, bb, *dest_bb, dest_place); - sets.apply_local_effect(); - self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, dest_bb); + normal_edge = Some((*dest_bb, EdgeKind::CallReturn(dest_place))); } + + self.propagate_bits_across_edges( + sets, + scratch_buf, + changed, + bb, + unwind_edge.into_iter().chain(normal_edge), + ) } mir::TerminatorKind::FalseEdges { real_target, ref imaginary_targets } => { self.propagate_bits_across_edges( @@ -911,23 +942,63 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation scratch_buf, changed, bb, - terminator, - iter::once(&real_target).chain(imaginary_targets), + iter::once((real_target, EdgeKind::Noop)) + .chain( + imaginary_targets.into_iter() + .map(|&target| (target, EdgeKind::FalseEdge)), + ), ) } } } - fn propagate_bits_across_edges<'bb>( + fn propagate_bits_across_edges<'ek>( &mut self, sets: &mut BlockSets<'_, D::Idx>, - _scratch_buf: &mut IdxSet, + scratch_buf: &mut IdxSet, changed: &mut bool, - _source: mir::BasicBlock, - _terminator: &mir::Terminator<'tcx>, - targets: impl IntoIterator, + source: mir::BasicBlock, + targets: impl IntoIterator)>, ) { - for target in targets { + // When true, the initial value of `sets.on_entry` has been copied + // into `scratch_buf`. + let mut is_saved = false; + + // When true, the value of `sets.on_entry` has been changed + // from its initial value (and not yet restored). + let mut is_dirty = false; + + // Just in case some previous caller left them dirty, clear + // the gen/kill sets to start. + sets.clear_local_effect(); + + for (target, edge_kind) in targets { + if is_dirty { + // Some previous edge generated (and applied) gen/kill + // effects. Undo them. + assert!(is_saved); + sets.clear_local_effect(); + sets.on_entry.clone_from(scratch_buf); + is_dirty = false; + } + + // Compute the gen/kill sets for this edge. + self.flow_state.operator.edge_effect(sets, source, edge_kind, target); + + // If those gen/kill sets are non-empty, apply them. + if !sets.has_empty_local_effect() { + if !is_saved { + // But first, save the "pristine" on-entry set so + // that we can restore it for other edges. + scratch_buf.clone_from(&sets.on_entry); + is_saved = true; + } + + sets.apply_local_effect(); + is_dirty = true; + } + + // Update the on-entry set for `target`. self.propagate_bits_into_entry_set_for(&sets.on_entry, changed, target); } } @@ -935,7 +1006,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation fn propagate_bits_into_entry_set_for(&mut self, in_out: &IdxSet, changed: &mut bool, - bb: &mir::BasicBlock) { + bb: mir::BasicBlock) { let entry_set = self.flow_state.sets.for_block(bb.index()).on_entry; let set_changed = bitwise(entry_set.words_mut(), in_out.words(), From 642815f72eb3d574e448cdea5ea7332c6aa0a455 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 31 Dec 2017 14:11:12 -0500 Subject: [PATCH 8/8] micro-optimize "anything changed?" check The "access tracker" should seamlessly distiguish: - No edge effects at all -- in this case there would be no assignments at all, and hence the "maybe mutated" flag should be constant propagated to false. - Sometimes edge effects -- in this case, we can determine if in fact a gen or kill bit was added in an O(1) check, rather than the old O(n) check. --- .../access_tracker.rs | 50 +++++++++++++++++++ src/librustc_data_structures/lib.rs | 1 + src/librustc_mir/dataflow/impls/borrows.rs | 5 +- src/librustc_mir/dataflow/impls/mod.rs | 11 ++-- .../dataflow/impls/storage_liveness.rs | 5 +- src/librustc_mir/dataflow/mod.rs | 14 ++++-- 6 files changed, 74 insertions(+), 12 deletions(-) create mode 100644 src/librustc_data_structures/access_tracker.rs diff --git a/src/librustc_data_structures/access_tracker.rs b/src/librustc_data_structures/access_tracker.rs new file mode 100644 index 0000000000000..8250ae1cfba50 --- /dev/null +++ b/src/librustc_data_structures/access_tracker.rs @@ -0,0 +1,50 @@ +// Copyright 2015 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. + +use std::ops::{Deref, DerefMut}; + +/// Takes ownership of `T` and tracks whether it was accessed mutably +/// (via `DerefMut`). You can access this via the `maybe_mutated` fn. +#[derive(Clone, Debug)] +pub struct AccessTracker { + value: T, + mutated: bool, +} + +impl AccessTracker { + pub fn new(value: T) -> Self { + AccessTracker { value, mutated: false } + } + + /// True if the owned value was accessed mutably (so far). + pub fn maybe_mutated(this: &Self) -> bool { + this.mutated + } + + pub fn into_inner(this: Self) -> (T, bool) { + (this.value, this.mutated) + } +} + +impl Deref for AccessTracker { + type Target = T; + + fn deref(&self) -> &T { + &self.value + } +} + +impl DerefMut for AccessTracker { + fn deref_mut(&mut self) -> &mut T { + self.mutated = true; + &mut self.value + } +} + diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index a35ef2f7ce7ba..946d0f57ea45c 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -52,6 +52,7 @@ extern crate stable_deref_trait; pub use rustc_serialize::hex::ToHex; pub mod array_vec; +pub mod access_tracker; pub mod accumulate_vec; pub mod small_vec; pub mod base_n; diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 59d988e9d5c9e..6e2131b1e774d 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -19,6 +19,7 @@ use rustc::ty::RegionKind; use rustc::ty::RegionKind::ReScope; use rustc::util::nodemap::{FxHashMap, FxHashSet}; +use rustc_data_structures::access_tracker::AccessTracker; use rustc_data_structures::bitslice::{BitwiseOperator}; use rustc_data_structures::indexed_set::{IdxSet}; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; @@ -679,7 +680,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Reservations<'a, 'gcx, 'tcx> { fn edge_effect( &self, - _sets: &mut BlockSets, + _sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, _edge_kind: EdgeKind<'_>, _target_terminator: mir::BasicBlock, @@ -742,7 +743,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for ActiveBorrows<'a, 'gcx, 'tcx> { fn edge_effect( &self, - _sets: &mut BlockSets, + _sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, _edge_kind: EdgeKind<'_>, _target_terminator: mir::BasicBlock, diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index b955d9faf9e2d..35d244925e918 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -14,6 +14,7 @@ use rustc::ty::TyCtxt; use rustc::mir::{self, Mir, Location}; +use rustc_data_structures::access_tracker::AccessTracker; use rustc_data_structures::bitslice::{BitwiseOperator}; use rustc_data_structures::indexed_set::{IdxSet}; use rustc_data_structures::indexed_vec::Idx; @@ -364,7 +365,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedLvals<'a, 'gcx, 'tcx> { fn edge_effect( &self, - sets: &mut BlockSets, + sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, edge_kind: EdgeKind<'_>, _target_block: mir::BasicBlock, @@ -432,7 +433,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedLvals<'a, 'gcx, 'tcx> { fn edge_effect( &self, - sets: &mut BlockSets, + sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, edge_kind: EdgeKind<'_>, _target_block: mir::BasicBlock, @@ -499,7 +500,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'gcx, 'tcx fn edge_effect( &self, - sets: &mut BlockSets, + sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, edge_kind: EdgeKind<'_>, _target_block: mir::BasicBlock, @@ -582,7 +583,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { fn edge_effect( &self, - sets: &mut BlockSets, + sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, edge_kind: EdgeKind<'_>, _target_terminator: mir::BasicBlock, @@ -689,7 +690,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { fn edge_effect( &self, - sets: &mut BlockSets, + sets: &mut AccessTracker<&mut BlockSets>, source_block: mir::BasicBlock, edge_kind: EdgeKind<'_>, _target_block: mir::BasicBlock, diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index bb400037cda65..7840e61355890 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -10,8 +10,9 @@ pub use super::*; -use rustc::mir::*; use dataflow::BitDenotation; +use rustc::mir::*; +use rustc_data_structures::access_tracker::AccessTracker; #[derive(Copy, Clone)] pub struct MaybeStorageLive<'a, 'tcx: 'a> { @@ -60,7 +61,7 @@ impl<'a, 'tcx> BitDenotation for MaybeStorageLive<'a, 'tcx> { fn edge_effect( &self, - _sets: &mut BlockSets, + _sets: &mut AccessTracker<&mut BlockSets>, _source_block: mir::BasicBlock, _edge_kind: EdgeKind<'_>, _target_terminator: mir::BasicBlock, diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index da583f89af321..f7699c8697e8d 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -10,6 +10,7 @@ use syntax::ast::{self, MetaItem}; +use rustc_data_structures::access_tracker::AccessTracker; use rustc_data_structures::indexed_set::{IdxSet, IdxSetBuf}; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::bitslice::{bitwise, BitwiseOperator}; @@ -758,7 +759,7 @@ pub trait BitDenotation: BitwiseOperator { /// the rest) as a "pure function", this need not concern you. fn edge_effect( &self, - sets: &mut BlockSets, + sets: &mut AccessTracker<&mut BlockSets>, source_block: mir::BasicBlock, edge_kind: EdgeKind<'_>, target_terminator: mir::BasicBlock, @@ -952,6 +953,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation } } + #[allow(non_camel_case_types)] // FIXME fn propagate_bits_across_edges<'ek>( &mut self, sets: &mut BlockSets<'_, D::Idx>, @@ -983,10 +985,14 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation } // Compute the gen/kill sets for this edge. - self.flow_state.operator.edge_effect(sets, source, edge_kind, target); + let sets_mutated = { + let mut tracked_sets = AccessTracker::new(&mut *sets); + self.flow_state.operator.edge_effect(&mut tracked_sets, source, edge_kind, target); + AccessTracker::maybe_mutated(&tracked_sets) + }; // If those gen/kill sets are non-empty, apply them. - if !sets.has_empty_local_effect() { + if sets_mutated { if !is_saved { // But first, save the "pristine" on-entry set so // that we can restore it for other edges. @@ -996,6 +1002,8 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation sets.apply_local_effect(); is_dirty = true; + } else { + debug_assert!(sets.has_empty_local_effect()); } // Update the on-entry set for `target`.