diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index dfd07f0fb1681..ca2491d4e5e3d 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -189,7 +189,8 @@ declare_passes! { BeforeConstProp, AfterGVN, Final - }; + }, + RemoveRedundantSwitch; mod simplify_branches : SimplifyConstCondition { AfterConstProp, Final @@ -675,6 +676,8 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { &check_null::CheckNull, // Before inlining: trim down MIR with passes to reduce inlining work. + // Remove some redundant `SwitchInt` terminators. + &simplify::RemoveRedundantSwitch, // Has to be done before inlining, otherwise actual call will be almost always inlined. // Also simple, so can just do first. &lower_slice_len::LowerSliceLenCalls, diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 84905f4a400f3..66b93a4149c34 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -26,6 +26,13 @@ //! Here the block (`{ return; }`) has the return type `char`, rather than `()`, but the MIR we //! naively generate still contains the `_a = ()` write in the unreachable block "after" the //! return. +//! +//! **WARNING**: `SimplifyCfg` is one of the few optimizations that runs on built and analysis +//! MIR, and so its effects may affect the type-checking, borrow-checking, and other analysis. +//! We must be extremely careful to only apply optimizations that preserve UB and all +//! non-determinism, since changes here can affect which programs compile in an insta-stable way. +//! The normal logic that a program with UB can be changed to do anything does not apply to +//! pre-"runtime" MIR! use rustc_index::{Idx, IndexSlice, IndexVec}; use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor}; @@ -144,7 +151,6 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { merged_blocks.clear(); while inner_changed { inner_changed = false; - inner_changed |= self.simplify_branch(&mut terminator); inner_changed |= self.merge_successor(&mut merged_blocks, &mut terminator); changed |= inner_changed; } @@ -251,32 +257,6 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { true } - // turn a branch with all successors identical to a goto - fn simplify_branch(&mut self, terminator: &mut Terminator<'tcx>) -> bool { - match terminator.kind { - TerminatorKind::SwitchInt { .. } => {} - _ => return false, - }; - - let first_succ = { - if let Some(first_succ) = terminator.successors().next() { - if terminator.successors().all(|s| s == first_succ) { - let count = terminator.successors().count(); - self.pred_count[first_succ] -= (count - 1) as u32; - first_succ - } else { - return false; - } - } else { - return false; - } - }; - - debug!("simplifying branch {:?}", terminator); - terminator.kind = TerminatorKind::Goto { target: first_succ }; - true - } - fn strip_nops(&mut self) { for blk in self.basic_blocks.iter_mut() { blk.statements.retain(|stmt| !matches!(stmt.kind, StatementKind::Nop)) @@ -615,3 +595,42 @@ impl<'tcx> MutVisitor<'tcx> for LocalUpdater<'tcx> { *l = self.map[*l].unwrap(); } } + +pub struct RemoveRedundantSwitch; + +impl<'tcx> crate::MirPass<'tcx> for RemoveRedundantSwitch { + fn run_pass(&self, _tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + loop { + let mut should_simplify = false; + + for block in body.basic_blocks_mut() { + let TerminatorKind::SwitchInt { discr: _, targets } = &block.terminator().kind + else { + continue; + }; + let Some((first_succ, rest)) = targets.all_targets().split_first() else { + continue; + }; + if !rest.iter().all(|succ| succ == first_succ) { + continue; + } + block.terminator_mut().kind = TerminatorKind::Goto { target: *first_succ }; + should_simplify = true; + } + + if should_simplify { + simplify_cfg(body); + } else { + break; + } + } + } + + fn is_enabled(&self, sess: &rustc_session::Session) -> bool { + sess.mir_opt_level() >= 1 + } + + fn is_required(&self) -> bool { + false + } +} diff --git a/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs b/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs index bd00823073139..998d0da2d6fe6 100644 --- a/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs +++ b/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs @@ -55,7 +55,9 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyComparisonIntegral { const FALSE: u128 = 0; let mut new_targets = opt.targets; - let first_value = new_targets.iter().next().unwrap().0; + let Some((first_value, _)) = new_targets.iter().next() else { + continue; + }; let first_is_false_target = first_value == FALSE; match opt.op { BinOp::Eq => { diff --git a/src/tools/miri/tests/fail/read_from_trivial_switch.rs b/src/tools/miri/tests/fail/read_from_trivial_switch.rs new file mode 100644 index 0000000000000..d34b1cd582009 --- /dev/null +++ b/src/tools/miri/tests/fail/read_from_trivial_switch.rs @@ -0,0 +1,14 @@ +// Ensure that we don't optimize out `SwitchInt` reads even if that terminator +// branches to the same basic block on every target, since the operand may have +// side-effects that affect analysis of the MIR. +// +// See . + +use std::mem::MaybeUninit; + +fn main() { + let uninit: MaybeUninit = MaybeUninit::uninit(); + let bad_ref: &i32 = unsafe { uninit.assume_init_ref() }; + let &(0 | _) = bad_ref; + //~^ ERROR: Undefined Behavior: using uninitialized data, but this operation requires initialized memory +} diff --git a/src/tools/miri/tests/fail/read_from_trivial_switch.stderr b/src/tools/miri/tests/fail/read_from_trivial_switch.stderr new file mode 100644 index 0000000000000..6b3d4539b9681 --- /dev/null +++ b/src/tools/miri/tests/fail/read_from_trivial_switch.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory + --> tests/fail/read_from_trivial_switch.rs:LL:CC + | +LL | let &(0 | _) = bad_ref; + | ^^^^^^^^ using uninitialized data, but this operation requires initialized memory + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at tests/fail/read_from_trivial_switch.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir index d52241b459ebd..2a965fe67b9e7 100644 --- a/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir @@ -24,43 +24,47 @@ fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { bb1: { _0 = const 0_u32; - goto -> bb10; + goto -> bb11; } bb2: { - _2 = discriminant((_1.2: std::option::Option)); - switchInt(move _2) -> [0: bb4, 1: bb3, otherwise: bb1]; + switchInt(copy (_1.1: bool)) -> [0: bb3, otherwise: bb3]; } bb3: { - switchInt(copy (((_1.2: std::option::Option) as Some).0: i32)) -> [1: bb4, 8: bb4, otherwise: bb1]; + _2 = discriminant((_1.2: std::option::Option)); + switchInt(move _2) -> [0: bb5, 1: bb4, otherwise: bb1]; } bb4: { - _5 = Le(const 6_u32, copy (_1.3: u32)); - switchInt(move _5) -> [0: bb5, otherwise: bb7]; + switchInt(copy (((_1.2: std::option::Option) as Some).0: i32)) -> [1: bb5, 8: bb5, otherwise: bb1]; } bb5: { - _3 = Le(const 13_u32, copy (_1.3: u32)); - switchInt(move _3) -> [0: bb1, otherwise: bb6]; + _5 = Le(const 6_u32, copy (_1.3: u32)); + switchInt(move _5) -> [0: bb6, otherwise: bb8]; } bb6: { - _4 = Le(copy (_1.3: u32), const 16_u32); - switchInt(move _4) -> [0: bb1, otherwise: bb8]; + _3 = Le(const 13_u32, copy (_1.3: u32)); + switchInt(move _3) -> [0: bb1, otherwise: bb7]; } bb7: { - _6 = Le(copy (_1.3: u32), const 9_u32); - switchInt(move _6) -> [0: bb5, otherwise: bb8]; + _4 = Le(copy (_1.3: u32), const 16_u32); + switchInt(move _4) -> [0: bb1, otherwise: bb9]; } bb8: { - falseEdge -> [real: bb9, imaginary: bb1]; + _6 = Le(copy (_1.3: u32), const 9_u32); + switchInt(move _6) -> [0: bb6, otherwise: bb9]; } bb9: { + falseEdge -> [real: bb10, imaginary: bb1]; + } + + bb10: { StorageLive(_7); _7 = copy (_1.0: u32); StorageLive(_8); @@ -74,10 +78,10 @@ fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { StorageDead(_9); StorageDead(_8); StorageDead(_7); - goto -> bb10; + goto -> bb11; } - bb10: { + bb11: { return; } } diff --git a/tests/mir-opt/early_otherwise_branch_unwind.poll.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_unwind.poll.EarlyOtherwiseBranch.diff index b6450e43b09e6..ecef2b8026e1c 100644 --- a/tests/mir-opt/early_otherwise_branch_unwind.poll.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_unwind.poll.EarlyOtherwiseBranch.diff @@ -12,6 +12,7 @@ let mut _7: bool; let mut _8: bool; let mut _9: isize; + let mut _10: isize; scope 1 { debug _trailers => _5; } @@ -99,12 +100,13 @@ } bb15: { - goto -> bb14; + _9 = discriminant(((((_1 as Ready).0: std::result::Result>, u8>) as Ok).0: std::option::Option>)); + switchInt(move _9) -> [1: bb14, otherwise: bb14]; } bb16: { - _9 = discriminant(((_1 as Ready).0: std::result::Result>, u8>)); - switchInt(move _9) -> [0: bb13, otherwise: bb12]; + _10 = discriminant(((_1 as Ready).0: std::result::Result>, u8>)); + switchInt(move _10) -> [0: bb13, otherwise: bb12]; } bb17: { @@ -116,7 +118,7 @@ } bb19 (cleanup): { - goto -> bb9; + switchInt(copy _2) -> [1: bb9, otherwise: bb9]; } bb20 (cleanup): { diff --git a/tests/mir-opt/early_otherwise_branch_unwind.unwind.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_unwind.unwind.EarlyOtherwiseBranch.diff index 2b2c007e082a7..7ba09ac487cf6 100644 --- a/tests/mir-opt/early_otherwise_branch_unwind.unwind.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_unwind.unwind.EarlyOtherwiseBranch.diff @@ -11,6 +11,7 @@ let mut _6: bool; let mut _7: bool; let mut _8: isize; + let mut _9: isize; scope 1 { debug _v => _5; } @@ -92,12 +93,13 @@ } bb15: { - goto -> bb14; + _8 = discriminant(((((_1 as Some).0: std::option::Option>) as Some).0: std::option::Option)); + switchInt(move _8) -> [1: bb14, otherwise: bb14]; } bb16: { - _8 = discriminant(((_1 as Some).0: std::option::Option>)); - switchInt(move _8) -> [1: bb13, otherwise: bb12]; + _9 = discriminant(((_1 as Some).0: std::option::Option>)); + switchInt(move _9) -> [1: bb13, otherwise: bb12]; } bb17: { @@ -109,7 +111,7 @@ } bb19 (cleanup): { - goto -> bb9; + switchInt(copy _2) -> [1: bb9, otherwise: bb9]; } bb20 (cleanup): { diff --git a/tests/mir-opt/gvn_copy_aggregate.remove_storage_dead.GVN.diff b/tests/mir-opt/gvn_copy_aggregate.remove_storage_dead.GVN.diff index c9cfc7efcd1a6..50bf0a6c259d0 100644 --- a/tests/mir-opt/gvn_copy_aggregate.remove_storage_dead.GVN.diff +++ b/tests/mir-opt/gvn_copy_aggregate.remove_storage_dead.GVN.diff @@ -39,6 +39,10 @@ + _2 = copy _5; + nop; _7 = discriminant(_3); + switchInt(move _7) -> bb2; + } + + bb2: { - StorageDead(_3); + nop; StorageLive(_6); diff --git a/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff b/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff index e28809f38afa6..9aa0c16ea83b5 100644 --- a/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff +++ b/tests/mir-opt/multiple_return_terminators.test.MultipleReturnTerminators.diff @@ -6,6 +6,10 @@ let mut _0: (); bb0: { + switchInt(copy _1) -> bb1; + } + + bb1: { return; } } diff --git a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir index fa6c6ce8e5750..1197ce220d8ce 100644 --- a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir +++ b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -33,6 +33,10 @@ fn unwrap(_1: Option) -> T { _0 = move _3; StorageDead(_3); _5 = discriminant(_1); + switchInt(move _5) -> [1: bb4, otherwise: bb4]; + } + + bb4: { return; } } diff --git a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 54fec3c0f9847..8f39d275ae34e 100644 --- a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir +++ b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -24,7 +24,7 @@ fn unwrap(_1: Option) -> T { bb2: { StorageLive(_4); - _4 = begin_panic::<&str>(const "explicit panic") -> bb4; + _4 = begin_panic::<&str>(const "explicit panic") -> bb6; } bb3: { @@ -33,11 +33,19 @@ fn unwrap(_1: Option) -> T { _0 = move _3; StorageDead(_3); _5 = discriminant(_1); - return; + switchInt(move _5) -> [1: bb5, otherwise: bb5]; } bb4 (cleanup): { - _7 = discriminant(_1); resume; } + + bb5: { + return; + } + + bb6 (cleanup): { + _7 = discriminant(_1); + switchInt(move _7) -> [1: bb4, otherwise: bb4]; + } } diff --git a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir index 889ff6f9f5e2b..be0931eaa61d7 100644 --- a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir @@ -14,7 +14,7 @@ fn single_switchint() -> () { } bb1: { - switchInt(copy (_2.0: i32)) -> [3: bb8, 4: bb8, otherwise: bb7]; + switchInt(copy (_2.0: i32)) -> [3: bb9, 4: bb9, otherwise: bb8]; } bb2: { @@ -22,7 +22,7 @@ fn single_switchint() -> () { } bb3: { - falseEdge -> [real: bb12, imaginary: bb4]; + falseEdge -> [real: bb14, imaginary: bb4]; } bb4: { @@ -30,43 +30,51 @@ fn single_switchint() -> () { } bb5: { - falseEdge -> [real: bb11, imaginary: bb6]; + falseEdge -> [real: bb13, imaginary: bb6]; } bb6: { - falseEdge -> [real: bb10, imaginary: bb1]; + switchInt(copy (_2.1: bool)) -> [0: bb7, otherwise: bb7]; } bb7: { - _1 = const 5_i32; - goto -> bb13; + falseEdge -> [real: bb12, imaginary: bb1]; } bb8: { - falseEdge -> [real: bb9, imaginary: bb7]; + _1 = const 5_i32; + goto -> bb15; } bb9: { - _1 = const 4_i32; - goto -> bb13; + switchInt(copy (_2.1: bool)) -> [0: bb10, otherwise: bb10]; } bb10: { - _1 = const 3_i32; - goto -> bb13; + falseEdge -> [real: bb11, imaginary: bb8]; } bb11: { - _1 = const 2_i32; - goto -> bb13; + _1 = const 4_i32; + goto -> bb15; } bb12: { - _1 = const 1_i32; - goto -> bb13; + _1 = const 3_i32; + goto -> bb15; } bb13: { + _1 = const 2_i32; + goto -> bb15; + } + + bb14: { + _1 = const 1_i32; + goto -> bb15; + } + + bb15: { StorageDead(_2); StorageDead(_1); _0 = const (); diff --git a/tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff b/tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff new file mode 100644 index 0000000000000..87758408a1c37 --- /dev/null +++ b/tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff @@ -0,0 +1,49 @@ +- // MIR for `main` before SimplifyCfg-initial ++ // MIR for `main` after SimplifyCfg-initial + + fn main() -> () { + let mut _0: (); + let _1: &i32; + let _2: i32; + scope 1 { + debug ref_ => _1; + scope 2 { + } + } + + bb0: { + StorageLive(_1); + StorageLive(_2); + _2 = const 1_i32; + _1 = &_2; + FakeRead(ForLet(None), _1); + PlaceMention(_1); +- switchInt(copy (*_1)) -> [0: bb2, otherwise: bb1]; ++ switchInt(copy (*_1)) -> [0: bb1, otherwise: bb1]; + } + + bb1: { +- goto -> bb5; +- } +- +- bb2: { +- goto -> bb5; +- } +- +- bb3: { +- goto -> bb1; +- } +- +- bb4: { +- FakeRead(ForMatchedPlace(None), _1); +- unreachable; +- } +- +- bb5: { + _0 = const (); + StorageDead(_2); + StorageDead(_1); + return; + } + } + diff --git a/tests/mir-opt/read_from_trivial_switch.rs b/tests/mir-opt/read_from_trivial_switch.rs new file mode 100644 index 0000000000000..16a9750a46141 --- /dev/null +++ b/tests/mir-opt/read_from_trivial_switch.rs @@ -0,0 +1,15 @@ +// Ensure that we don't optimize out `SwitchInt` reads even if that terminator +// branches to the same basic block on every target, since the operand may have +// side-effects that affect analysis of the MIR. +// +// See . + +//@ test-mir-pass: SimplifyCfg-initial +//@ compile-flags: -Zmir-opt-level=0 + +// EMIT_MIR read_from_trivial_switch.main.SimplifyCfg-initial.diff +fn main() { + let ref_ = &1i32; + // CHECK: switchInt + let &(0 | _) = ref_; +} diff --git a/tests/mir-opt/simplify_locals_removes_unused_discriminant_reads.map.SimplifyLocals-before-const-prop.diff b/tests/mir-opt/simplify_locals_removes_unused_discriminant_reads.map.SimplifyLocals-before-const-prop.diff index 5611d679b7801..3bfe3636cbd59 100644 --- a/tests/mir-opt/simplify_locals_removes_unused_discriminant_reads.map.SimplifyLocals-before-const-prop.diff +++ b/tests/mir-opt/simplify_locals_removes_unused_discriminant_reads.map.SimplifyLocals-before-const-prop.diff @@ -10,6 +10,7 @@ - let mut _5: bool; - let mut _6: isize; - let mut _7: isize; ++ let mut _5: isize; scope 1 { debug x => _3; } @@ -33,17 +34,23 @@ _0 = Option::>::Some(move _4); StorageDead(_4); StorageDead(_3); - goto -> bb4; + goto -> bb5; } bb3: { _0 = Option::>::None; - goto -> bb4; + goto -> bb5; } bb4: { -- _6 = discriminant(_1); return; } + + bb5: { +- _6 = discriminant(_1); +- switchInt(move _6) -> [1: bb4, otherwise: bb4]; ++ _5 = discriminant(_1); ++ switchInt(move _5) -> [1: bb4, otherwise: bb4]; + } } diff --git a/tests/ui/pattern/uninit-trivial.rs b/tests/ui/pattern/uninit-trivial.rs new file mode 100644 index 0000000000000..6ea6796c1c1f0 --- /dev/null +++ b/tests/ui/pattern/uninit-trivial.rs @@ -0,0 +1,8 @@ +// Regression test for the semantic changes in +// . + +fn main() { + let x; + let (0 | _) = x; + //~^ ERROR used binding `x` isn't initialized +} diff --git a/tests/ui/pattern/uninit-trivial.stderr b/tests/ui/pattern/uninit-trivial.stderr new file mode 100644 index 0000000000000..2ff8557c94543 --- /dev/null +++ b/tests/ui/pattern/uninit-trivial.stderr @@ -0,0 +1,16 @@ +error[E0381]: used binding `x` isn't initialized + --> $DIR/uninit-trivial.rs:6:10 + | +LL | let x; + | - binding declared here but left uninitialized +LL | let (0 | _) = x; + | ^^^^^ `x` used here but it isn't initialized + | +help: consider assigning a value + | +LL | let x = 42; + | ++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0381`.