diff --git a/compiler/rustc_mir_transform/src/branch_duplicator.rs b/compiler/rustc_mir_transform/src/branch_duplicator.rs new file mode 100644 index 0000000000000..db390333cc213 --- /dev/null +++ b/compiler/rustc_mir_transform/src/branch_duplicator.rs @@ -0,0 +1,86 @@ +use std::mem; + +use rustc_middle::mir::*; +use rustc_middle::ty::TyCtxt; +use tracing::{debug, instrument, trace}; + +pub(super) struct BranchDuplicator; + +impl<'tcx> crate::MirPass<'tcx> for BranchDuplicator { + fn is_enabled(&self, sess: &rustc_session::Session) -> bool { + sess.mir_opt_level() >= 2 + } + + #[instrument(skip_all level = "debug")] + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + let def_id = body.source.def_id(); + debug!(?def_id); + + // Optimizing coroutines creates query cycles. + if tcx.is_coroutine(def_id) { + trace!("Skipped for coroutine {:?}", def_id); + return; + } + + let is_branch = |targets: &SwitchTargets| { + targets.all_targets().len() == 2 + || (targets.all_values().len() == 2 + && body.basic_blocks[targets.otherwise()].is_empty_unreachable()) + }; + + let mut candidates = Vec::new(); + for (bb, bbdata) in body.basic_blocks.iter_enumerated() { + if let TerminatorKind::SwitchInt { targets, .. } = &bbdata.terminator().kind + && is_branch(targets) + && let Ok(preds) = + <[BasicBlock; 2]>::try_from(body.basic_blocks.predecessors()[bb].as_slice()) + && preds.iter().copied().all(|p| { + matches!(body.basic_blocks[p].terminator().kind, TerminatorKind::Goto { .. }) + }) + && bbdata.statements.iter().all(|x| is_negligible(&x.kind)) + { + candidates.push((bb, preds)); + } + } + + if candidates.is_empty() { + return; + } + + let basic_blocks = body.basic_blocks.as_mut(); + for (bb, [p0, p1]) in candidates { + let bbdata = &mut basic_blocks[bb]; + let statements = mem::take(&mut bbdata.statements); + let unreachable = Terminator { + source_info: bbdata.terminator().source_info, + kind: TerminatorKind::Unreachable, + }; + let terminator = mem::replace(bbdata.terminator_mut(), unreachable); + + let pred0data = &mut basic_blocks[p0]; + pred0data.statements.extend(statements.iter().cloned()); + *pred0data.terminator_mut() = terminator.clone(); + + let pred1data = &mut basic_blocks[p1]; + pred1data.statements.extend(statements); + *pred1data.terminator_mut() = terminator; + } + } + + fn is_required(&self) -> bool { + false + } +} + +fn is_negligible<'tcx>(stmt: &StatementKind<'tcx>) -> bool { + use Rvalue::*; + use StatementKind::*; + match stmt { + StorageLive(..) | StorageDead(..) => true, + Assign(place_and_rvalue) => match &place_and_rvalue.1 { + Ref(..) | RawPtr(..) | Discriminant(..) | NullaryOp(..) => true, + _ => false, + }, + _ => false, + } +} diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 04c9375b83176..84f5bba5f7eee 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -118,6 +118,7 @@ declare_passes! { mod add_moves_for_packed_drops : AddMovesForPackedDrops; mod add_retag : AddRetag; mod add_subtyping_projections : Subtyper; + mod branch_duplicator: BranchDuplicator; mod check_inline : CheckForceInline; mod check_call_recursion : CheckCallRecursion, CheckDropRecursion; mod check_alignment : CheckAlignment; @@ -670,6 +671,9 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { &unreachable_enum_branching::UnreachableEnumBranching, &unreachable_prop::UnreachablePropagation, &o1(simplify::SimplifyCfg::AfterUnreachableEnumBranching), + // If we inlined something returning `Option` or `Result`, + // duplicating the discriminant branches on those helps later passes. + &branch_duplicator::BranchDuplicator, // Inlining may have introduced a lot of redundant code and a large move pattern. // Now, we need to shrink the generated MIR. &ref_prop::ReferencePropagation, diff --git a/tests/mir-opt/const_goto_const_eval_fail.f.JumpThreading.diff b/tests/mir-opt/const_goto_const_eval_fail.f.JumpThreading.diff index 086abeba0f848..d417db27ac0af 100644 --- a/tests/mir-opt/const_goto_const_eval_fail.f.JumpThreading.diff +++ b/tests/mir-opt/const_goto_const_eval_fail.f.JumpThreading.diff @@ -12,36 +12,27 @@ bb1: { _1 = const true; -- goto -> bb3; -+ goto -> bb7; + goto -> bb3; } bb2: { _1 = const B; - goto -> bb3; + switchInt(copy _1) -> [0: bb4, otherwise: bb3]; } bb3: { - switchInt(copy _1) -> [0: bb5, otherwise: bb4]; - } - - bb4: { _0 = const 2_u64; - goto -> bb6; + goto -> bb5; } - bb5: { + bb4: { _0 = const 1_u64; - goto -> bb6; + goto -> bb5; } - bb6: { + bb5: { StorageDead(_1); return; -+ } -+ -+ bb7: { -+ goto -> bb4; } } diff --git a/tests/mir-opt/pre-codegen/checked_ops.rs b/tests/mir-opt/pre-codegen/checked_ops.rs index 56f8e3f833845..b9dcd845e55ef 100644 --- a/tests/mir-opt/pre-codegen/checked_ops.rs +++ b/tests/mir-opt/pre-codegen/checked_ops.rs @@ -1,4 +1,3 @@ -// skip-filecheck //@ compile-flags: -O -Zmir-opt-level=2 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY @@ -8,10 +7,36 @@ // EMIT_MIR checked_ops.step_forward.PreCodegen.after.mir pub fn step_forward(x: u16, n: usize) -> u16 { // This uses `u16` so that the conversion to usize is always widening. + + // CHECK-LABEL: fn step_forward + // CHECK: inlined{{.+}}forward std::iter::Step::forward(x, n) } // EMIT_MIR checked_ops.checked_shl.PreCodegen.after.mir pub fn checked_shl(x: u32, rhs: u32) -> Option { + // CHECK-LABEL: fn checked_shl + // CHECK: [[TEMP:_[0-9]+]] = ShlUnchecked(copy _1, copy _2) + // CHECK: _0 = Option::::Some({{move|copy}} [[TEMP]]) x.checked_shl(rhs) } + +// EMIT_MIR checked_ops.use_checked_sub.PreCodegen.after.mir +// EMIT_MIR checked_ops.use_checked_sub.BranchDuplicator.diff +// EMIT_MIR checked_ops.use_checked_sub.GVN.diff +pub fn use_checked_sub(x: u32, rhs: u32) { + // We want this to be equivalent to open-coding it, leaving no `Option`s around. + + // CHECK-LABEL: fn use_checked_sub + // FIXME-CHECK-NOT: let{{.+}}Option + // CHECK: inlined{{.+}}u32{{.+}}checked_sub + // CHECK: [[DELTA:_[0-9]+]] = SubUnchecked(copy _1, copy _2) + // FIXME-CHECK: do_something({{move|copy}} [[DELTA]]) + if let Some(delta) = x.checked_sub(rhs) { + do_something(delta); + } +} + +unsafe extern "Rust" { + safe fn do_something(_: u32); +} diff --git a/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-abort.diff b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-abort.diff new file mode 100644 index 0000000000000..c2d7e853662b7 --- /dev/null +++ b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-abort.diff @@ -0,0 +1,91 @@ +- // MIR for `use_checked_sub` before BranchDuplicator ++ // MIR for `use_checked_sub` after BranchDuplicator + + fn use_checked_sub(_1: u32, _2: u32) -> () { + debug x => _1; + debug rhs => _2; + let mut _0: (); + let mut _3: std::option::Option; + let mut _4: u32; + let mut _5: u32; + let mut _6: isize; + let _8: (); + let mut _9: u32; + scope 1 { + debug delta => _7; + let _7: u32; + scope 2 (inlined core::num::::checked_sub) { + let mut _10: bool; + let mut _11: u32; + } + } + + bb0: { + StorageLive(_3); + StorageLive(_4); + _4 = copy _1; + StorageLive(_5); + _5 = copy _2; + StorageLive(_10); + _10 = Lt(copy _4, copy _5); + switchInt(move _10) -> [0: bb5, otherwise: bb4]; + } + + bb1: { + StorageLive(_7); + _7 = copy ((_3 as Some).0: u32); + StorageLive(_9); + _9 = copy _7; + _8 = do_something(move _9) -> [return: bb2, unwind unreachable]; + } + + bb2: { + StorageDead(_9); + StorageDead(_7); + goto -> bb3; + } + + bb3: { + StorageDead(_3); + return; + } + + bb4: { + _3 = const Option::::None; +- goto -> bb6; ++ StorageDead(_10); ++ StorageDead(_5); ++ StorageDead(_4); ++ _6 = discriminant(_3); ++ switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; + } + + bb5: { + StorageLive(_11); + _11 = SubUnchecked(copy _4, copy _5); + _3 = Option::::Some(move _11); + StorageDead(_11); +- goto -> bb6; +- } +- +- bb6: { + StorageDead(_10); + StorageDead(_5); + StorageDead(_4); + _6 = discriminant(_3); + switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; ++ } ++ ++ bb6: { ++ unreachable; + } + + bb7: { + unreachable; + } + } + + ALLOC0 (size: 8, align: 4) { + 00 00 00 00 __ __ __ __ │ ....░░░░ + } + diff --git a/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-unwind.diff b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-unwind.diff new file mode 100644 index 0000000000000..a3a6b160aa99a --- /dev/null +++ b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-unwind.diff @@ -0,0 +1,91 @@ +- // MIR for `use_checked_sub` before BranchDuplicator ++ // MIR for `use_checked_sub` after BranchDuplicator + + fn use_checked_sub(_1: u32, _2: u32) -> () { + debug x => _1; + debug rhs => _2; + let mut _0: (); + let mut _3: std::option::Option; + let mut _4: u32; + let mut _5: u32; + let mut _6: isize; + let _8: (); + let mut _9: u32; + scope 1 { + debug delta => _7; + let _7: u32; + scope 2 (inlined core::num::::checked_sub) { + let mut _10: bool; + let mut _11: u32; + } + } + + bb0: { + StorageLive(_3); + StorageLive(_4); + _4 = copy _1; + StorageLive(_5); + _5 = copy _2; + StorageLive(_10); + _10 = Lt(copy _4, copy _5); + switchInt(move _10) -> [0: bb5, otherwise: bb4]; + } + + bb1: { + StorageLive(_7); + _7 = copy ((_3 as Some).0: u32); + StorageLive(_9); + _9 = copy _7; + _8 = do_something(move _9) -> [return: bb2, unwind continue]; + } + + bb2: { + StorageDead(_9); + StorageDead(_7); + goto -> bb3; + } + + bb3: { + StorageDead(_3); + return; + } + + bb4: { + _3 = const Option::::None; +- goto -> bb6; ++ StorageDead(_10); ++ StorageDead(_5); ++ StorageDead(_4); ++ _6 = discriminant(_3); ++ switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; + } + + bb5: { + StorageLive(_11); + _11 = SubUnchecked(copy _4, copy _5); + _3 = Option::::Some(move _11); + StorageDead(_11); +- goto -> bb6; +- } +- +- bb6: { + StorageDead(_10); + StorageDead(_5); + StorageDead(_4); + _6 = discriminant(_3); + switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; ++ } ++ ++ bb6: { ++ unreachable; + } + + bb7: { + unreachable; + } + } + + ALLOC0 (size: 8, align: 4) { + 00 00 00 00 __ __ __ __ │ ....░░░░ + } + diff --git a/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.GVN.panic-abort.diff b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.GVN.panic-abort.diff new file mode 100644 index 0000000000000..f60aa8f7ddae6 --- /dev/null +++ b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.GVN.panic-abort.diff @@ -0,0 +1,91 @@ +- // MIR for `use_checked_sub` before GVN ++ // MIR for `use_checked_sub` after GVN + + fn use_checked_sub(_1: u32, _2: u32) -> () { + debug x => _1; + debug rhs => _2; + let mut _0: (); + let mut _3: std::option::Option; + let mut _4: u32; + let mut _5: u32; + let mut _6: isize; + let _8: (); + let mut _9: u32; + scope 1 { + debug delta => _7; + let _7: u32; + scope 2 (inlined core::num::::checked_sub) { + let mut _10: bool; + let mut _11: u32; + } + } + + bb0: { + StorageLive(_3); + StorageLive(_4); + _4 = copy _1; + StorageLive(_5); + _5 = copy _2; + StorageLive(_10); +- _10 = Lt(copy _4, copy _5); ++ _10 = Lt(copy _1, copy _2); + switchInt(move _10) -> [0: bb5, otherwise: bb4]; + } + + bb1: { +- StorageLive(_7); ++ nop; + _7 = copy ((_3 as Some).0: u32); + StorageLive(_9); + _9 = copy _7; +- _8 = do_something(move _9) -> [return: bb2, unwind unreachable]; ++ _8 = do_something(copy _7) -> [return: bb2, unwind unreachable]; + } + + bb2: { + StorageDead(_9); +- StorageDead(_7); ++ nop; + goto -> bb3; + } + + bb3: { + StorageDead(_3); + return; + } + + bb4: { + _3 = const Option::::None; + StorageDead(_10); + StorageDead(_5); + StorageDead(_4); + _6 = discriminant(_3); + switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; + } + + bb5: { + StorageLive(_11); +- _11 = SubUnchecked(copy _4, copy _5); ++ _11 = SubUnchecked(copy _1, copy _2); + _3 = Option::::Some(move _11); + StorageDead(_11); + StorageDead(_10); + StorageDead(_5); + StorageDead(_4); + _6 = discriminant(_3); + switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; + } + + bb6: { + unreachable; + } + + bb7: { + unreachable; + } + } + + ALLOC0 (size: 8, align: 4) { + 00 00 00 00 __ __ __ __ │ ....░░░░ + } + diff --git a/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.GVN.panic-unwind.diff b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.GVN.panic-unwind.diff new file mode 100644 index 0000000000000..1a9f472364a8d --- /dev/null +++ b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.GVN.panic-unwind.diff @@ -0,0 +1,91 @@ +- // MIR for `use_checked_sub` before GVN ++ // MIR for `use_checked_sub` after GVN + + fn use_checked_sub(_1: u32, _2: u32) -> () { + debug x => _1; + debug rhs => _2; + let mut _0: (); + let mut _3: std::option::Option; + let mut _4: u32; + let mut _5: u32; + let mut _6: isize; + let _8: (); + let mut _9: u32; + scope 1 { + debug delta => _7; + let _7: u32; + scope 2 (inlined core::num::::checked_sub) { + let mut _10: bool; + let mut _11: u32; + } + } + + bb0: { + StorageLive(_3); + StorageLive(_4); + _4 = copy _1; + StorageLive(_5); + _5 = copy _2; + StorageLive(_10); +- _10 = Lt(copy _4, copy _5); ++ _10 = Lt(copy _1, copy _2); + switchInt(move _10) -> [0: bb5, otherwise: bb4]; + } + + bb1: { +- StorageLive(_7); ++ nop; + _7 = copy ((_3 as Some).0: u32); + StorageLive(_9); + _9 = copy _7; +- _8 = do_something(move _9) -> [return: bb2, unwind continue]; ++ _8 = do_something(copy _7) -> [return: bb2, unwind continue]; + } + + bb2: { + StorageDead(_9); +- StorageDead(_7); ++ nop; + goto -> bb3; + } + + bb3: { + StorageDead(_3); + return; + } + + bb4: { + _3 = const Option::::None; + StorageDead(_10); + StorageDead(_5); + StorageDead(_4); + _6 = discriminant(_3); + switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; + } + + bb5: { + StorageLive(_11); +- _11 = SubUnchecked(copy _4, copy _5); ++ _11 = SubUnchecked(copy _1, copy _2); + _3 = Option::::Some(move _11); + StorageDead(_11); + StorageDead(_10); + StorageDead(_5); + StorageDead(_4); + _6 = discriminant(_3); + switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7]; + } + + bb6: { + unreachable; + } + + bb7: { + unreachable; + } + } + + ALLOC0 (size: 8, align: 4) { + 00 00 00 00 __ __ __ __ │ ....░░░░ + } + diff --git a/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.PreCodegen.after.panic-abort.mir new file mode 100644 index 0000000000000..3c475cd403091 --- /dev/null +++ b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.PreCodegen.after.panic-abort.mir @@ -0,0 +1,44 @@ +// MIR for `use_checked_sub` after PreCodegen + +fn use_checked_sub(_1: u32, _2: u32) -> () { + debug x => _1; + debug rhs => _2; + let mut _0: (); + let mut _5: std::option::Option; + let _7: (); + scope 1 { + debug delta => _6; + let _6: u32; + scope 2 (inlined core::num::::checked_sub) { + let mut _3: bool; + let mut _4: u32; + } + } + + bb0: { + StorageLive(_5); + StorageLive(_3); + _3 = Lt(copy _1, copy _2); + switchInt(move _3) -> [0: bb1, otherwise: bb2]; + } + + bb1: { + StorageLive(_4); + _4 = SubUnchecked(copy _1, copy _2); + _5 = Option::::Some(move _4); + StorageDead(_4); + StorageDead(_3); + _6 = copy ((_5 as Some).0: u32); + _7 = do_something(move _6) -> [return: bb3, unwind unreachable]; + } + + bb2: { + StorageDead(_3); + goto -> bb3; + } + + bb3: { + StorageDead(_5); + return; + } +} diff --git a/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.PreCodegen.after.panic-unwind.mir new file mode 100644 index 0000000000000..3ef09764b1c5b --- /dev/null +++ b/tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.PreCodegen.after.panic-unwind.mir @@ -0,0 +1,44 @@ +// MIR for `use_checked_sub` after PreCodegen + +fn use_checked_sub(_1: u32, _2: u32) -> () { + debug x => _1; + debug rhs => _2; + let mut _0: (); + let mut _5: std::option::Option; + let _7: (); + scope 1 { + debug delta => _6; + let _6: u32; + scope 2 (inlined core::num::::checked_sub) { + let mut _3: bool; + let mut _4: u32; + } + } + + bb0: { + StorageLive(_5); + StorageLive(_3); + _3 = Lt(copy _1, copy _2); + switchInt(move _3) -> [0: bb1, otherwise: bb2]; + } + + bb1: { + StorageLive(_4); + _4 = SubUnchecked(copy _1, copy _2); + _5 = Option::::Some(move _4); + StorageDead(_4); + StorageDead(_3); + _6 = copy ((_5 as Some).0: u32); + _7 = do_something(move _6) -> [return: bb3, unwind continue]; + } + + bb2: { + StorageDead(_3); + goto -> bb3; + } + + bb3: { + StorageDead(_5); + return; + } +} diff --git a/tests/mir-opt/separate_const_switch.identity.JumpThreading.diff b/tests/mir-opt/separate_const_switch.identity.JumpThreading.diff index ce9d812701a8f..93f9783ca5e3b 100644 --- a/tests/mir-opt/separate_const_switch.identity.JumpThreading.diff +++ b/tests/mir-opt/separate_const_switch.identity.JumpThreading.diff @@ -42,7 +42,7 @@ StorageLive(_7); StorageLive(_8); _6 = discriminant(_1); - switchInt(move _6) -> [0: bb6, 1: bb5, otherwise: bb1]; + switchInt(move _6) -> [0: bb5, 1: bb4, otherwise: bb1]; } bb1: { @@ -65,36 +65,26 @@ } bb4: { - StorageDead(_8); - StorageDead(_7); - StorageDead(_6); - _3 = discriminant(_2); -- switchInt(move _3) -> [0: bb2, 1: bb3, otherwise: bb1]; -+ goto -> bb2; - } - - bb5: { _8 = copy ((_1 as Err).0: i32); StorageLive(_9); _9 = Result::::Err(copy _8); _2 = ControlFlow::, i32>::Break(move _9); StorageDead(_9); -- goto -> bb4; -+ goto -> bb7; + StorageDead(_8); + StorageDead(_7); + StorageDead(_6); + _3 = const 1_isize; + goto -> bb3; } - bb6: { + bb5: { _7 = copy ((_1 as Ok).0: i32); _2 = ControlFlow::, i32>::Continue(copy _7); - goto -> bb4; -+ } -+ -+ bb7: { -+ StorageDead(_8); -+ StorageDead(_7); -+ StorageDead(_6); -+ _3 = discriminant(_2); -+ goto -> bb3; + StorageDead(_8); + StorageDead(_7); + StorageDead(_6); + _3 = const 0_isize; + goto -> bb2; } } diff --git a/tests/mir-opt/separate_const_switch.too_complex.JumpThreading.diff b/tests/mir-opt/separate_const_switch.too_complex.JumpThreading.diff index c88c63e0c1334..aa0b4ac9e29f5 100644 --- a/tests/mir-opt/separate_const_switch.too_complex.JumpThreading.diff +++ b/tests/mir-opt/separate_const_switch.too_complex.JumpThreading.diff @@ -37,44 +37,34 @@ bb2: { _5 = copy ((_1 as Err).0: usize); _2 = ControlFlow::::Break(copy _5); -- goto -> bb4; -+ goto -> bb8; + _6 = const 1_isize; + goto -> bb4; } bb3: { _4 = copy ((_1 as Ok).0: i32); _2 = ControlFlow::::Continue(copy _4); - goto -> bb4; + _6 = const 0_isize; + goto -> bb5; } bb4: { - _6 = discriminant(_2); -- switchInt(move _6) -> [0: bb6, 1: bb5, otherwise: bb1]; -+ goto -> bb6; - } - - bb5: { StorageLive(_8); _8 = copy ((_2 as Break).0: usize); _0 = const Option::::None; StorageDead(_8); - goto -> bb7; + goto -> bb6; } - bb6: { + bb5: { _7 = copy ((_2 as Continue).0: i32); _0 = Option::::Some(copy _7); - goto -> bb7; + goto -> bb6; } - bb7: { + bb6: { StorageDead(_2); return; -+ } -+ -+ bb8: { -+ _6 = discriminant(_2); -+ goto -> bb5; } }