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

MIR pass to remove unneeded drops on types not needing drop #76673

Merged
merged 7 commits into from
Sep 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions compiler/rustc_mir/src/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub mod nrvo;
pub mod promote_consts;
pub mod qualify_min_const_fn;
pub mod remove_noop_landing_pads;
pub mod remove_unneeded_drops;
pub mod required_consts;
pub mod rustc_peek;
pub mod simplify;
Expand Down Expand Up @@ -461,6 +462,7 @@ fn run_optimization_passes<'tcx>(

// The main optimizations that we do on MIR.
let optimizations: &[&dyn MirPass<'tcx>] = &[
&remove_unneeded_drops::RemoveUnneededDrops,
&match_branches::MatchBranchSimplification,
// inst combine is after MatchBranchSimplification to clean up Ne(_1, false)
&instcombine::InstCombine,
Expand Down
59 changes: 59 additions & 0 deletions compiler/rustc_mir/src/transform/remove_unneeded_drops.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//! This pass replaces a drop of a type that does not need dropping, with a goto

use crate::transform::{MirPass, MirSource};
use rustc_hir::def_id::LocalDefId;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;

use super::simplify::simplify_cfg;

pub struct RemoveUnneededDrops;

impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops {
fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) {
trace!("Running RemoveUnneededDrops on {:?}", source);
let mut opt_finder = RemoveUnneededDropsOptimizationFinder {
tcx,
body,
optimizations: vec![],
def_id: source.def_id().expect_local(),
};
opt_finder.visit_body(body);
let should_simplify = !opt_finder.optimizations.is_empty();
for (loc, target) in opt_finder.optimizations {
let terminator = body.basic_blocks_mut()[loc.block].terminator_mut();
debug!("SUCCESS: replacing `drop` with goto({:?})", target);
terminator.kind = TerminatorKind::Goto { target };
}
simonvandel marked this conversation as resolved.
Show resolved Hide resolved

// if we applied optimizations, we potentially have some cfg to cleanup to
// make it easier for further passes
if should_simplify {
simplify_cfg(body);
}
}
}

impl<'a, 'tcx> Visitor<'tcx> for RemoveUnneededDropsOptimizationFinder<'a, 'tcx> {
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
match terminator.kind {
TerminatorKind::Drop { place, target, .. }
| TerminatorKind::DropAndReplace { place, target, .. } => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DropAndReplace terminator should never occur in an input to this pass, since it should be removed during drop elaboration. Additionally, its semantics is to drop and assign a new value, so the transformation wouldn't be correct without additional assignment. Either way, I would just remove it from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tmiasko, thanks! See PR #77165

let ty = place.ty(self.body, self.tcx);
let needs_drop = ty.ty.needs_drop(self.tcx, self.tcx.param_env(self.def_id));
if !needs_drop {
self.optimizations.push((location, target));
}
}
_ => {}
}
self.super_terminator(terminator, location);
}
}
pub struct RemoveUnneededDropsOptimizationFinder<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
body: &'a Body<'tcx>,
optimizations: Vec<(Location, BasicBlock)>,
def_id: LocalDefId,
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@
- _6 = _1; // scope 3 at $DIR/cycle.rs:14:10: 14:11
+ _6 = _4; // scope 3 at $DIR/cycle.rs:14:10: 14:11
_5 = const (); // scope 4 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
drop(_6) -> bb2; // scope 4 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb2: {
StorageDead(_6); // scope 3 at $DIR/cycle.rs:14:11: 14:12
StorageDead(_5); // scope 3 at $DIR/cycle.rs:14:12: 14:13
_0 = const (); // scope 0 at $DIR/cycle.rs:8:11: 15:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@
StorageLive(_4); // scope 1 at $DIR/union.rs:15:10: 15:26
_4 = (_1.0: u32); // scope 2 at $DIR/union.rs:15:19: 15:24
_3 = const (); // scope 3 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
drop(_4) -> bb2; // scope 3 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb2: {
StorageDead(_4); // scope 1 at $DIR/union.rs:15:26: 15:27
StorageDead(_3); // scope 1 at $DIR/union.rs:15:27: 15:28
_0 = const (); // scope 0 at $DIR/union.rs:8:11: 16:2
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
- // MIR for `cannot_opt_generic` before RemoveUnneededDrops
+ // MIR for `cannot_opt_generic` after RemoveUnneededDrops

fn cannot_opt_generic(_1: T) -> () {
debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:20:26: 20:27
let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:20:32: 20:32
let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
let mut _3: T; // in scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
scope 1 {
debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
_3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
_2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb1 (cleanup): {
resume; // scope 0 at $DIR/remove_unneeded_drops.rs:20:1: 22:2
}

bb2: {
StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:21:11: 21:12
StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:21:12: 21:13
_0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:20:32: 22:2
return; // scope 0 at $DIR/remove_unneeded_drops.rs:22:2: 22:2
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
- // MIR for `dont_opt` before RemoveUnneededDrops
+ // MIR for `dont_opt` after RemoveUnneededDrops

fn dont_opt(_1: Vec<bool>) -> () {
debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:8:13: 8:14
let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:8:27: 8:27
let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
let mut _3: std::vec::Vec<bool>; // in scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
scope 1 {
debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
_3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
_2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb1 (cleanup): {
resume; // scope 0 at $DIR/remove_unneeded_drops.rs:8:1: 10:2
}

bb2: {
StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:9:11: 9:12
StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:9:12: 9:13
_0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:8:27: 10:2
return; // scope 0 at $DIR/remove_unneeded_drops.rs:10:2: 10:2
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
- // MIR for `opt` before RemoveUnneededDrops
+ // MIR for `opt` after RemoveUnneededDrops

fn opt(_1: bool) -> () {
debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:3:8: 3:9
let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:3:17: 3:17
let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
let mut _3: bool; // in scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
scope 1 {
debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
_3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
_2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
- drop(_3) -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
- }
-
- bb1: {
StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:4:11: 4:12
StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:4:12: 4:13
_0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:3:17: 5:2
return; // scope 0 at $DIR/remove_unneeded_drops.rs:5:2: 5:2
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
- // MIR for `opt_generic_copy` before RemoveUnneededDrops
+ // MIR for `opt_generic_copy` after RemoveUnneededDrops

fn opt_generic_copy(_1: T) -> () {
debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:13:30: 13:31
let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:13:36: 13:36
let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
let mut _3: T; // in scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
scope 1 {
debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
_3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
_2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
- drop(_3) -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL
- }
-
- bb1: {
StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:14:11: 14:12
StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:14:12: 14:13
_0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:13:36: 15:2
return; // scope 0 at $DIR/remove_unneeded_drops.rs:15:2: 15:2
}
}

29 changes: 29 additions & 0 deletions src/test/mir-opt/remove_unneeded_drops.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// ignore-wasm32-bare compiled with panic=abort by default
// EMIT_MIR remove_unneeded_drops.opt.RemoveUnneededDrops.diff
fn opt(x: bool) {
drop(x);
}

// EMIT_MIR remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff
fn dont_opt(x: Vec<bool>) {
drop(x);
}

// EMIT_MIR remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff
fn opt_generic_copy<T: Copy>(x: T) {
drop(x);
}

// EMIT_MIR remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff
// since the pass is not running on monomorphisized code,
// we can't (but probably should) optimize this
fn cannot_opt_generic<T>(x: T) {
drop(x);
}

fn main() {
opt(true);
opt_generic_copy(42);
cannot_opt_generic(42);
dont_opt(vec![true]);
}