Skip to content

Commit

Permalink
Auto merge of #77466 - Aaron1011:reland-drop-tree, r=matthewjasper
Browse files Browse the repository at this point in the history
Re-land PR #71840 (Rework MIR drop tree lowering)

PR #71840 was reverted in #72989 to fix an LLVM error (#72470). That LLVM error no longer occurs with the recent upgrade to LLVM 11 (#73526), so let's try re-landing this PR.

I've cherry-picked the commits from the original PR (with the exception of the commit blessing test output), making as few modifications as possible. I addressed the rebase fallout in separate commits on top of those.

r? `@matthewjasper`
  • Loading branch information
bors committed Oct 5, 2020
2 parents beb5ae4 + ce8d757 commit ced813f
Show file tree
Hide file tree
Showing 81 changed files with 2,504 additions and 2,748 deletions.
13 changes: 7 additions & 6 deletions compiler/rustc_mir/src/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,18 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
fn gather_terminator(&mut self, term: &Terminator<'tcx>) {
match term.kind {
TerminatorKind::Goto { target: _ }
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
// In some sense returning moves the return place into the current
// call's destination, however, since there are no statements after
// this that could possibly access the return place, this doesn't
// need recording.
| TerminatorKind::Return
| TerminatorKind::Resume
| TerminatorKind::Abort
| TerminatorKind::GeneratorDrop
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::Unreachable => {}

TerminatorKind::Return => {
self.gather_move(Place::return_place());
}

TerminatorKind::Assert { ref cond, .. } => {
self.gather_operand(cond);
}
Expand Down
46 changes: 11 additions & 35 deletions compiler/rustc_mir/src/util/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,6 @@ where
.patch_terminator(bb, TerminatorKind::Goto { target: self.succ });
}
DropStyle::Static => {
let loc = self.terminator_loc(bb);
self.elaborator.clear_drop_flag(loc, self.path, DropFlagMode::Deep);
self.elaborator.patch().patch_terminator(
bb,
TerminatorKind::Drop {
Expand All @@ -243,9 +241,7 @@ where
);
}
DropStyle::Conditional => {
let unwind = self.unwind; // FIXME(#43234)
let succ = self.succ;
let drop_bb = self.complete_drop(Some(DropFlagMode::Deep), succ, unwind);
let drop_bb = self.complete_drop(self.succ, self.unwind);
self.elaborator
.patch()
.patch_terminator(bb, TerminatorKind::Goto { target: drop_bb });
Expand Down Expand Up @@ -317,7 +313,7 @@ where
// our own drop flag.
path: self.path,
}
.complete_drop(None, succ, unwind)
.complete_drop(succ, unwind)
}
}

Expand Down Expand Up @@ -346,13 +342,7 @@ where
// Clear the "master" drop flag at the end. This is needed
// because the "master" drop protects the ADT's discriminant,
// which is invalidated after the ADT is dropped.
let (succ, unwind) = (self.succ, self.unwind); // FIXME(#43234)
(
self.drop_flag_reset_block(DropFlagMode::Shallow, succ, unwind),
unwind.map(|unwind| {
self.drop_flag_reset_block(DropFlagMode::Shallow, unwind, Unwind::InCleanup)
}),
)
(self.drop_flag_reset_block(DropFlagMode::Shallow, self.succ, self.unwind), self.unwind)
}

/// Creates a full drop ladder, consisting of 2 connected half-drop-ladders
Expand Down Expand Up @@ -884,11 +874,7 @@ where
self.open_drop_for_adt(def, substs)
}
}
ty::Dynamic(..) => {
let unwind = self.unwind; // FIXME(#43234)
let succ = self.succ;
self.complete_drop(Some(DropFlagMode::Deep), succ, unwind)
}
ty::Dynamic(..) => self.complete_drop(self.succ, self.unwind),
ty::Array(ety, size) => {
let size = size.try_eval_usize(self.tcx(), self.elaborator.param_env());
self.open_drop_for_array(ety, size)
Expand All @@ -899,20 +885,10 @@ where
}
}

fn complete_drop(
&mut self,
drop_mode: Option<DropFlagMode>,
succ: BasicBlock,
unwind: Unwind,
) -> BasicBlock {
debug!("complete_drop({:?},{:?})", self, drop_mode);
fn complete_drop(&mut self, succ: BasicBlock, unwind: Unwind) -> BasicBlock {
debug!("complete_drop(succ={:?}, unwind={:?})", succ, unwind);

let drop_block = self.drop_block(succ, unwind);
let drop_block = if let Some(mode) = drop_mode {
self.drop_flag_reset_block(mode, drop_block, unwind)
} else {
drop_block
};

self.drop_flag_test_block(drop_block, succ, unwind)
}
Expand All @@ -927,6 +903,11 @@ where
) -> BasicBlock {
debug!("drop_flag_reset_block({:?},{:?})", self, mode);

if unwind.is_cleanup() {
// The drop flag isn't read again on the unwind path, so don't
// bother setting it.
return succ;
}
let block = self.new_block(unwind, TerminatorKind::Goto { target: succ });
let block_start = Location { block, statement_index: 0 };
self.elaborator.clear_drop_flag(block_start, self.path, mode);
Expand Down Expand Up @@ -1044,11 +1025,6 @@ where
self.elaborator.patch().new_temp(ty, self.source_info.span)
}

fn terminator_loc(&mut self, bb: BasicBlock) -> Location {
let body = self.elaborator.body();
self.elaborator.patch().terminator_loc(body, bb)
}

fn constant_usize(&self, val: u16) -> Operand<'tcx> {
Operand::Constant(box Constant {
span: self.source_info.span,
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_mir/src/util/graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,19 @@ where
write!(w, r#"<table border="0" cellborder="1" cellspacing="0">"#)?;

// Basic block number at the top.
let (blk, bgcolor) = if data.is_cleanup {
(format!("{} (cleanup)", block.index()), "lightblue")
} else {
let color = if dark_mode { "dimgray" } else { "gray" };
(format!("{}", block.index()), color)
};
write!(
w,
r#"<tr><td bgcolor="{bgcolor}" {attrs} colspan="{colspan}">{blk}</td></tr>"#,
bgcolor = if dark_mode { "dimgray" } else { "gray" },
attrs = r#"align="center""#,
colspan = num_cols,
blk = block.index()
blk = blk,
bgcolor = bgcolor
)?;

init(w)?;
Expand Down
18 changes: 10 additions & 8 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.in_opt_scope(opt_destruction_scope.map(|de| (de, source_info)), move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
// This is a `break`-able block
let exit_block = this.cfg.start_new_block();
let block_exit =
this.in_breakable_scope(None, exit_block, destination, |this| {
this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
});
this.cfg.goto(unpack!(block_exit), source_info, exit_block);
exit_block.unit()
this.in_breakable_scope(None, destination, span, |this| {
Some(this.ast_block_stmts(
destination,
block,
span,
stmts,
expr,
safety_mode,
))
})
} else {
this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
}
Expand Down
24 changes: 11 additions & 13 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,32 +140,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// body, even when the exact code in the body cannot unwind

let loop_block = this.cfg.start_new_block();
let exit_block = this.cfg.start_new_block();

// Start the loop.
this.cfg.goto(block, source_info, loop_block);

this.in_breakable_scope(Some(loop_block), exit_block, destination, move |this| {
this.in_breakable_scope(Some(loop_block), destination, expr_span, move |this| {
// conduct the test, if necessary
let body_block = this.cfg.start_new_block();
let diverge_cleanup = this.diverge_cleanup();
this.cfg.terminate(
loop_block,
source_info,
TerminatorKind::FalseUnwind {
real_target: body_block,
unwind: Some(diverge_cleanup),
},
TerminatorKind::FalseUnwind { real_target: body_block, unwind: None },
);
this.diverge_from(loop_block);

// The “return” value of the loop body must always be an unit. We therefore
// introduce a unit temporary as the destination for the loop body.
let tmp = this.get_unit_temp();
// Execute the body, branching back to the test.
let body_block_end = unpack!(this.into(tmp, body_block, body));
this.cfg.goto(body_block_end, source_info, loop_block);
});
exit_block.unit()

// Loops are only exited by `break` expressions.
None
})
}
ExprKind::Call { ty, fun, args, from_hir_call, fn_span } => {
let intrinsic = match *ty.kind() {
Expand Down Expand Up @@ -206,7 +204,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.collect();

let success = this.cfg.start_new_block();
let cleanup = this.diverge_cleanup();

this.record_operands_moved(&args);

Expand All @@ -218,7 +215,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
TerminatorKind::Call {
func: fun,
args,
cleanup: Some(cleanup),
cleanup: None,
// FIXME(varkor): replace this with an uninhabitedness-based check.
// This requires getting access to the current module to call
// `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
Expand All @@ -231,6 +228,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn_span,
},
);
this.diverge_from(block);
success.unit()
}
}
Expand Down Expand Up @@ -437,12 +435,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let scope = this.local_scope();
let value = unpack!(block = this.as_operand(block, scope, value));
let resume = this.cfg.start_new_block();
let cleanup = this.generator_drop_cleanup();
this.cfg.terminate(
block,
source_info,
TerminatorKind::Yield { value, resume, resume_arg: destination, drop: cleanup },
TerminatorKind::Yield { value, resume, resume_arg: destination, drop: None },
);
this.generator_drop_cleanup(block);
resume.unit()
}

Expand Down
17 changes: 5 additions & 12 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
outer_source_info: SourceInfo,
fake_borrow_temps: Vec<(Place<'tcx>, Local)>,
) -> BlockAnd<()> {
let match_scope = self.scopes.topmost();

let arm_end_blocks: Vec<_> = arm_candidates
.into_iter()
.map(|(arm, candidate)| {
Expand All @@ -250,7 +248,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let arm_block = this.bind_pattern(
outer_source_info,
candidate,
arm.guard.as_ref().map(|g| (g, match_scope)),
arm.guard.as_ref(),
&fake_borrow_temps,
scrutinee_span,
Some(arm.scope),
Expand Down Expand Up @@ -287,7 +285,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut self,
outer_source_info: SourceInfo,
candidate: Candidate<'_, 'tcx>,
guard: Option<(&Guard<'tcx>, region::Scope)>,
guard: Option<&Guard<'tcx>>,
fake_borrow_temps: &Vec<(Place<'tcx>, Local)>,
scrutinee_span: Span,
arm_scope: Option<region::Scope>,
Expand Down Expand Up @@ -1592,7 +1590,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut self,
candidate: Candidate<'pat, 'tcx>,
parent_bindings: &[(Vec<Binding<'tcx>>, Vec<Ascription<'tcx>>)],
guard: Option<(&Guard<'tcx>, region::Scope)>,
guard: Option<&Guard<'tcx>>,
fake_borrows: &Vec<(Place<'tcx>, Local)>,
scrutinee_span: Span,
schedule_drops: bool,
Expand Down Expand Up @@ -1704,7 +1702,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// the reference that we create for the arm.
// * So we eagerly create the reference for the arm and then take a
// reference to that.
if let Some((guard, region_scope)) = guard {
if let Some(guard) = guard {
let tcx = self.hir.tcx();
let bindings = parent_bindings
.iter()
Expand Down Expand Up @@ -1748,12 +1746,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
unreachable
});
let outside_scope = self.cfg.start_new_block();
self.exit_scope(
source_info.span,
region_scope,
otherwise_post_guard_block,
outside_scope,
);
self.exit_top_scope(otherwise_post_guard_block, outside_scope, source_info);
self.false_edges(
outside_scope,
otherwise_block,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let bool_ty = self.hir.bool_ty();
let eq_result = self.temp(bool_ty, source_info.span);
let eq_block = self.cfg.start_new_block();
let cleanup = self.diverge_cleanup();
self.cfg.terminate(
block,
source_info,
Expand All @@ -436,11 +435,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}),
args: vec![val, expect],
destination: Some((eq_result, eq_block)),
cleanup: Some(cleanup),
cleanup: None,
from_hir_call: false,
fn_span: source_info.span,
},
);
self.diverge_from(block);

if let [success_block, fail_block] = *make_target_blocks(self) {
// check the result
Expand Down
Loading

0 comments on commit ced813f

Please sign in to comment.