From c4e8ab9bc5189b861d0252e9c3724328e574e42d Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 6 Jan 2023 19:29:24 -0500 Subject: [PATCH 1/6] MultipleReturnTerminators + SingleReturnTerminator MultipleReturnTerminators is maybe good for us but bad for LLVM. So this enables MultipleReturnTerminators and runs a SingleReturnTerminator pass at the end of optimizations. --- compiler/rustc_mir_transform/src/lib.rs | 3 +- .../src/multiple_return_terminators.rs | 9 +-- .../src/single_return_terminator.rs | 58 +++++++++++++++++++ ..._regression.encode.SimplifyBranchSame.diff | 6 +- .../inline/inline_generator.main.Inline.diff | 4 +- ...nators.test.MultipleReturnTerminators.diff | 8 +++ .../try_identity_e2e.new.PreCodegen.after.mir | 8 ++- .../try_identity_e2e.old.PreCodegen.after.mir | 8 ++- 8 files changed, 88 insertions(+), 16 deletions(-) create mode 100644 compiler/rustc_mir_transform/src/single_return_terminator.rs diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 16b8a901f3651..52166b0848da4 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -86,6 +86,7 @@ mod required_consts; mod reveal_all; mod separate_const_switch; mod shim; +mod single_return_terminator; // This pass is public to allow external drivers to perform MIR cleanup pub mod simplify; mod simplify_branches; @@ -577,7 +578,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { &o1(simplify::SimplifyCfg::new("final")), &nrvo::RenameReturnPlace, &simplify::SimplifyLocals::new("final"), - &multiple_return_terminators::MultipleReturnTerminators, + &single_return_terminator::SingleReturnTerminator, &deduplicate_blocks::DeduplicateBlocks, // Some cleanup necessary at least for LLVM and potentially other codegen backends. &add_call_guards::CriticalCallEdges, diff --git a/compiler/rustc_mir_transform/src/multiple_return_terminators.rs b/compiler/rustc_mir_transform/src/multiple_return_terminators.rs index 3957cd92c4e39..d1d48a70f146c 100644 --- a/compiler/rustc_mir_transform/src/multiple_return_terminators.rs +++ b/compiler/rustc_mir_transform/src/multiple_return_terminators.rs @@ -10,7 +10,7 @@ pub struct MultipleReturnTerminators; impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - sess.mir_opt_level() >= 4 + sess.mir_opt_level() >= 1 } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { @@ -26,14 +26,15 @@ impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators { } } - for bb in bbs { + for bb in 0..bbs.len() { + let bb = bb.into(); if !tcx.consider_optimizing(|| format!("MultipleReturnTerminators {:?} ", def_id)) { break; } - if let TerminatorKind::Goto { target } = bb.terminator().kind { + if let TerminatorKind::Goto { target } = bbs[bb].terminator().kind { if bbs_simple_returns.contains(target) { - bb.terminator_mut().kind = TerminatorKind::Return; + *bbs[bb].terminator_mut() = bbs[target].terminator().clone(); } } } diff --git a/compiler/rustc_mir_transform/src/single_return_terminator.rs b/compiler/rustc_mir_transform/src/single_return_terminator.rs new file mode 100644 index 0000000000000..0ff705c79a071 --- /dev/null +++ b/compiler/rustc_mir_transform/src/single_return_terminator.rs @@ -0,0 +1,58 @@ +use crate::MirPass; +use rustc_middle::mir::*; +use rustc_middle::ty::TyCtxt; + +use super::simplify::simplify_cfg; + +pub struct SingleReturnTerminator; + +impl<'tcx> MirPass<'tcx> for SingleReturnTerminator { + fn is_enabled(&self, sess: &rustc_session::Session) -> bool { + sess.mir_opt_level() >= 1 + } + + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + let mut returns = 0; + let mut return_terminator = None; + let mut return_block = None; + let mut has_extra_return_blocks = false; + + for (block, block_data) in body.basic_blocks.iter_enumerated() { + let terminator = block_data.terminator(); + if let TerminatorKind::Return = terminator.kind { + return_terminator = Some(terminator.clone()); + returns += 1; + if block_data.statements.is_empty() { + if return_block.is_some() { + has_extra_return_blocks = true; + } else { + return_block = Some(block); + } + } + } + } + + if returns < 2 { + return; + } + assert!(return_terminator.is_some()); + + let return_block = return_block.unwrap_or_else(|| { + body.basic_blocks_mut().push(BasicBlockData::new(return_terminator)) + }); + + for (bb, block) in body.basic_blocks_mut().iter_enumerated_mut() { + if bb == return_block { + continue; + } + let terminator = block.terminator_mut(); + if let TerminatorKind::Return = terminator.kind { + terminator.kind = TerminatorKind::Goto { target: return_block }; + } + } + + if has_extra_return_blocks { + simplify_cfg(tcx, body); + } + } +} diff --git a/tests/mir-opt/76803_regression.encode.SimplifyBranchSame.diff b/tests/mir-opt/76803_regression.encode.SimplifyBranchSame.diff index 9780332d8bf18..be58d5ee913fc 100644 --- a/tests/mir-opt/76803_regression.encode.SimplifyBranchSame.diff +++ b/tests/mir-opt/76803_regression.encode.SimplifyBranchSame.diff @@ -13,16 +13,12 @@ bb1: { _0 = move _1; // scope 0 at $DIR/76803_regression.rs:+3:14: +3:15 - goto -> bb3; // scope 0 at $DIR/76803_regression.rs:+3:14: +3:15 + return; // scope 0 at $DIR/76803_regression.rs:+5:2: +5:2 } bb2: { Deinit(_0); // scope 0 at $DIR/76803_regression.rs:+2:20: +2:27 discriminant(_0) = 1; // scope 0 at $DIR/76803_regression.rs:+2:20: +2:27 - goto -> bb3; // scope 0 at $DIR/76803_regression.rs:+2:20: +2:27 - } - - bb3: { return; // scope 0 at $DIR/76803_regression.rs:+5:2: +5:2 } } diff --git a/tests/mir-opt/inline/inline_generator.main.Inline.diff b/tests/mir-opt/inline/inline_generator.main.Inline.diff index f27b64c305457..6dde140093731 100644 --- a/tests/mir-opt/inline/inline_generator.main.Inline.diff +++ b/tests/mir-opt/inline/inline_generator.main.Inline.diff @@ -111,7 +111,7 @@ + discriminant(_1) = 0; // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 + _11 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]); // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 + discriminant((*_11)) = 3; // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 -+ goto -> bb1; // scope 0 at $DIR/inline_generator.rs:15:11: 15:39 ++ goto -> bb1; // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 + } + + bb7: { @@ -122,7 +122,7 @@ + discriminant(_1) = 1; // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 + _12 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]); // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 + discriminant((*_12)) = 1; // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 -+ goto -> bb1; // scope 0 at $DIR/inline_generator.rs:15:41: 15:41 ++ goto -> bb1; // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 + } + + bb8: { diff --git a/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff b/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff index 48a11c950639a..18faa8f01c7ab 100644 --- a/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff +++ b/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff @@ -4,8 +4,16 @@ fn test(_1: bool) -> () { debug x => _1; // in scope 0 at $DIR/multiple_return_terminators.rs:+0:9: +0:10 let mut _0: (); // return place in scope 0 at $DIR/multiple_return_terminators.rs:+0:18: +0:18 + let mut _2: bool; // in scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 + let mut _3: bool; // in scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 bb0: { + StorageLive(_2); // scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 + _2 = _1; // scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 + StorageLive(_3); // scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 + _3 = move _2; // scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 + StorageDead(_3); // scope 0 at $DIR/multiple_return_terminators.rs:+1:8: +1:9 + StorageDead(_2); // scope 0 at $DIR/multiple_return_terminators.rs:+5:5: +5:6 return; // scope 0 at $DIR/multiple_return_terminators.rs:+6:2: +6:2 } } diff --git a/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir b/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir index b254bfeb7c992..5251fe0fa67da 100644 --- a/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir +++ b/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir @@ -53,7 +53,7 @@ fn new(_1: Result) -> Result { ((_0 as Err).0: E) = move _8; // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51 discriminant(_0) = 1; // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51 StorageDead(_2); // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2 - return; // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2 + goto -> bb6; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 } bb4: { @@ -66,6 +66,10 @@ fn new(_1: Result) -> Result { ((_0 as Ok).0: T) = move _7; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6 discriminant(_0) = 0; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6 StorageDead(_2); // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2 - return; // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2 + goto -> bb6; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 + } + + bb6: { + return; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 } } diff --git a/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir b/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir index cdbc0681cb8a3..ec649dd126270 100644 --- a/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir +++ b/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir @@ -23,7 +23,7 @@ fn old(_1: Result) -> Result { Deinit(_0); // scope 2 at $DIR/try_identity_e2e.rs:+4:30: +4:36 ((_0 as Err).0: E) = move _4; // scope 2 at $DIR/try_identity_e2e.rs:+4:30: +4:36 discriminant(_0) = 1; // scope 2 at $DIR/try_identity_e2e.rs:+4:30: +4:36 - return; // scope 0 at $DIR/try_identity_e2e.rs:+7:1: +7:2 + goto -> bb4; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 } bb2: { @@ -35,6 +35,10 @@ fn old(_1: Result) -> Result { Deinit(_0); // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +6:6 ((_0 as Ok).0: T) = move _3; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +6:6 discriminant(_0) = 0; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +6:6 - return; // scope 0 at $DIR/try_identity_e2e.rs:+7:1: +7:2 + goto -> bb4; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 + } + + bb4: { + return; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 } } From 00335ab5fb53c4f319e32bc132a7c1f85b57e29c Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 8 Jan 2023 16:52:09 -0500 Subject: [PATCH 2/6] Convert to a single return terminator in codegen I also noticed that MultipleReturnTerminators doesn't seem to preserve the SourceInfo quite as well as it could, the tweak to it here might improve debuginfo. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 8 ++- compiler/rustc_codegen_ssa/src/mir/mod.rs | 17 ++++++ compiler/rustc_mir_transform/src/lib.rs | 2 - .../src/single_return_terminator.rs | 58 ------------------- .../inline/inline_generator.main.Inline.diff | 4 +- ...t_switch.identity.SeparateConstSwitch.diff | 4 +- .../try_identity_e2e.new.PreCodegen.after.mir | 6 +- .../try_identity_e2e.old.PreCodegen.after.mir | 6 +- 8 files changed, 29 insertions(+), 76 deletions(-) delete mode 100644 compiler/rustc_mir_transform/src/single_return_terminator.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 978aff511bfa7..c98271a24194f 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -361,7 +361,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } } - fn codegen_return_terminator(&mut self, bx: &mut Bx) { + pub fn codegen_return_terminator(&mut self, bx: &mut Bx) { // Call `va_end` if this is the definition of a C-variadic function. if self.fn_abi.c_variadic { // The `VaList` "spoofed" argument is just after all the real arguments. @@ -1296,7 +1296,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } mir::TerminatorKind::Return => { - self.codegen_return_terminator(bx); + if let Some(return_block) = self.return_block { + bx.br(return_block); + } else { + self.codegen_return_terminator(bx); + } MergingSucc::False } diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 79c66a955e76d..01cf55cf0fc65 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -72,6 +72,9 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { /// Cached unreachable block unreachable_block: Option, + /// Cached return block + return_block: Option, + /// Cached double unwind guarding block double_unwind_guard: Option, @@ -184,6 +187,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( personality_slot: None, cached_llbbs, unreachable_block: None, + return_block: None, double_unwind_guard: None, cleanup_kinds, landing_pads: IndexVec::from_elem(None, &mir.basic_blocks), @@ -256,6 +260,19 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // Apply debuginfo to the newly allocated locals. fx.debug_introduce_locals(&mut start_bx); + if mir + .basic_blocks + .iter() + .filter(|block| matches!(block.terminator().kind, mir::TerminatorKind::Return)) + .count() + > 1 + { + let llbb = Bx::append_block(fx.cx, fx.llfn, "return"); + let mut bx = Bx::build(fx.cx, llbb); + fx.codegen_return_terminator(&mut bx); + fx.return_block = Some(llbb); + } + // Codegen the body of each block using reverse postorder for (bb, _) in traversal::reverse_postorder(&mir) { fx.codegen_block(bb); diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 52166b0848da4..c2c7de7e66261 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -86,7 +86,6 @@ mod required_consts; mod reveal_all; mod separate_const_switch; mod shim; -mod single_return_terminator; // This pass is public to allow external drivers to perform MIR cleanup pub mod simplify; mod simplify_branches; @@ -578,7 +577,6 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { &o1(simplify::SimplifyCfg::new("final")), &nrvo::RenameReturnPlace, &simplify::SimplifyLocals::new("final"), - &single_return_terminator::SingleReturnTerminator, &deduplicate_blocks::DeduplicateBlocks, // Some cleanup necessary at least for LLVM and potentially other codegen backends. &add_call_guards::CriticalCallEdges, diff --git a/compiler/rustc_mir_transform/src/single_return_terminator.rs b/compiler/rustc_mir_transform/src/single_return_terminator.rs deleted file mode 100644 index 0ff705c79a071..0000000000000 --- a/compiler/rustc_mir_transform/src/single_return_terminator.rs +++ /dev/null @@ -1,58 +0,0 @@ -use crate::MirPass; -use rustc_middle::mir::*; -use rustc_middle::ty::TyCtxt; - -use super::simplify::simplify_cfg; - -pub struct SingleReturnTerminator; - -impl<'tcx> MirPass<'tcx> for SingleReturnTerminator { - fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - sess.mir_opt_level() >= 1 - } - - fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let mut returns = 0; - let mut return_terminator = None; - let mut return_block = None; - let mut has_extra_return_blocks = false; - - for (block, block_data) in body.basic_blocks.iter_enumerated() { - let terminator = block_data.terminator(); - if let TerminatorKind::Return = terminator.kind { - return_terminator = Some(terminator.clone()); - returns += 1; - if block_data.statements.is_empty() { - if return_block.is_some() { - has_extra_return_blocks = true; - } else { - return_block = Some(block); - } - } - } - } - - if returns < 2 { - return; - } - assert!(return_terminator.is_some()); - - let return_block = return_block.unwrap_or_else(|| { - body.basic_blocks_mut().push(BasicBlockData::new(return_terminator)) - }); - - for (bb, block) in body.basic_blocks_mut().iter_enumerated_mut() { - if bb == return_block { - continue; - } - let terminator = block.terminator_mut(); - if let TerminatorKind::Return = terminator.kind { - terminator.kind = TerminatorKind::Goto { target: return_block }; - } - } - - if has_extra_return_blocks { - simplify_cfg(tcx, body); - } - } -} diff --git a/tests/mir-opt/inline/inline_generator.main.Inline.diff b/tests/mir-opt/inline/inline_generator.main.Inline.diff index 6dde140093731..f27b64c305457 100644 --- a/tests/mir-opt/inline/inline_generator.main.Inline.diff +++ b/tests/mir-opt/inline/inline_generator.main.Inline.diff @@ -111,7 +111,7 @@ + discriminant(_1) = 0; // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 + _11 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]); // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 + discriminant((*_11)) = 3; // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 -+ goto -> bb1; // scope 6 at $DIR/inline_generator.rs:15:11: 15:39 ++ goto -> bb1; // scope 0 at $DIR/inline_generator.rs:15:11: 15:39 + } + + bb7: { @@ -122,7 +122,7 @@ + discriminant(_1) = 1; // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 + _12 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]); // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 + discriminant((*_12)) = 1; // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 -+ goto -> bb1; // scope 6 at $DIR/inline_generator.rs:15:41: 15:41 ++ goto -> bb1; // scope 0 at $DIR/inline_generator.rs:15:41: 15:41 + } + + bb8: { diff --git a/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff b/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff index e57544e09e2a0..eb2586502ce85 100644 --- a/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff +++ b/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff @@ -125,7 +125,7 @@ discriminant(_3) = 1; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL StorageDead(_14); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL StorageDead(_13); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL -- goto -> bb1; // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL +- goto -> bb1; // scope 0 at $SRC_DIR/core/src/result.rs:LL:COL + StorageDead(_4); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 + _5 = discriminant(_3); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 + switchInt(move _5) -> [0: bb1, 1: bb3, otherwise: bb2]; // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 @@ -147,7 +147,7 @@ discriminant(_3) = 0; // scope 6 at $SRC_DIR/core/src/result.rs:LL:COL StorageDead(_12); // scope 6 at $SRC_DIR/core/src/result.rs:LL:COL StorageDead(_11); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL -- goto -> bb1; // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL +- goto -> bb1; // scope 0 at $SRC_DIR/core/src/result.rs:LL:COL + StorageDead(_4); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 + _5 = discriminant(_3); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 + switchInt(move _5) -> [0: bb1, 1: bb3, otherwise: bb2]; // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 diff --git a/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir b/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir index 5251fe0fa67da..0fc7c6e219bbd 100644 --- a/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir +++ b/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir @@ -53,7 +53,7 @@ fn new(_1: Result) -> Result { ((_0 as Err).0: E) = move _8; // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51 discriminant(_0) = 1; // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51 StorageDead(_2); // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2 - goto -> bb6; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 + return; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 } bb4: { @@ -66,10 +66,6 @@ fn new(_1: Result) -> Result { ((_0 as Ok).0: T) = move _7; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6 discriminant(_0) = 0; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6 StorageDead(_2); // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2 - goto -> bb6; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 - } - - bb6: { return; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 } } diff --git a/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir b/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir index ec649dd126270..04624b308fb13 100644 --- a/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir +++ b/tests/mir-opt/try_identity_e2e.old.PreCodegen.after.mir @@ -23,7 +23,7 @@ fn old(_1: Result) -> Result { Deinit(_0); // scope 2 at $DIR/try_identity_e2e.rs:+4:30: +4:36 ((_0 as Err).0: E) = move _4; // scope 2 at $DIR/try_identity_e2e.rs:+4:30: +4:36 discriminant(_0) = 1; // scope 2 at $DIR/try_identity_e2e.rs:+4:30: +4:36 - goto -> bb4; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 + return; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 } bb2: { @@ -35,10 +35,6 @@ fn old(_1: Result) -> Result { Deinit(_0); // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +6:6 ((_0 as Ok).0: T) = move _3; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +6:6 discriminant(_0) = 0; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +6:6 - goto -> bb4; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 - } - - bb4: { return; // scope 0 at $DIR/try_identity_e2e.rs:+7:2: +7:2 } } From 98ec48d96792844834721af7c9525ac88eeb140f Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 8 Jan 2023 19:27:49 -0500 Subject: [PATCH 3/6] Codegen the return block last --- compiler/rustc_codegen_ssa/src/mir/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 01cf55cf0fc65..1612742c10568 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -260,6 +260,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // Apply debuginfo to the newly allocated locals. fx.debug_introduce_locals(&mut start_bx); + // If we have multiple return terminators, create a special return block. If this block is + // present, return terminator codegen will be a jump to it instead of a return. if mir .basic_blocks .iter() @@ -268,8 +270,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( > 1 { let llbb = Bx::append_block(fx.cx, fx.llfn, "return"); - let mut bx = Bx::build(fx.cx, llbb); - fx.codegen_return_terminator(&mut bx); fx.return_block = Some(llbb); } @@ -277,6 +277,14 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( for (bb, _) in traversal::reverse_postorder(&mir) { fx.codegen_block(bb); } + + // Actually codegen the return block. We need to delay until here because our return place + // might not be generated yet, and will only be generated by handling codegen for the + // assignment to it. + if let Some(llbb) = fx.return_block { + let mut bx = Bx::build(fx.cx, llbb); + fx.codegen_return_terminator(&mut bx); + } } /// Produces, for each argument, a `Value` pointing at the From e5ebdd8f541b8022249fab50e7aba8de1c8fd56b Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Wed, 11 Jan 2023 20:54:40 -0500 Subject: [PATCH 4/6] Experiment: Try letting DeduplicateBlocks delete redundant return blocks --- .../src/deduplicate_blocks.rs | 21 ++++++---- ..._line_doc_comment_2.DeduplicateBlocks.diff | 38 ++++++++----------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_mir_transform/src/deduplicate_blocks.rs b/compiler/rustc_mir_transform/src/deduplicate_blocks.rs index 909116a77f54f..418ffc8dc44b9 100644 --- a/compiler/rustc_mir_transform/src/deduplicate_blocks.rs +++ b/compiler/rustc_mir_transform/src/deduplicate_blocks.rs @@ -16,12 +16,22 @@ pub struct DeduplicateBlocks; impl<'tcx> MirPass<'tcx> for DeduplicateBlocks { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - sess.mir_opt_level() >= 4 + sess.mir_opt_level() >= 1 } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { debug!("Running DeduplicateBlocks on `{:?}`", body.source); - let duplicates = find_duplicates(body); + + let limit = if tcx.sess.mir_opt_level() < 3 { + 0 + } else { + // Basic blocks can get really big, so to avoid checking for duplicates in basic blocks + // that are unlikely to have duplicates, we stop early. The early bail number has been + // found experimentally by eprintln while compiling the crates in the rustc-perf suite. + 10 + }; + + let duplicates = find_duplicates(body, limit); let has_opts_to_apply = !duplicates.is_empty(); if has_opts_to_apply { @@ -54,7 +64,7 @@ impl<'tcx> MutVisitor<'tcx> for OptApplier<'tcx> { } } -fn find_duplicates(body: &Body<'_>) -> FxHashMap { +fn find_duplicates(body: &Body<'_>, limit: usize) -> FxHashMap { let mut duplicates = FxHashMap::default(); let bbs_to_go_through = @@ -72,10 +82,7 @@ fn find_duplicates(body: &Body<'_>) -> FxHashMap { // with replacement bb3. // When the duplicates are removed, we will end up with only bb3. for (bb, bbd) in body.basic_blocks.iter_enumerated().rev().filter(|(_, bbd)| !bbd.is_cleanup) { - // Basic blocks can get really big, so to avoid checking for duplicates in basic blocks - // that are unlikely to have duplicates, we stop early. The early bail number has been - // found experimentally by eprintln while compiling the crates in the rustc-perf suite. - if bbd.statements.len() > 10 { + if bbd.statements.len() > limit { continue; } diff --git a/tests/mir-opt/deduplicate_blocks.is_line_doc_comment_2.DeduplicateBlocks.diff b/tests/mir-opt/deduplicate_blocks.is_line_doc_comment_2.DeduplicateBlocks.diff index 3b1f81175cbfc..b13c984000326 100644 --- a/tests/mir-opt/deduplicate_blocks.is_line_doc_comment_2.DeduplicateBlocks.diff +++ b/tests/mir-opt/deduplicate_blocks.is_line_doc_comment_2.DeduplicateBlocks.diff @@ -44,8 +44,7 @@ } bb5: { -- switchInt((*_2)[3 of 4]) -> [47: bb11, otherwise: bb6]; // scope 0 at $DIR/deduplicate_blocks.rs:+1:5: +1:23 -+ switchInt((*_2)[3 of 4]) -> [47: bb10, otherwise: bb6]; // scope 0 at $DIR/deduplicate_blocks.rs:+1:5: +1:23 + switchInt((*_2)[3 of 4]) -> [47: bb11, otherwise: bb6]; // scope 0 at $DIR/deduplicate_blocks.rs:+1:5: +1:23 } bb6: { @@ -64,35 +63,30 @@ } bb9: { -- switchInt((*_2)[2 of 3]) -> [47: bb12, 33: bb13, otherwise: bb10]; // scope 0 at $DIR/deduplicate_blocks.rs:+1:5: +1:23 -+ switchInt((*_2)[2 of 3]) -> [47: bb11, 33: bb11, otherwise: bb10]; // scope 0 at $DIR/deduplicate_blocks.rs:+1:5: +1:23 + switchInt((*_2)[2 of 3]) -> [47: bb12, 33: bb13, otherwise: bb10]; // scope 0 at $DIR/deduplicate_blocks.rs:+1:5: +1:23 } bb10: { -- _0 = const false; // scope 0 at $DIR/deduplicate_blocks.rs:+5:14: +5:19 -- goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+5:14: +5:19 -- } -- -- bb11: { + _0 = const false; // scope 0 at $DIR/deduplicate_blocks.rs:+5:14: +5:19 + goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+5:14: +5:19 + } + + bb11: { _0 = const false; // scope 0 at $DIR/deduplicate_blocks.rs:+2:41: +2:46 -- goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+2:41: +2:46 -+ goto -> bb12; // scope 0 at $DIR/deduplicate_blocks.rs:+2:41: +2:46 + goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+2:41: +2:46 + } + + bb12: { + _0 = const true; // scope 0 at $DIR/deduplicate_blocks.rs:+3:35: +3:39 + goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+3:35: +3:39 } -- bb12: { -- _0 = const true; // scope 0 at $DIR/deduplicate_blocks.rs:+3:35: +3:39 -- goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+3:35: +3:39 -- } -- -- bb13: { -+ bb11: { + bb13: { _0 = const true; // scope 0 at $DIR/deduplicate_blocks.rs:+4:35: +4:39 -- goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+4:35: +4:39 -+ goto -> bb12; // scope 0 at $DIR/deduplicate_blocks.rs:+4:35: +4:39 + goto -> bb14; // scope 0 at $DIR/deduplicate_blocks.rs:+4:35: +4:39 } -- bb14: { -+ bb12: { + bb14: { StorageDead(_2); // scope 0 at $DIR/deduplicate_blocks.rs:+7:1: +7:2 return; // scope 0 at $DIR/deduplicate_blocks.rs:+7:2: +7:2 } From de672396675a9f5f4057e4ae1ee66765d3aced56 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 12 Jan 2023 19:12:03 -0500 Subject: [PATCH 5/6] Only deduplicate Resume terminators --- compiler/rustc_mir_transform/src/deduplicate_blocks.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_mir_transform/src/deduplicate_blocks.rs b/compiler/rustc_mir_transform/src/deduplicate_blocks.rs index 418ffc8dc44b9..fa01f063e28f1 100644 --- a/compiler/rustc_mir_transform/src/deduplicate_blocks.rs +++ b/compiler/rustc_mir_transform/src/deduplicate_blocks.rs @@ -86,6 +86,10 @@ fn find_duplicates(body: &Body<'_>, limit: usize) -> FxHashMap Date: Thu, 12 Jan 2023 19:53:48 -0500 Subject: [PATCH 6/6] bless --- .../try_identity_e2e.new.PreCodegen.after.mir | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir b/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir index 0fc7c6e219bbd..90044b40d0a6c 100644 --- a/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir +++ b/tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir @@ -26,7 +26,7 @@ fn new(_1: Result) -> Result { bb0: { StorageLive(_2); // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10 _3 = discriminant(_1); // scope 0 at $DIR/try_identity_e2e.rs:+3:19: +3:20 - switchInt(move _3) -> [0: bb2, 1: bb1, otherwise: bb4]; // scope 0 at $DIR/try_identity_e2e.rs:+3:13: +3:20 + switchInt(move _3) -> [0: bb3, 1: bb1, otherwise: bb2]; // scope 0 at $DIR/try_identity_e2e.rs:+3:13: +3:20 } bb1: { @@ -35,19 +35,23 @@ fn new(_1: Result) -> Result { ((_2 as Break).0: E) = move _5; // scope 2 at $DIR/try_identity_e2e.rs:+5:27: +5:48 discriminant(_2) = 1; // scope 2 at $DIR/try_identity_e2e.rs:+5:27: +5:48 _6 = discriminant(_2); // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10 - switchInt(move _6) -> [0: bb5, 1: bb3, otherwise: bb4]; // scope 0 at $DIR/try_identity_e2e.rs:+2:9: +7:10 + switchInt(move _6) -> [0: bb6, 1: bb4, otherwise: bb5]; // scope 0 at $DIR/try_identity_e2e.rs:+2:9: +7:10 } bb2: { + unreachable; // scope 0 at $DIR/try_identity_e2e.rs:+3:19: +3:20 + } + + bb3: { _4 = move ((_1 as Ok).0: T); // scope 0 at $DIR/try_identity_e2e.rs:+4:20: +4:21 Deinit(_2); // scope 1 at $DIR/try_identity_e2e.rs:+4:26: +4:50 ((_2 as Continue).0: T) = move _4; // scope 1 at $DIR/try_identity_e2e.rs:+4:26: +4:50 discriminant(_2) = 0; // scope 1 at $DIR/try_identity_e2e.rs:+4:26: +4:50 _6 = discriminant(_2); // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10 - switchInt(move _6) -> [0: bb5, 1: bb3, otherwise: bb4]; // scope 0 at $DIR/try_identity_e2e.rs:+2:9: +7:10 + switchInt(move _6) -> [0: bb6, 1: bb4, otherwise: bb5]; // scope 0 at $DIR/try_identity_e2e.rs:+2:9: +7:10 } - bb3: { + bb4: { _8 = move ((_2 as Break).0: E); // scope 0 at $DIR/try_identity_e2e.rs:+9:32: +9:33 Deinit(_0); // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51 ((_0 as Err).0: E) = move _8; // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51 @@ -56,11 +60,11 @@ fn new(_1: Result) -> Result { return; // scope 0 at $DIR/try_identity_e2e.rs:+12:2: +12:2 } - bb4: { + bb5: { unreachable; // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10 } - bb5: { + bb6: { _7 = move ((_2 as Continue).0: T); // scope 0 at $DIR/try_identity_e2e.rs:+8:35: +8:36 Deinit(_0); // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6 ((_0 as Ok).0: T) = move _7; // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6