Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove dead unwinds before drop elaboration #106430

Merged
merged 3 commits into from
Feb 25, 2023
Merged

Remove dead unwinds before drop elaboration #106430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00eec85
Emit diff instead of after mir in ElaborateDrops tests
tmiasko Feb 23, 2023
23e3840
Remove dead unwinds before drop elaboration
tmiasko Jan 5, 2023
2a70524
./x.py test --bless
tmiasko Feb 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 3 additions & 28 deletions compiler/rustc_mir_dataflow/src/framework/direction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::{self, BasicBlock, Location, SwitchTargets};
use rustc_middle::ty::TyCtxt;
use std::ops::RangeInclusive;
Expand Down Expand Up @@ -54,7 +53,6 @@ pub trait Direction {
analysis: &A,
tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>,
dead_unwinds: Option<&BitSet<BasicBlock>>,
exit_state: &mut A::Domain,
block: (BasicBlock, &'_ mir::BasicBlockData<'tcx>),
propagate: impl FnMut(BasicBlock, &A::Domain),
Expand Down Expand Up @@ -221,7 +219,6 @@ impl Direction for Backward {
analysis: &A,
_tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>,
dead_unwinds: Option<&BitSet<BasicBlock>>,
exit_state: &mut A::Domain,
(bb, _bb_data): (BasicBlock, &'_ mir::BasicBlockData<'tcx>),
mut propagate: impl FnMut(BasicBlock, &A::Domain),
Expand Down Expand Up @@ -278,20 +275,6 @@ impl Direction for Backward {
}
}

// Ignore dead unwinds.
mir::TerminatorKind::Call { cleanup: Some(unwind), .. }
| mir::TerminatorKind::Assert { cleanup: Some(unwind), .. }
| mir::TerminatorKind::Drop { unwind: Some(unwind), .. }
| mir::TerminatorKind::DropAndReplace { unwind: Some(unwind), .. }
| mir::TerminatorKind::FalseUnwind { unwind: Some(unwind), .. }
| mir::TerminatorKind::InlineAsm { cleanup: Some(unwind), .. }
if unwind == bb =>
{
if dead_unwinds.map_or(true, |dead| !dead.contains(pred)) {
propagate(pred, exit_state);
}
}

_ => propagate(pred, exit_state),
}
}
Expand All @@ -304,7 +287,6 @@ struct BackwardSwitchIntEdgeEffectsApplier<'a, 'tcx, D, F> {
exit_state: &'a mut D,
bb: BasicBlock,
propagate: &'a mut F,

effects_applied: bool,
}

Expand Down Expand Up @@ -484,7 +466,6 @@ impl Direction for Forward {
analysis: &A,
_tcx: TyCtxt<'tcx>,
_body: &mir::Body<'tcx>,
dead_unwinds: Option<&BitSet<BasicBlock>>,
exit_state: &mut A::Domain,
(bb, bb_data): (BasicBlock, &'_ mir::BasicBlockData<'tcx>),
mut propagate: impl FnMut(BasicBlock, &A::Domain),
Expand All @@ -502,9 +483,7 @@ impl Direction for Forward {
| DropAndReplace { target, unwind, value: _, place: _ }
| FalseUnwind { real_target: target, unwind } => {
if let Some(unwind) = unwind {
if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) {
propagate(unwind, exit_state);
}
propagate(unwind, exit_state);
}

propagate(target, exit_state);
Expand Down Expand Up @@ -534,9 +513,7 @@ impl Direction for Forward {
fn_span: _,
} => {
if let Some(unwind) = cleanup {
if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) {
propagate(unwind, exit_state);
}
propagate(unwind, exit_state);
}

if let Some(target) = target {
Expand All @@ -560,9 +537,7 @@ impl Direction for Forward {
cleanup,
} => {
if let Some(unwind) = cleanup {
if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) {
propagate(unwind, exit_state);
}
propagate(unwind, exit_state);
}

if let Some(target) = destination {
Expand Down
32 changes: 2 additions & 30 deletions compiler/rustc_mir_dataflow/src/framework/engine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use rustc_ast as ast;
use rustc_data_structures::work_queue::WorkQueue;
use rustc_graphviz as dot;
use rustc_hir::def_id::DefId;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::{self, traversal, BasicBlock};
use rustc_middle::mir::{create_dump_file, dump_enabled};
Expand Down Expand Up @@ -78,7 +77,6 @@ where
{
tcx: TyCtxt<'tcx>,
body: &'a mir::Body<'tcx>,
dead_unwinds: Option<&'a BitSet<BasicBlock>>,
entry_sets: IndexVec<BasicBlock, A::Domain>,
pass_name: Option<&'static str>,
analysis: A,
Expand Down Expand Up @@ -154,25 +152,7 @@ where
bug!("`initialize_start_block` is not yet supported for backward dataflow analyses");
}

Engine {
analysis,
tcx,
body,
dead_unwinds: None,
pass_name: None,
entry_sets,
apply_trans_for_block,
}
}

/// Signals that we do not want dataflow state to propagate across unwind edges for these
/// `BasicBlock`s.
///
/// You must take care that `dead_unwinds` does not contain a `BasicBlock` that *can* actually
/// unwind during execution. Otherwise, your dataflow results will not be correct.
pub fn dead_unwinds(mut self, dead_unwinds: &'a BitSet<BasicBlock>) -> Self {
self.dead_unwinds = Some(dead_unwinds);
self
Engine { analysis, tcx, body, pass_name: None, entry_sets, apply_trans_for_block }
}

/// Adds an identifier to the graphviz output for this particular run of a dataflow analysis.
Expand All @@ -190,14 +170,7 @@ where
A::Domain: DebugWithContext<A>,
{
let Engine {
analysis,
body,
dead_unwinds,
mut entry_sets,
tcx,
apply_trans_for_block,
pass_name,
..
analysis, body, mut entry_sets, tcx, apply_trans_for_block, pass_name, ..
} = self;

let mut dirty_queue: WorkQueue<BasicBlock> = WorkQueue::with_none(body.basic_blocks.len());
Expand Down Expand Up @@ -236,7 +209,6 @@ where
&analysis,
tcx,
body,
dead_unwinds,
&mut state,
(bb, bb_data),
|target: BasicBlock, state: &A::Domain| {
Expand Down
57 changes: 39 additions & 18 deletions compiler/rustc_mir_transform/src/elaborate_drops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,25 +67,24 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
};
let un_derefer = UnDerefer { tcx: tcx, derefer_sidetable: side_table };
let elaborate_patch = {
let body = &*body;
let env = MoveDataParamEnv { move_data, param_env };
let dead_unwinds = find_dead_unwinds(tcx, body, &env, &un_derefer);
remove_dead_unwinds(tcx, body, &env, &un_derefer);

let inits = MaybeInitializedPlaces::new(tcx, body, &env)
.into_engine(tcx, body)
.dead_unwinds(&dead_unwinds)
.pass_name("elaborate_drops")
.iterate_to_fixpoint()
.into_results_cursor(body);

let uninits = MaybeUninitializedPlaces::new(tcx, body, &env)
.mark_inactive_variants_as_uninit()
.into_engine(tcx, body)
.dead_unwinds(&dead_unwinds)
.pass_name("elaborate_drops")
.iterate_to_fixpoint()
.into_results_cursor(body);

let reachable = traversal::reachable_as_bitset(body);
tmiasko marked this conversation as resolved.
Show resolved Hide resolved

ElaborateDropsCtxt {
tcx,
body,
Expand All @@ -94,6 +93,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
drop_flags: Default::default(),
patch: MirPatch::new(body),
un_derefer: un_derefer,
reachable,
}
.elaborate()
};
Expand All @@ -102,22 +102,21 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
}
}

/// Returns the set of basic blocks whose unwind edges are known
/// to not be reachable, because they are `drop` terminators
/// Removes unwind edges which are known to be unreachable, because they are in `drop` terminators
/// that can't drop anything.
fn find_dead_unwinds<'tcx>(
fn remove_dead_unwinds<'tcx>(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
body: &mut Body<'tcx>,
env: &MoveDataParamEnv<'tcx>,
und: &UnDerefer<'tcx>,
) -> BitSet<BasicBlock> {
debug!("find_dead_unwinds({:?})", body.span);
) {
debug!("remove_dead_unwinds({:?})", body.span);
// 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 mut dead_unwinds = Vec::new();
let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &env)
.into_engine(tcx, body)
.pass_name("find_dead_unwinds")
.pass_name("remove_dead_unwinds")
.iterate_to_fixpoint()
.into_results_cursor(body);
for (bb, bb_data) in body.basic_blocks.iter_enumerated() {
Expand All @@ -129,16 +128,16 @@ fn find_dead_unwinds<'tcx>(
_ => continue,
};

debug!("find_dead_unwinds @ {:?}: {:?}", bb, bb_data);
debug!("remove_dead_unwinds @ {:?}: {:?}", bb, bb_data);

let LookupResult::Exact(path) = env.move_data.rev_lookup.find(place.as_ref()) else {
debug!("find_dead_unwinds: has parent; skipping");
debug!("remove_dead_unwinds: has parent; skipping");
continue;
};

flow_inits.seek_before_primary_effect(body.terminator_loc(bb));
debug!(
"find_dead_unwinds @ {:?}: path({:?})={:?}; init_data={:?}",
"remove_dead_unwinds @ {:?}: path({:?})={:?}; init_data={:?}",
bb,
place,
path,
Expand All @@ -150,13 +149,22 @@ fn find_dead_unwinds<'tcx>(
maybe_live |= flow_inits.contains(child);
});

debug!("find_dead_unwinds @ {:?}: maybe_live={}", bb, maybe_live);
debug!("remove_dead_unwinds @ {:?}: maybe_live={}", bb, maybe_live);
if !maybe_live {
dead_unwinds.insert(bb);
dead_unwinds.push(bb);
}
}

dead_unwinds
if dead_unwinds.is_empty() {
return;
}

let basic_blocks = body.basic_blocks.as_mut();
for &bb in dead_unwinds.iter() {
if let Some(unwind) = basic_blocks[bb].terminator_mut().unwind_mut() {
*unwind = None;
}
}
}

struct InitializationData<'mir, 'tcx> {
Expand Down Expand Up @@ -290,6 +298,7 @@ struct ElaborateDropsCtxt<'a, 'tcx> {
drop_flags: FxHashMap<MovePathIndex, Local>,
patch: MirPatch<'tcx>,
un_derefer: UnDerefer<'tcx>,
reachable: BitSet<BasicBlock>,
}

impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
Expand Down Expand Up @@ -329,6 +338,9 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {

fn collect_drop_flags(&mut self) {
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
let terminator = data.terminator();
let place = match terminator.kind {
TerminatorKind::Drop { ref place, .. }
Expand Down Expand Up @@ -384,6 +396,9 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {

fn elaborate_drops(&mut self) {
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
let loc = Location { block: bb, statement_index: data.statements.len() };
let terminator = data.terminator();

Expand Down Expand Up @@ -541,6 +556,9 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {

fn drop_flags_for_fn_rets(&mut self) {
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
if let TerminatorKind::Call {
destination, target: Some(tgt), cleanup: Some(_), ..
} = data.terminator().kind
Expand Down Expand Up @@ -576,6 +594,9 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
// clobbered before they are read.

for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
debug!("drop_flags_for_locs({:?})", data);
for i in 0..(data.statements.len() + 1) {
debug!("drop_flag_for_locs: stmt {}", i);
Expand Down
70 changes: 0 additions & 70 deletions tests/mir-opt/issue_41110.main.ElaborateDrops.after.mir
View file
Open in desktop

This file was deleted.

Loading