From 95186201724a087bacafc74317722314f203cfdc Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 21 Jan 2020 20:00:55 -0800 Subject: [PATCH 1/4] Use `ResultsCursor` for `elaborate_drops` The old code hard-coded the transfer function for the initialized places analyses. --- src/librustc_mir/transform/elaborate_drops.rs | 114 +++++------------- 1 file changed, 33 insertions(+), 81 deletions(-) diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 99f13f68cbc79..28a50547b190c 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -1,8 +1,8 @@ use crate::dataflow; -use crate::dataflow::generic::{Analysis, Results}; +use crate::dataflow::generic::{Analysis, ResultsCursor}; use crate::dataflow::move_paths::{LookupResult, MoveData, MovePathIndex}; +use crate::dataflow::on_lookup_result_bits; use crate::dataflow::MoveDataParamEnv; -use crate::dataflow::{drop_flag_effects_for_location, on_lookup_result_bits}; use crate::dataflow::{on_all_children_bits, on_all_drop_children_bits}; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; use crate::transform::{MirPass, MirSource}; @@ -41,22 +41,23 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { let env = MoveDataParamEnv { move_data, param_env }; let dead_unwinds = find_dead_unwinds(tcx, body, def_id, &env); - let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) + let inits = MaybeInitializedPlaces::new(tcx, body, &env) .into_engine(tcx, body, def_id) .dead_unwinds(&dead_unwinds) - .iterate_to_fixpoint(); + .iterate_to_fixpoint() + .into_results_cursor(body); - let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &env) + let uninits = MaybeUninitializedPlaces::new(tcx, body, &env) .into_engine(tcx, body, def_id) .dead_unwinds(&dead_unwinds) - .iterate_to_fixpoint(); + .iterate_to_fixpoint() + .into_results_cursor(body); ElaborateDropsCtxt { tcx, body, env: &env, - flow_inits, - flow_uninits, + init_data: InitializationData { inits, uninits }, drop_flags: Default::default(), patch: MirPatch::new(body), } @@ -79,9 +80,10 @@ fn find_dead_unwinds<'tcx>( // We only need to do this pass once, because unwind edges can only // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); - let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) + let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) .into_engine(tcx, body, def_id) - .iterate_to_fixpoint(); + .iterate_to_fixpoint() + .into_results_cursor(body); for (bb, bb_data) in body.basic_blocks().iter_enumerated() { let location = match bb_data.terminator().kind { TerminatorKind::Drop { ref location, unwind: Some(_), .. } @@ -89,15 +91,7 @@ fn find_dead_unwinds<'tcx>( _ => continue, }; - let mut init_data = InitializationData { - live: flow_inits.entry_set_for_block(bb).clone(), - dead: BitSet::new_empty(env.move_data.move_paths.len()), - }; - debug!("find_dead_unwinds @ {:?}: {:?}; init_data={:?}", bb, bb_data, init_data.live); - for stmt in 0..bb_data.statements.len() { - let loc = Location { block: bb, statement_index: stmt }; - init_data.apply_location(tcx, body, env, loc); - } + debug!("find_dead_unwinds @ {:?}: {:?}; init_data={:?}", bb, bb_data, flow_inits.get()); let path = match env.move_data.rev_lookup.find(location.as_ref()) { LookupResult::Exact(e) => e, @@ -109,10 +103,10 @@ fn find_dead_unwinds<'tcx>( debug!("find_dead_unwinds @ {:?}: path({:?})={:?}", bb, location, path); + flow_inits.seek_after(body.terminator_loc(bb)); let mut maybe_live = false; on_all_drop_children_bits(tcx, body, &env, path, |child| { - let (child_maybe_live, _) = init_data.state(child); - maybe_live |= child_maybe_live; + maybe_live |= flow_inits.contains(child); }); debug!("find_dead_unwinds @ {:?}: maybe_live={}", bb, maybe_live); @@ -124,41 +118,23 @@ fn find_dead_unwinds<'tcx>( dead_unwinds } -struct InitializationData { - live: BitSet, - dead: BitSet, +struct InitializationData<'mir, 'tcx> { + inits: ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, + uninits: ResultsCursor<'mir, 'tcx, MaybeUninitializedPlaces<'mir, 'tcx>>, } -impl InitializationData { - fn apply_location<'tcx>( - &mut self, - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - env: &MoveDataParamEnv<'tcx>, - loc: Location, - ) { - drop_flag_effects_for_location(tcx, body, env, loc, |path, df| { - debug!("at location {:?}: setting {:?} to {:?}", loc, path, df); - match df { - DropFlagState::Present => { - self.live.insert(path); - self.dead.remove(path); - } - DropFlagState::Absent => { - self.dead.insert(path); - self.live.remove(path); - } - } - }); +impl InitializationData<'_, '_> { + fn seek_after(&mut self, loc: Location) { + self.inits.seek_after(loc); + self.uninits.seek_after(loc); } fn state(&self, path: MovePathIndex) -> (bool, bool) { - (self.live.contains(path), self.dead.contains(path)) + (self.inits.contains(path), self.uninits.contains(path)) } } struct Elaborator<'a, 'b, 'tcx> { - init_data: &'a InitializationData, ctxt: &'a mut ElaborateDropsCtxt<'b, 'tcx>, } @@ -189,13 +165,13 @@ impl<'a, 'b, 'tcx> DropElaborator<'a, 'tcx> for Elaborator<'a, 'b, 'tcx> { fn drop_style(&self, path: Self::Path, mode: DropFlagMode) -> DropStyle { let ((maybe_live, maybe_dead), multipart) = match mode { - DropFlagMode::Shallow => (self.init_data.state(path), false), + DropFlagMode::Shallow => (self.ctxt.init_data.state(path), false), DropFlagMode::Deep => { let mut some_live = false; let mut some_dead = false; let mut children_count = 0; on_all_drop_children_bits(self.tcx(), self.body(), self.ctxt.env, path, |child| { - let (live, dead) = self.init_data.state(child); + let (live, dead) = self.ctxt.init_data.state(child); debug!("elaborate_drop: state({:?}) = {:?}", child, (live, dead)); some_live |= live; some_dead |= dead; @@ -269,8 +245,7 @@ struct ElaborateDropsCtxt<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, env: &'a MoveDataParamEnv<'tcx>, - flow_inits: Results<'tcx, MaybeInitializedPlaces<'a, 'tcx>>, - flow_uninits: Results<'tcx, MaybeUninitializedPlaces<'a, 'tcx>>, + init_data: InitializationData<'a, 'tcx>, drop_flags: FxHashMap, patch: MirPatch<'tcx>, } @@ -284,25 +259,6 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { self.env.param_env } - // FIXME(ecstaticmorse): This duplicates `dataflow::ResultsCursor` but hardcodes the transfer - // function for `Maybe{Un,}InitializedPlaces` directly. It should be replaced by a a pair of - // `ResultsCursor`s. - fn initialization_data_at(&self, loc: Location) -> InitializationData { - let mut data = InitializationData { - live: self.flow_inits.entry_set_for_block(loc.block).to_owned(), - dead: self.flow_uninits.entry_set_for_block(loc.block).to_owned(), - }; - for stmt in 0..loc.statement_index { - data.apply_location( - self.tcx, - self.body, - self.env, - Location { block: loc.block, statement_index: stmt }, - ); - } - data - } - fn create_drop_flag(&mut self, index: MovePathIndex, span: Span) { let tcx = self.tcx; let patch = &mut self.patch; @@ -338,10 +294,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { _ => continue, }; - let init_data = self.initialization_data_at(Location { - block: bb, - statement_index: data.statements.len(), - }); + self.init_data.seek_after(self.body.terminator_loc(bb)); let path = self.move_data().rev_lookup.find(location.as_ref()); debug!("collect_drop_flags: {:?}, place {:?} ({:?})", bb, location, path); @@ -350,7 +303,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { LookupResult::Exact(e) => e, LookupResult::Parent(None) => continue, LookupResult::Parent(Some(parent)) => { - let (_maybe_live, maybe_dead) = init_data.state(parent); + let (_maybe_live, maybe_dead) = self.init_data.state(parent); if maybe_dead { span_bug!( terminator.source_info.span, @@ -365,7 +318,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { }; on_all_drop_children_bits(self.tcx, self.body, self.env, path, |child| { - let (maybe_live, maybe_dead) = init_data.state(child); + let (maybe_live, maybe_dead) = self.init_data.state(child); debug!( "collect_drop_flags: collecting {:?} from {:?}@{:?} - {:?}", child, @@ -388,10 +341,10 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { let resume_block = self.patch.resume_block(); match terminator.kind { TerminatorKind::Drop { ref location, target, unwind } => { - let init_data = self.initialization_data_at(loc); + self.init_data.seek_after(loc); match self.move_data().rev_lookup.find(location.as_ref()) { LookupResult::Exact(path) => elaborate_drop( - &mut Elaborator { init_data: &init_data, ctxt: self }, + &mut Elaborator { ctxt: self }, terminator.source_info, location, path, @@ -471,10 +424,9 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { match self.move_data().rev_lookup.find(location.as_ref()) { LookupResult::Exact(path) => { debug!("elaborate_drop_and_replace({:?}) - tracked {:?}", terminator, path); - let init_data = self.initialization_data_at(loc); - + self.init_data.seek_after(loc); elaborate_drop( - &mut Elaborator { init_data: &init_data, ctxt: self }, + &mut Elaborator { ctxt: self }, terminator.source_info, location, path, From f0260ae196b62f27c64738d52519efb424724915 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 13 Feb 2020 20:48:17 -0800 Subject: [PATCH 2/4] Use `seek_before` instead of `seek_after` --- src/librustc_mir/transform/elaborate_drops.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 28a50547b190c..306fe14266ff9 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -103,7 +103,7 @@ fn find_dead_unwinds<'tcx>( debug!("find_dead_unwinds @ {:?}: path({:?})={:?}", bb, location, path); - flow_inits.seek_after(body.terminator_loc(bb)); + flow_inits.seek_before(body.terminator_loc(bb)); let mut maybe_live = false; on_all_drop_children_bits(tcx, body, &env, path, |child| { maybe_live |= flow_inits.contains(child); @@ -124,9 +124,9 @@ struct InitializationData<'mir, 'tcx> { } impl InitializationData<'_, '_> { - fn seek_after(&mut self, loc: Location) { - self.inits.seek_after(loc); - self.uninits.seek_after(loc); + fn seek_before(&mut self, loc: Location) { + self.inits.seek_before(loc); + self.uninits.seek_before(loc); } fn state(&self, path: MovePathIndex) -> (bool, bool) { @@ -294,7 +294,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { _ => continue, }; - self.init_data.seek_after(self.body.terminator_loc(bb)); + self.init_data.seek_before(self.body.terminator_loc(bb)); let path = self.move_data().rev_lookup.find(location.as_ref()); debug!("collect_drop_flags: {:?}, place {:?} ({:?})", bb, location, path); @@ -341,7 +341,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { let resume_block = self.patch.resume_block(); match terminator.kind { TerminatorKind::Drop { ref location, target, unwind } => { - self.init_data.seek_after(loc); + self.init_data.seek_before(loc); match self.move_data().rev_lookup.find(location.as_ref()) { LookupResult::Exact(path) => elaborate_drop( &mut Elaborator { ctxt: self }, @@ -424,7 +424,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { match self.move_data().rev_lookup.find(location.as_ref()) { LookupResult::Exact(path) => { debug!("elaborate_drop_and_replace({:?}) - tracked {:?}", terminator, path); - self.init_data.seek_after(loc); + self.init_data.seek_before(loc); elaborate_drop( &mut Elaborator { ctxt: self }, terminator.source_info, From 2aa39350d3dde13f5e82f24d42a51cbfe92f723e Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 13 Feb 2020 21:30:10 -0800 Subject: [PATCH 3/4] Use more descriptive name to get `InitializationData` state --- src/librustc_mir/transform/elaborate_drops.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 306fe14266ff9..a4531e53ce178 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -129,7 +129,7 @@ impl InitializationData<'_, '_> { self.uninits.seek_before(loc); } - fn state(&self, path: MovePathIndex) -> (bool, bool) { + fn maybe_live_dead(&self, path: MovePathIndex) -> (bool, bool) { (self.inits.contains(path), self.uninits.contains(path)) } } @@ -165,13 +165,13 @@ impl<'a, 'b, 'tcx> DropElaborator<'a, 'tcx> for Elaborator<'a, 'b, 'tcx> { fn drop_style(&self, path: Self::Path, mode: DropFlagMode) -> DropStyle { let ((maybe_live, maybe_dead), multipart) = match mode { - DropFlagMode::Shallow => (self.ctxt.init_data.state(path), false), + DropFlagMode::Shallow => (self.ctxt.init_data.maybe_live_dead(path), false), DropFlagMode::Deep => { let mut some_live = false; let mut some_dead = false; let mut children_count = 0; on_all_drop_children_bits(self.tcx(), self.body(), self.ctxt.env, path, |child| { - let (live, dead) = self.ctxt.init_data.state(child); + let (live, dead) = self.ctxt.init_data.maybe_live_dead(child); debug!("elaborate_drop: state({:?}) = {:?}", child, (live, dead)); some_live |= live; some_dead |= dead; @@ -303,7 +303,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { LookupResult::Exact(e) => e, LookupResult::Parent(None) => continue, LookupResult::Parent(Some(parent)) => { - let (_maybe_live, maybe_dead) = self.init_data.state(parent); + let (_maybe_live, maybe_dead) = self.init_data.maybe_live_dead(parent); if maybe_dead { span_bug!( terminator.source_info.span, @@ -318,7 +318,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { }; on_all_drop_children_bits(self.tcx, self.body, self.env, path, |child| { - let (maybe_live, maybe_dead) = self.init_data.state(child); + let (maybe_live, maybe_dead) = self.init_data.maybe_live_dead(child); debug!( "collect_drop_flags: collecting {:?} from {:?}@{:?} - {:?}", child, From 26451d007e7482f0c4cdae152a972f4e7c8bf970 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 13 Feb 2020 21:34:19 -0800 Subject: [PATCH 4/4] Print flow state in debug messages for `find_dead_unwinds` --- src/librustc_mir/transform/elaborate_drops.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index a4531e53ce178..5d02074aaaa6b 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -91,7 +91,7 @@ fn find_dead_unwinds<'tcx>( _ => continue, }; - debug!("find_dead_unwinds @ {:?}: {:?}; init_data={:?}", bb, bb_data, flow_inits.get()); + debug!("find_dead_unwinds @ {:?}: {:?}", bb, bb_data); let path = match env.move_data.rev_lookup.find(location.as_ref()) { LookupResult::Exact(e) => e, @@ -101,9 +101,15 @@ fn find_dead_unwinds<'tcx>( } }; - debug!("find_dead_unwinds @ {:?}: path({:?})={:?}", bb, location, path); - flow_inits.seek_before(body.terminator_loc(bb)); + debug!( + "find_dead_unwinds @ {:?}: path({:?})={:?}; init_data={:?}", + bb, + location, + path, + flow_inits.get() + ); + let mut maybe_live = false; on_all_drop_children_bits(tcx, body, &env, path, |child| { maybe_live |= flow_inits.contains(child);