Auto merge of #76673 - simonvandel:remove-unneeded-drops, r=oli-obk

MIR pass to remove unneeded drops on types not needing drop This is heavily dependent on MIR inlining running to actually see the drop statement. Do we want to special case replacing a call to std::mem::drop with a goto aswell?
rust-lang · Sep 23, 2020 · 8b40853 · 8b40853
2 parents a6008fa + 05f84c6
commit 8b40853
Show file tree

Hide file tree

Showing 9 changed files with 210 additions and 8 deletions.
diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs
@@ -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;
@@ -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,

diff --git a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs
@@ -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 };
+        }
+
+        // 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, .. } => {
+                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,
+}
diff --git a/src/test/mir-opt/dest-prop/cycle.main.DestinationPropagation.diff b/src/test/mir-opt/dest-prop/cycle.main.DestinationPropagation.diff
@@ -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

diff --git a/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff b/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff
@@ -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

diff --git a/src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff
@@ -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
+      }
+  }
+
diff --git a/src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff
@@ -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
+      }
+  }
+
diff --git a/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff
@@ -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
+      }
+  }
+
diff --git a/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff
@@ -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
+      }
+  }
+
diff --git a/src/test/mir-opt/remove_unneeded_drops.rs b/src/test/mir-opt/remove_unneeded_drops.rs
@@ -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]);
+}