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.

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

[PERF] further drop changes #66703

Closed
wants to merge 16 commits into from
Closed
5 changes: 1 addition & 4 deletions src/librustc_mir/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,17 +361,14 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
fn gather_terminator(&mut self, term: &Terminator<'tcx>) {
match term.kind {
TerminatorKind::Goto { target: _ }
| TerminatorKind::Return
| TerminatorKind::Resume
| TerminatorKind::Abort
| TerminatorKind::GeneratorDrop
| TerminatorKind::FalseEdges { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::Unreachable => {}

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

TerminatorKind::Assert { ref cond, .. } => {
self.gather_operand(cond);
}
Expand Down
50 changes: 12 additions & 38 deletions src/librustc_mir/util/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,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 @@ -175,9 +173,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 @@ -249,7 +245,7 @@ where
// our own drop flag.
path: self.path,
}
.complete_drop(None, succ, unwind)
.complete_drop(succ, unwind)
}
}

Expand Down Expand Up @@ -280,13 +276,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 @@ -813,11 +803,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 @@ -829,26 +815,14 @@ where
}

/// Returns a basic block that drop a place using the context
/// and path in `c`. If `mode` is something, also clear `c`
/// according to it.
/// and path in `c`.
///
/// if FLAG(self.path)
/// if let Some(mode) = mode: FLAG(self.path)[mode] = false
/// drop(self.place)
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 @@ -861,6 +835,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: block, statement_index: 0 };
self.elaborator.clear_drop_flag(block_start, self.path, mode);
Expand Down Expand Up @@ -964,11 +943,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
11 changes: 8 additions & 3 deletions src/librustc_mir/util/graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,17 @@ where
write!(w, r#"<table border="0" cellborder="1" cellspacing="0">"#)?;

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

init(w)?;
Expand Down
26 changes: 16 additions & 10 deletions src/librustc_mir_build/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::build::matches::ArmHasGuard;
use crate::build::ForGuard::OutsideGuard;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::hair::*;
use rustc::middle::region;
use rustc::mir::*;
use rustc_hir as hir;
use rustc_span::Span;
Expand All @@ -10,6 +11,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
crate fn ast_block(
&mut self,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
ast_block: &'tcx hir::Block<'tcx>,
source_info: SourceInfo,
Expand All @@ -26,16 +28,19 @@ 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.clone(), |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.clone(), scope, span, |this| {
Some(this.ast_block_stmts(
destination,
scope,
block,
span,
stmts,
expr,
safety_mode,
))
})
} else {
this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
this.ast_block_stmts(destination, scope, block, span, stmts, expr, safety_mode)
}
})
})
Expand All @@ -44,6 +49,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn ast_block_stmts(
&mut self,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
mut block: BasicBlock,
span: Span,
stmts: Vec<StmtRef<'tcx>>,
Expand Down Expand Up @@ -175,7 +181,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
destination_ty.is_unit() || this.block_context.currently_ignores_tail_results();
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored });

unpack!(block = this.into(destination, block, expr));
unpack!(block = this.into(destination, scope, block, expr));
let popped = this.block_context.pop();

assert!(popped.map_or(false, |bf| bf.is_tail_expr()));
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_mir_build/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty);
this.cfg.push_assign(block, source_info, &Place::from(result), box_);

// initialize the box contents:
// Initialize the box contents. No scope is needed since the
// `Box` is already scheduled to be dropped.
unpack!(
block = this.into(
&this.hir.tcx().mk_place_deref(Place::from(result)),
None,
block,
value
value,
)
);
block.and(Rvalue::Use(Operand::Move(Place::from(result))))
Expand Down
6 changes: 1 addition & 5 deletions src/librustc_mir_build/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

unpack!(block = this.into(temp_place, block, expr));

if let Some(temp_lifetime) = temp_lifetime {
this.schedule_drop(expr_span, temp_lifetime, temp, DropKind::Value);
}
unpack!(block = this.into(temp_place, temp_lifetime, block, expr));

block.and(temp)
}
Expand Down
Loading