From 1447daa01ddc6536724eb4f3e10972404da0cd56 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 31 Jul 2017 16:00:12 +0300 Subject: [PATCH 1/5] remove the span field from `diverge_cleanup` --- src/librustc_mir/build/block.rs | 2 +- src/librustc_mir/build/expr/into.rs | 2 +- src/librustc_mir/build/matches/test.rs | 2 +- src/librustc_mir/build/scope.rs | 27 +++++++++++++++----------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 865174aa272ec..4583d80b83ddc 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -86,7 +86,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let tcx = this.hir.tcx(); // Enter the remainder scope, i.e. the bindings' destruction scope. - this.push_scope(remainder_scope); + this.push_scope((remainder_scope, source_info)); let_extent_stack.push(remainder_scope); // Declare the bindings, which may create a visibility scope. diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 326c1df69ebeb..7ae5d6b0ec19a 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -237,7 +237,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .collect(); let success = this.cfg.start_new_block(); - let cleanup = this.diverge_cleanup(expr_span); + let cleanup = this.diverge_cleanup(); this.cfg.terminate(block, source_info, TerminatorKind::Call { func: fun, args: args, diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index f4d43e041ae87..28386fa598ce6 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -306,7 +306,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let bool_ty = self.hir.bool_ty(); let eq_result = self.temp(bool_ty, test.span); let eq_block = self.cfg.start_new_block(); - let cleanup = self.diverge_cleanup(test.span); + let cleanup = self.diverge_cleanup(); self.cfg.terminate(block, source_info, TerminatorKind::Call { func: Operand::Constant(box Constant { span: test.span, diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 2244ffde3c9d0..2b52198c25065 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -107,6 +107,9 @@ pub struct Scope<'tcx> { /// the extent of this scope within source code. extent: CodeExtent, + /// the span of that extent + extent_span: Span, + /// Whether there's anything to do for the cleanup path, that is, /// when unwinding through this scope. This includes destructors, /// but not StorageDead statements, which don't get emitted at all @@ -116,7 +119,7 @@ pub struct Scope<'tcx> { /// * pollutting the cleanup MIR with StorageDead creates /// landing pads even though there's no actual destructors /// * freeing up stack space has no effect during unwinding - pub(super) needs_cleanup: bool, + needs_cleanup: bool, /// set of lvalues to drop when exiting this scope. This starts /// out empty but grows as variables are declared during the @@ -282,7 +285,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) -> BlockAnd { debug!("in_opt_scope(opt_extent={:?}, block={:?})", opt_extent, block); - if let Some(extent) = opt_extent { self.push_scope(extent.0); } + if let Some(extent) = opt_extent { self.push_scope(extent); } let rv = unpack!(block = f(self)); if let Some(extent) = opt_extent { unpack!(block = self.pop_scope(extent, block)); @@ -301,7 +304,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) -> BlockAnd { debug!("in_scope(extent={:?}, block={:?})", extent, block); - self.push_scope(extent.0); + self.push_scope(extent); let rv = unpack!(block = f(self)); unpack!(block = self.pop_scope(extent, block)); debug!("in_scope: exiting extent={:?} block={:?}", extent, block); @@ -312,12 +315,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// scope and call `pop_scope` afterwards. Note that these two /// calls must be paired; using `in_scope` as a convenience /// wrapper maybe preferable. - pub fn push_scope(&mut self, extent: CodeExtent) { + pub fn push_scope(&mut self, extent: (CodeExtent, SourceInfo)) { debug!("push_scope({:?})", extent); let vis_scope = self.visibility_scope; self.scopes.push(Scope { visibility_scope: vis_scope, - extent: extent, + extent: extent.0, + extent_span: extent.1.span, needs_cleanup: false, drops: vec![], free: None, @@ -335,7 +339,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { debug!("pop_scope({:?}, {:?})", extent, block); // We need to have `cached_block`s available for all the drops, so we call diverge_cleanup // to make sure all the `cached_block`s are filled in. - self.diverge_cleanup(extent.1.span); + self.diverge_cleanup(); let scope = self.scopes.pop().unwrap(); assert_eq!(scope.extent, extent.0); unpack!(block = build_scope_drops(&mut self.cfg, @@ -618,7 +622,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// This path terminates in Resume. Returns the start of the path. /// See module comment for more details. None indicates there’s no /// cleanup to do at this point. - pub fn diverge_cleanup(&mut self, span: Span) -> Option { + pub fn diverge_cleanup(&mut self) -> Option { if !self.scopes.iter().any(|scope| scope.needs_cleanup) { return None; } @@ -652,7 +656,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }; for scope in scopes.iter_mut() { - target = build_diverge_scope(hir.tcx(), cfg, &unit_temp, span, scope, target); + target = build_diverge_scope( + hir.tcx(), cfg, &unit_temp, scope.extent_span, scope, target); } Some(target) } @@ -668,7 +673,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } let source_info = self.source_info(span); let next_target = self.cfg.start_new_block(); - let diverge_target = self.diverge_cleanup(span); + let diverge_target = self.diverge_cleanup(); self.cfg.terminate(block, source_info, TerminatorKind::Drop { location: location, @@ -686,7 +691,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { value: Operand<'tcx>) -> BlockAnd<()> { let source_info = self.source_info(span); let next_target = self.cfg.start_new_block(); - let diverge_target = self.diverge_cleanup(span); + let diverge_target = self.diverge_cleanup(); self.cfg.terminate(block, source_info, TerminatorKind::DropAndReplace { location: location, @@ -709,7 +714,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = self.source_info(span); let success_block = self.cfg.start_new_block(); - let cleanup = self.diverge_cleanup(span); + let cleanup = self.diverge_cleanup(); self.cfg.terminate(block, source_info, TerminatorKind::Assert { From 85c102757a744956989e5217484a6f650eed3146 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 31 Jul 2017 23:25:27 +0300 Subject: [PATCH 2/5] rustc_mir: don't build unused unwind cleanup blocks The unused blocks are removed by SimplifyCfg, but they can cause a significant performance slowdown before they are removed. --- src/librustc_mir/build/scope.rs | 89 ++++++++++++++++++---------- src/test/mir-opt/basic_assignment.rs | 26 ++++---- src/test/mir-opt/end_region_4.rs | 38 ++++++------ src/test/mir-opt/end_region_5.rs | 27 ++++----- src/test/mir-opt/end_region_6.rs | 28 ++++----- src/test/mir-opt/end_region_7.rs | 36 +++++------ src/test/mir-opt/end_region_8.rs | 75 ++++++++++++----------- src/test/mir-opt/issue-41110.rs | 15 +++-- 8 files changed, 181 insertions(+), 153 deletions(-) diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 2b52198c25065..bf39e52bd1b28 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -200,6 +200,15 @@ pub struct BreakableScope<'tcx> { pub break_destination: Lvalue<'tcx>, } +impl DropKind { + fn may_panic(&self) -> bool { + match *self { + DropKind::Value { .. } => true, + DropKind::Storage => false + } + } +} + impl<'tcx> Scope<'tcx> { /// Invalidate all the cached blocks in the scope. /// @@ -337,9 +346,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { mut block: BasicBlock) -> BlockAnd<()> { debug!("pop_scope({:?}, {:?})", extent, block); - // We need to have `cached_block`s available for all the drops, so we call diverge_cleanup - // to make sure all the `cached_block`s are filled in. - self.diverge_cleanup(); + // If we are emitting a `drop` statement, we need to have the cached + // diverge cleanup pads ready in case that drop panics. + let may_panic = + self.scopes.last().unwrap().drops.iter().any(|s| s.kind.may_panic()); + if may_panic { + self.diverge_cleanup(); + } let scope = self.scopes.pop().unwrap(); assert_eq!(scope.extent, extent.0); unpack!(block = build_scope_drops(&mut self.cfg, @@ -370,6 +383,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let len = self.scopes.len(); assert!(scope_count < len, "should not use `exit_scope` to pop ALL scopes"); let tmp = self.get_unit_temp(); + + // If we are emitting a `drop` statement, we need to have the cached + // diverge cleanup pads ready in case that drop panics. + let may_panic = + self.scopes[(len - scope_count)..].iter().any(|s| s.drops.iter().any(|s| s.kind.may_panic())); + if may_panic { + self.diverge_cleanup(); + } + { let mut rest = &mut self.scopes[(len - scope_count)..]; while let Some((scope, rest_)) = {rest}.split_last_mut() { @@ -736,45 +758,48 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>, mut block: BasicBlock, arg_count: usize) -> BlockAnd<()> { + debug!("build_scope_drops({:?} -> {:?})", block, scope); let mut iter = scope.drops.iter().rev().peekable(); while let Some(drop_data) = iter.next() { let source_info = scope.source_info(drop_data.span); - if let DropKind::Value { .. } = drop_data.kind { - // Try to find the next block with its cached block - // for us to diverge into in case the drop panics. - let on_diverge = iter.peek().iter().filter_map(|dd| { - match dd.kind { - DropKind::Value { cached_block } => cached_block, - DropKind::Storage => None - } - }).next(); - // If there’s no `cached_block`s within current scope, - // we must look for one in the enclosing scope. - let on_diverge = on_diverge.or_else(||{ - earlier_scopes.iter().rev().flat_map(|s| s.cached_block()).next() - }); - let next = cfg.start_new_block(); - cfg.terminate(block, source_info, TerminatorKind::Drop { - location: drop_data.location.clone(), - target: next, - unwind: on_diverge - }); - block = next; - } match drop_data.kind { - DropKind::Value { .. } | - DropKind::Storage => { - // Only temps and vars need their storage dead. - match drop_data.location { - Lvalue::Local(index) if index.index() > arg_count => {} - _ => continue - } + DropKind::Value { .. } => { + // Try to find the next block with its cached block + // for us to diverge into in case the drop panics. + let on_diverge = iter.peek().iter().filter_map(|dd| { + match dd.kind { + DropKind::Value { cached_block: None } => + span_bug!(drop_data.span, "cached block not present?"), + DropKind::Value { cached_block } => cached_block, + DropKind::Storage => None + } + }).next(); + // If there’s no `cached_block`s within current scope, + // we must look for one in the enclosing scope. + let on_diverge = on_diverge.or_else(|| { + earlier_scopes.iter().rev().flat_map(|s| s.cached_block()).next() + }); + let next = cfg.start_new_block(); + cfg.terminate(block, source_info, TerminatorKind::Drop { + location: drop_data.location.clone(), + target: next, + unwind: on_diverge + }); + block = next; + } + DropKind::Storage => {} + } + // Drop the storage for both value and storage drops. + // Only temps and vars need their storage dead. + match drop_data.location { + Lvalue::Local(index) if index.index() > arg_count => { cfg.push(block, Statement { source_info: source_info, kind: StatementKind::StorageDead(drop_data.location.clone()) }); } + _ => continue } } block.unit() diff --git a/src/test/mir-opt/basic_assignment.rs b/src/test/mir-opt/basic_assignment.rs index ef5158a403a93..6afc344ced847 100644 --- a/src/test/mir-opt/basic_assignment.rs +++ b/src/test/mir-opt/basic_assignment.rs @@ -47,39 +47,39 @@ fn main() { // StorageDead(_3); // StorageLive(_4); // _4 = std::option::Option>::None; +// StorageLive(_5); // StorageLive(_6); -// StorageLive(_7); -// _7 = _4; -// replace(_6 <- _7) -> [return: bb6, unwind: bb7]; +// _6 = _4; +// replace(_5 <- _6) -> [return: bb1, unwind: bb7]; // } // bb1: { -// resume; +// drop(_6) -> [return: bb8, unwind: bb5]; // } // bb2: { -// drop(_4) -> bb1; +// resume; // } // bb3: { -// goto -> bb2; +// drop(_4) -> bb2; // } // bb4: { -// drop(_6) -> bb3; +// goto -> bb3; // } // bb5: { -// goto -> bb4; +// drop(_5) -> bb4; // } // bb6: { -// drop(_7) -> [return: bb8, unwind: bb4]; +// goto -> bb5; // } // bb7: { -// drop(_7) -> bb5; +// drop(_6) -> bb6; // } // bb8: { -// StorageDead(_7); +// StorageDead(_6); // _0 = (); -// drop(_6) -> [return: bb9, unwind: bb2]; +// drop(_5) -> [return: bb9, unwind: bb3]; // } // bb9: { -// StorageDead(_6); +// StorageDead(_5); // drop(_4) -> bb10; // } // bb10: { diff --git a/src/test/mir-opt/end_region_4.rs b/src/test/mir-opt/end_region_4.rs index 16ade9f96fd1e..bfb1b3b652890 100644 --- a/src/test/mir-opt/end_region_4.rs +++ b/src/test/mir-opt/end_region_4.rs @@ -32,41 +32,41 @@ fn foo(i: i32) { // START rustc.node4.SimplifyCfg-qualify-consts.after.mir // let mut _0: (); // let _1: D; -// let _3: i32; -// let _4: &'6_2rce i32; +// let _2: i32; +// let _3: &'6_2rce i32; // let _7: &'6_4rce i32; -// let mut _5: (); -// let mut _6: i32; -// +// let mut _4: (); +// let mut _5: i32; +// let mut _6: (); // bb0: { // StorageLive(_1); // _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_2); +// _2 = const 0i32; // StorageLive(_3); -// _3 = const 0i32; -// StorageLive(_4); -// _4 = &'6_2rce _3; -// StorageLive(_6); -// _6 = (*_4); -// _5 = const foo(_6) -> [return: bb2, unwind: bb3]; +// _3 = &'6_2rce _2; +// StorageLive(_5); +// _5 = (*_3); +// _4 = const foo(_5) -> [return: bb1, unwind: bb3]; // } // bb1: { -// resume; -// } -// bb2: { -// StorageDead(_6); +// StorageDead(_5); // StorageLive(_7); -// _7 = &'6_4rce _3; +// _7 = &'6_4rce _2; // _0 = (); // StorageDead(_7); // EndRegion('6_4rce); -// StorageDead(_4); -// EndRegion('6_2rce); // StorageDead(_3); +// EndRegion('6_2rce); +// StorageDead(_2); // drop(_1) -> bb4; // } +// bb2: { +// resume; +// } // bb3: { // EndRegion('6_2rce); -// drop(_1) -> bb1; +// drop(_1) -> bb2; // } // bb4: { // StorageDead(_1); diff --git a/src/test/mir-opt/end_region_5.rs b/src/test/mir-opt/end_region_5.rs index 513632a4cdf38..773a348a93977 100644 --- a/src/test/mir-opt/end_region_5.rs +++ b/src/test/mir-opt/end_region_5.rs @@ -31,32 +31,31 @@ fn foo(f: F) where F: FnOnce() -> i32 { // let mut _0: (); // let _1: D; // let mut _2: (); -// let mut _3: (); -// let mut _4: [closure@NodeId(18) d: &'19mce D]; -// let mut _5: &'19mce D; -// +// let mut _3: [closure@NodeId(18) d:&'19mce D]; +// let mut _4: &'19mce D; +// let mut _5: (); // bb0: { // StorageLive(_1); // _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_3); // StorageLive(_4); -// StorageLive(_5); -// _5 = &'19mce _1; -// _4 = [closure@NodeId(18)] { d: _5 }; -// StorageDead(_5); -// _3 = const foo(_4) -> [return: bb2, unwind: bb3]; +// _4 = &'19mce _1; +// _3 = [closure@NodeId(18)] { d: _4 }; +// StorageDead(_4); +// _2 = const foo(_3) -> [return: bb1, unwind: bb3]; // } // bb1: { -// resume; -// } -// bb2: { -// StorageDead(_4); +// StorageDead(_3); // EndRegion('19mce); // _0 = (); // drop(_1) -> bb4; // } +// bb2: { +// resume; +// } // bb3: { // EndRegion('19mce); -// drop(_1) -> bb1; +// drop(_1) -> bb2; // } // bb4: { // StorageDead(_1); diff --git a/src/test/mir-opt/end_region_6.rs b/src/test/mir-opt/end_region_6.rs index e82556f3ce4bb..112c93843e042 100644 --- a/src/test/mir-opt/end_region_6.rs +++ b/src/test/mir-opt/end_region_6.rs @@ -27,35 +27,35 @@ fn foo(f: F) where F: FnOnce() -> i32 { // END RUST SOURCE // START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// fn main() -> () { // let mut _0: (); // let _1: D; // let mut _2: (); -// let mut _3: (); -// let mut _4: [closure@NodeId(22) d:&'23mce D]; -// let mut _5: &'23mce D; -// +// let mut _3: [closure@NodeId(22) d:&'23mce D]; +// let mut _4: &'23mce D; +// let mut _5: (); // bb0: { // StorageLive(_1); // _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_3); // StorageLive(_4); -// StorageLive(_5); -// _5 = &'23mce _1; -// _4 = [closure@NodeId(22)] { d: _5 }; -// StorageDead(_5); -// _3 = const foo(_4) -> [return: bb2, unwind: bb3]; +// _4 = &'23mce _1; +// _3 = [closure@NodeId(22)] { d: _4 }; +// StorageDead(_4); +// _2 = const foo(_3) -> [return: bb1, unwind: bb3]; // } // bb1: { -// resume; -// } -// bb2: { -// StorageDead(_4); +// StorageDead(_3); // EndRegion('23mce); // _0 = (); // drop(_1) -> bb4; // } +// bb2: { +// resume; +// } // bb3: { // EndRegion('23mce); -// drop(_1) -> bb1; +// drop(_1) -> bb2; // } // bb4: { // StorageDead(_1); diff --git a/src/test/mir-opt/end_region_7.rs b/src/test/mir-opt/end_region_7.rs index 3fbd3f368659d..913986ae816a6 100644 --- a/src/test/mir-opt/end_region_7.rs +++ b/src/test/mir-opt/end_region_7.rs @@ -31,18 +31,18 @@ fn foo(f: F) where F: FnOnce() -> i32 { // let mut _0: (); // let _1: D; // let mut _2: (); -// let mut _3: (); -// let mut _4: [closure@NodeId(22) d:D]; -// let mut _5: D; +// let mut _3: [closure@NodeId(22) d:D]; +// let mut _4: D; +// let mut _5: (); // // bb0: { // StorageLive(_1); // _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_3); // StorageLive(_4); -// StorageLive(_5); -// _5 = _1; -// _4 = [closure@NodeId(22)] { d: _5 }; -// drop(_5) -> [return: bb4, unwind: bb3]; +// _4 = _1; +// _3 = [closure@NodeId(22)] { d: _4 }; +// drop(_4) -> [return: bb4, unwind: bb3]; // } // bb1: { // resume; @@ -51,17 +51,17 @@ fn foo(f: F) where F: FnOnce() -> i32 { // drop(_1) -> bb1; // } // bb3: { -// drop(_4) -> bb2; +// drop(_3) -> bb2; // } // bb4: { -// StorageDead(_5); -// _3 = const foo(_4) -> [return: bb5, unwind: bb3]; +// StorageDead(_4); +// _2 = const foo(_3) -> [return: bb5, unwind: bb3]; // } // bb5: { -// drop(_4) -> [return: bb6, unwind: bb2]; +// drop(_3) -> [return: bb6, unwind: bb2]; // } // bb6: { -// StorageDead(_4); +// StorageDead(_3); // _0 = (); // drop(_1) -> bb7; // } @@ -76,16 +76,16 @@ fn foo(f: F) where F: FnOnce() -> i32 { // fn main::{{closure}}(_1: [closure@NodeId(22) d:D]) -> i32 { // let mut _0: i32; // let _2: &'14_0rce D; -// let mut _3: (); -// let mut _4: i32; +// let mut _3: i32; +// let mut _4: (); // // bb0: { // StorageLive(_2); // _2 = &'14_0rce (_1.0: D); -// StorageLive(_4); -// _4 = ((*_2).0: i32); -// _0 = _4; -// StorageDead(_4); +// StorageLive(_3); +// _3 = ((*_2).0: i32); +// _0 = _3; +// StorageDead(_3); // StorageDead(_2); // EndRegion('14_0rce); // drop(_1) -> bb1; diff --git a/src/test/mir-opt/end_region_8.rs b/src/test/mir-opt/end_region_8.rs index 7fb3f0b91181a..dc8f8ea11f51c 100644 --- a/src/test/mir-opt/end_region_8.rs +++ b/src/test/mir-opt/end_region_8.rs @@ -29,44 +29,43 @@ fn foo(f: F) where F: FnOnce() -> i32 { // END RUST SOURCE // START rustc.node4.SimplifyCfg-qualify-consts.after.mir // fn main() -> () { -// let mut _0: (); -// let _1: D; -// let _3: &'6_1rce D; -// let mut _2: (); -// let mut _4: (); -// let mut _5: [closure@NodeId(22) r:&'6_1rce D]; -// let mut _6: &'6_1rce D; -// -// bb0: { -// StorageLive(_1); -// _1 = D::{{constructor}}(const 0i32,); -// StorageLive(_3); -// _3 = &'6_1rce _1; -// StorageLive(_5); -// StorageLive(_6); -// _6 = _3; -// _5 = [closure@NodeId(22)] { r: _6 }; -// StorageDead(_6); -// _4 = const foo(_5) -> [return: bb2, unwind: bb3]; -// } -// bb1: { -// resume; -// } -// bb2: { -// StorageDead(_5); -// _0 = (); -// StorageDead(_3); -// EndRegion('6_1rce); -// drop(_1) -> bb4; -// } -// bb3: { -// EndRegion('6_1rce); -// drop(_1) -> bb1; -// } -// bb4: { -// StorageDead(_1); -// return; -// } +// let mut _0: (); +// let _1: D; +// let _2: &'6_1rce D; +// let mut _3: (); +// let mut _4: [closure@NodeId(22) r:&'6_1rce D]; +// let mut _5: &'6_1rce D; +// let mut _6: (); +// bb0: { +// StorageLive(_1); +// _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_2); +// _2 = &'6_1rce _1; +// StorageLive(_4); +// StorageLive(_5); +// _5 = _2; +// _4 = [closure@NodeId(22)] { r: _5 }; +// StorageDead(_5); +// _3 = const foo(_4) -> [return: bb1, unwind: bb3]; +// } +// bb1: { +// StorageDead(_4); +// _0 = (); +// StorageDead(_2); +// EndRegion('6_1rce); +// drop(_1) -> bb4; +// } +// bb2: { +// resume; +// } +// bb3: { +// EndRegion('6_1rce); +// drop(_1) -> bb2; +// } +// bb4: { +// StorageDead(_1); +// return; +// } // } // END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/issue-41110.rs b/src/test/mir-opt/issue-41110.rs index fec635b3abf60..1daa18256dcee 100644 --- a/src/test/mir-opt/issue-41110.rs +++ b/src/test/mir-opt/issue-41110.rs @@ -34,18 +34,23 @@ impl S { // END RUST SOURCE // START rustc.node4.ElaborateDrops.after.mir +// let mut _0: (); +// let _1: (); // let mut _2: S; -// let mut _3: (); +// let mut _3: S; // let mut _4: S; -// let mut _5: S; +// let mut _5: (); // let mut _6: bool; // // bb0: { // END rustc.node4.ElaborateDrops.after.mir // START rustc.node13.ElaborateDrops.after.mir -// let mut _2: (); -// let mut _4: (); -// let mut _5: S; +// let mut _0: (); +// let _1: S; +// let mut _2: S; +// let mut _3: (); +// let mut _4: S; +// let mut _5: (); // let mut _6: S; // let mut _7: bool; // From ca3105cfdf4221d6855f16f2f841d359248c349a Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 1 Aug 2017 00:09:32 +0300 Subject: [PATCH 3/5] use an iterator when visiting MIR basic blocks I saw MIR cache invalidation somewhat hot on my profiler when per-BB indexin was used. That shouldn't matter much, but there is no good reason not to use an iterator. --- src/librustc/mir/visit.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index fd3a9f8cd2d9a..a24b2ad0e4324 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -14,7 +14,6 @@ use ty::subst::Substs; use ty::{ClosureSubsts, Region, Ty}; use mir::*; use rustc_const_math::ConstUsize; -use rustc_data_structures::indexed_vec::Idx; use syntax_pos::Span; // # The MIR Visitor @@ -260,9 +259,15 @@ macro_rules! make_mir_visitor { fn super_mir(&mut self, mir: & $($mutability)* Mir<'tcx>) { - for index in 0..mir.basic_blocks().len() { - let block = BasicBlock::new(index); - self.visit_basic_block_data(block, &$($mutability)* mir[block]); + // for best performance, we want to use an iterator rather + // than a for-loop, to avoid calling Mir::invalidate for + // each basic block. + macro_rules! basic_blocks { + (mut) => (mir.basic_blocks_mut().iter_enumerated_mut()); + () => (mir.basic_blocks().iter_enumerated()); + }; + for (bb, data) in basic_blocks!($($mutability)*) { + self.visit_basic_block_data(bb, data); } for scope in &$($mutability)* mir.visibility_scopes { From 5b99523de9cb362a2328829959618aef0becb38e Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 1 Aug 2017 00:10:46 +0300 Subject: [PATCH 4/5] rustc_mir::transform::simplify - remove nops first Removing nops can allow more basic blocks to be merged, but merging basic blocks can't allow for more nops to be removed, so we should remove nops first. This doesn't matter *that* much, because normally we run SimplifyCfg several times, but there's no reason not to do it. --- src/librustc_mir/transform/simplify.rs | 4 ++-- src/test/mir-opt/basic_assignment.rs | 22 ++++++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/transform/simplify.rs b/src/librustc_mir/transform/simplify.rs index d5b79c0d1c382..a1d56ccd874a1 100644 --- a/src/librustc_mir/transform/simplify.rs +++ b/src/librustc_mir/transform/simplify.rs @@ -105,6 +105,8 @@ impl<'a, 'tcx: 'a> CfgSimplifier<'a, 'tcx> { } pub fn simplify(mut self) { + self.strip_nops(); + loop { let mut changed = false; @@ -141,8 +143,6 @@ impl<'a, 'tcx: 'a> CfgSimplifier<'a, 'tcx> { if !changed { break } } - - self.strip_nops() } // Collapse a goto chain starting from `start` diff --git a/src/test/mir-opt/basic_assignment.rs b/src/test/mir-opt/basic_assignment.rs index 6afc344ced847..d3bf7f68785d5 100644 --- a/src/test/mir-opt/basic_assignment.rs +++ b/src/test/mir-opt/basic_assignment.rs @@ -50,10 +50,10 @@ fn main() { // StorageLive(_5); // StorageLive(_6); // _6 = _4; -// replace(_5 <- _6) -> [return: bb1, unwind: bb7]; +// replace(_5 <- _6) -> [return: bb1, unwind: bb5]; // } // bb1: { -// drop(_6) -> [return: bb8, unwind: bb5]; +// drop(_6) -> [return: bb6, unwind: bb4]; // } // bb2: { // resume; @@ -62,27 +62,21 @@ fn main() { // drop(_4) -> bb2; // } // bb4: { -// goto -> bb3; +// drop(_5) -> bb3; // } // bb5: { -// drop(_5) -> bb4; +// drop(_6) -> bb4; // } // bb6: { -// goto -> bb5; -// } -// bb7: { -// drop(_6) -> bb6; -// } -// bb8: { // StorageDead(_6); // _0 = (); -// drop(_5) -> [return: bb9, unwind: bb3]; +// drop(_5) -> [return: bb7, unwind: bb3]; // } -// bb9: { +// bb7: { // StorageDead(_5); -// drop(_4) -> bb10; +// drop(_4) -> bb8; // } -// bb10: { +// bb8: { // StorageDead(_4); // StorageDead(_2); // StorageDead(_1); From ce0ca763808f4b5d153aaa2787ea253286b449ef Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 1 Aug 2017 11:57:26 +0300 Subject: [PATCH 5/5] pacify the merciless tidy --- src/librustc_mir/build/scope.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index bf39e52bd1b28..ccba87a4d26a6 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -386,8 +386,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // If we are emitting a `drop` statement, we need to have the cached // diverge cleanup pads ready in case that drop panics. - let may_panic = - self.scopes[(len - scope_count)..].iter().any(|s| s.drops.iter().any(|s| s.kind.may_panic())); + let may_panic = self.scopes[(len - scope_count)..].iter() + .any(|s| s.drops.iter().any(|s| s.kind.may_panic())); if may_panic { self.diverge_cleanup(); }