From 8b184ff1b8106bb88d3cf0cc6c54eaa5b30b3a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 13 Jan 2021 00:00:00 +0000 Subject: [PATCH 1/3] Remove storage markers if they won't be used during code generation The storage markers constitute a substantial portion of all MIR statements. At the same time, for builds without any optimizations, the storage markers have no further use during and after MIR optimization phase. If storage markers are not necessary for code generation, remove them. --- compiler/rustc_mir/src/transform/mod.rs | 2 + .../src/transform/remove_storage_markers.rs | 25 ++++ ...age_markers.main.RemoveStorageMarkers.diff | 115 ++++++++++++++++++ src/test/mir-opt/remove_storage_markers.rs | 11 ++ src/test/run-make/const_fn_mir/dump.mir | 2 - 5 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 compiler/rustc_mir/src/transform/remove_storage_markers.rs create mode 100644 src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff create mode 100644 src/test/mir-opt/remove_storage_markers.rs diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs index a79f855965a..cc6b2ff5fd4 100644 --- a/compiler/rustc_mir/src/transform/mod.rs +++ b/compiler/rustc_mir/src/transform/mod.rs @@ -42,6 +42,7 @@ pub mod no_landing_pads; pub mod nrvo; pub mod promote_consts; pub mod remove_noop_landing_pads; +pub mod remove_storage_markers; pub mod remove_unneeded_drops; pub mod required_consts; pub mod rustc_peek; @@ -493,6 +494,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { // The main optimizations that we do on MIR. let optimizations: &[&dyn MirPass<'tcx>] = &[ + &remove_storage_markers::RemoveStorageMarkers, &const_goto::ConstGoto, &remove_unneeded_drops::RemoveUnneededDrops, &match_branches::MatchBranchSimplification, diff --git a/compiler/rustc_mir/src/transform/remove_storage_markers.rs b/compiler/rustc_mir/src/transform/remove_storage_markers.rs new file mode 100644 index 00000000000..2d529feb072 --- /dev/null +++ b/compiler/rustc_mir/src/transform/remove_storage_markers.rs @@ -0,0 +1,25 @@ +//! This pass removes storage markers if they won't be emitted during codegen. + +use crate::transform::MirPass; +use rustc_middle::mir::*; +use rustc_middle::ty::TyCtxt; + +pub struct RemoveStorageMarkers; + +impl<'tcx> MirPass<'tcx> for RemoveStorageMarkers { + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + if tcx.sess.emit_lifetime_markers() { + return; + } + + trace!("Running RemoveStorageMarkers on {:?}", body.source); + for data in body.basic_blocks_mut() { + data.statements.retain(|statement| match statement.kind { + StatementKind::StorageLive(..) + | StatementKind::StorageDead(..) + | StatementKind::Nop => false, + _ => true, + }) + } + } +} diff --git a/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff b/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff new file mode 100644 index 00000000000..dd8a9251042 --- /dev/null +++ b/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff @@ -0,0 +1,115 @@ +- // MIR for `main` before RemoveStorageMarkers ++ // MIR for `main` after RemoveStorageMarkers + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/remove_storage_markers.rs:6:11: 6:11 + let mut _1: i32; // in scope 0 at $DIR/remove_storage_markers.rs:7:9: 7:16 + let mut _2: std::ops::Range; // in scope 0 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _3: std::ops::Range; // in scope 0 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _5: (); // in scope 0 at $DIR/remove_storage_markers.rs:6:1: 11:2 + let _7: (); // in scope 0 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _8: std::option::Option; // in scope 0 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _9: &mut std::ops::Range; // in scope 0 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _10: &mut std::ops::Range; // in scope 0 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _11: isize; // in scope 0 at $DIR/remove_storage_markers.rs:8:9: 8:10 + let mut _13: i32; // in scope 0 at $DIR/remove_storage_markers.rs:8:9: 8:10 + let mut _14: !; // in scope 0 at $DIR/remove_storage_markers.rs:8:5: 10:6 + let _16: (); // in scope 0 at $DIR/remove_storage_markers.rs:8:20: 10:6 + let mut _17: i32; // in scope 0 at $DIR/remove_storage_markers.rs:9:16: 9:17 + scope 1 { + debug sum => _1; // in scope 1 at $DIR/remove_storage_markers.rs:7:9: 7:16 + let mut _4: std::ops::Range; // in scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 + scope 2 { + debug iter => _4; // in scope 2 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let mut _6: i32; // in scope 2 at $DIR/remove_storage_markers.rs:8:14: 8:19 + scope 3 { + debug __next => _6; // in scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 + let _12: i32; // in scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 + let _15: i32; // in scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 + scope 4 { + debug val => _12; // in scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10 + } + scope 5 { + debug i => _15; // in scope 5 at $DIR/remove_storage_markers.rs:8:9: 8:10 + } + } + } + scope 6 (inlined as IntoIterator>::into_iter) { // at $DIR/remove_storage_markers.rs:8:14: 8:19 + debug self => _3; // in scope 6 at $DIR/remove_storage_markers.rs:8:14: 8:19 + } + } + + bb0: { +- StorageLive(_1); // scope 0 at $DIR/remove_storage_markers.rs:7:9: 7:16 + _1 = const 0_i32; // scope 0 at $DIR/remove_storage_markers.rs:7:19: 7:20 +- StorageLive(_2); // scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageLive(_3); // scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 + (_3.0: i32) = const 0_i32; // scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 + (_3.1: i32) = const 10_i32; // scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 + _2 = move _3; // scope 6 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageDead(_3); // scope 1 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageLive(_4); // scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 + _4 = move _2; // scope 1 at $DIR/remove_storage_markers.rs:8:14: 8:19 + goto -> bb1; // scope 2 at $DIR/remove_storage_markers.rs:8:5: 10:6 + } + + bb1: { +- StorageLive(_6); // scope 2 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageLive(_7); // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageLive(_8); // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageLive(_9); // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageLive(_10); // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 + _10 = &mut _4; // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 + _9 = &mut (*_10); // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 + _8 = as Iterator>::next(move _9) -> bb2; // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 + // mir::Constant + // + span: $DIR/remove_storage_markers.rs:8:14: 8:19 + // + literal: Const { ty: for<'r> fn(&'r mut std::ops::Range) -> std::option::Option< as std::iter::Iterator>::Item> { as std::iter::Iterator>::next}, val: Value(Scalar()) } + } + + bb2: { +- StorageDead(_9); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 + _11 = discriminant(_8); // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 + switchInt(move _11) -> [0_isize: bb3, otherwise: bb4]; // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 + } + + bb3: { + _0 = const (); // scope 3 at $DIR/remove_storage_markers.rs:8:5: 10:6 +- StorageDead(_10); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_8); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_7); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_6); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 +- StorageDead(_4); // scope 1 at $DIR/remove_storage_markers.rs:10:5: 10:6 +- StorageDead(_2); // scope 1 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_1); // scope 0 at $DIR/remove_storage_markers.rs:11:1: 11:2 + return; // scope 0 at $DIR/remove_storage_markers.rs:11:2: 11:2 + } + + bb4: { +- StorageLive(_12); // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 + _12 = ((_8 as Some).0: i32); // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 +- StorageLive(_13); // scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10 + _13 = _12; // scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10 + _6 = move _13; // scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10 + _7 = const (); // scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10 +- StorageDead(_13); // scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10 +- StorageDead(_12); // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 +- StorageDead(_10); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_8); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_7); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageLive(_15); // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10 + _15 = _6; // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageLive(_16); // scope 5 at $DIR/remove_storage_markers.rs:8:20: 10:6 +- StorageLive(_17); // scope 5 at $DIR/remove_storage_markers.rs:9:16: 9:17 + _17 = _15; // scope 5 at $DIR/remove_storage_markers.rs:9:16: 9:17 + _1 = Add(_1, move _17); // scope 5 at $DIR/remove_storage_markers.rs:9:9: 9:17 +- StorageDead(_17); // scope 5 at $DIR/remove_storage_markers.rs:9:16: 9:17 + _16 = const (); // scope 5 at $DIR/remove_storage_markers.rs:8:20: 10:6 +- StorageDead(_16); // scope 5 at $DIR/remove_storage_markers.rs:10:5: 10:6 + _5 = const (); // scope 2 at $DIR/remove_storage_markers.rs:8:5: 10:6 +- StorageDead(_15); // scope 3 at $DIR/remove_storage_markers.rs:10:5: 10:6 +- StorageDead(_6); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 + goto -> bb1; // scope 2 at $DIR/remove_storage_markers.rs:8:5: 10:6 + } + } + diff --git a/src/test/mir-opt/remove_storage_markers.rs b/src/test/mir-opt/remove_storage_markers.rs new file mode 100644 index 00000000000..c144d3ff795 --- /dev/null +++ b/src/test/mir-opt/remove_storage_markers.rs @@ -0,0 +1,11 @@ +// Checks that storage markers are removed at opt-level=0. +// +// compile-flags: -C opt-level=0 -Coverflow-checks=off + +// EMIT_MIR remove_storage_markers.main.RemoveStorageMarkers.diff +fn main() { + let mut sum = 0; + for i in 0..10 { + sum += i; + } +} diff --git a/src/test/run-make/const_fn_mir/dump.mir b/src/test/run-make/const_fn_mir/dump.mir index 3dac42c6782..45d018cf305 100644 --- a/src/test/run-make/const_fn_mir/dump.mir +++ b/src/test/run-make/const_fn_mir/dump.mir @@ -5,7 +5,6 @@ fn main() -> () { let _1: i32; // in scope 0 at main.rs:9:5: 9:10 bb0: { - StorageLive(_1); // scope 0 at main.rs:9:5: 9:10 _1 = foo() -> bb1; // scope 0 at main.rs:9:5: 9:10 // mir::Constant // + span: main.rs:9:5: 9:8 @@ -13,7 +12,6 @@ fn main() -> () { } bb1: { - StorageDead(_1); // scope 0 at main.rs:9:10: 9:11 _0 = const (); // scope 0 at main.rs:8:11: 10:2 return; // scope 0 at main.rs:10:2: 10:2 } From 70760507e6c8f98acbad02bba79ba152f1c740ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 13 Jan 2021 00:00:00 +0000 Subject: [PATCH 2/3] compiletest: run mir-opt tests with -C opt-level=1 The storage markers are removed at -C opt-level=0 and as a result output of mir opt tests vary based on used optimization level. Use opt-level=1 for mir-opt tests to avoid the issue. --- src/tools/compiletest/src/runtest.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 61b21bce10b..723c7f86832 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1957,6 +1957,7 @@ impl<'test> TestCx<'test> { } MirOpt => { rustc.args(&[ + "-Copt-level=1", "-Zdump-mir=all", "-Zmir-opt-level=3", "-Zvalidate-mir", From 57de468ff98387a998afe089b77466ee50733c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sun, 28 Feb 2021 00:00:00 +0000 Subject: [PATCH 3/3] Use -O in try_identity test that requires storage markers --- src/test/codegen/try_identity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/codegen/try_identity.rs b/src/test/codegen/try_identity.rs index d30b706eafc..d51bd575719 100644 --- a/src/test/codegen/try_identity.rs +++ b/src/test/codegen/try_identity.rs @@ -1,4 +1,4 @@ -// compile-flags: -C no-prepopulate-passes -Z mir-opt-level=2 -Zunsound-mir-opts +// compile-flags: -C no-prepopulate-passes -O -Z mir-opt-level=2 -Zunsound-mir-opts // Ensure that `x?` has no overhead on `Result` due to identity `match`es in lowering. // This requires inlining to trigger the MIR optimizations in `SimplifyArmIdentity`.