From 11f7e302e154cdb378a8ae4b0ca8332c40218b20 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 4 Oct 2024 14:59:07 +0000 Subject: [PATCH 1/2] Add diff test for MatchBranchSimplification --- ....my_is_some.MatchBranchSimplification.diff | 32 +++++++++++++++++++ tests/mir-opt/matches_reduce_branches.rs | 14 ++++++++ 2 files changed, 46 insertions(+) create mode 100644 tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff diff --git a/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff b/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff new file mode 100644 index 000000000000..d55e764e1c25 --- /dev/null +++ b/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff @@ -0,0 +1,32 @@ +- // MIR for `my_is_some` before MatchBranchSimplification ++ // MIR for `my_is_some` after MatchBranchSimplification + + fn my_is_some(_1: Option<()>) -> bool { + debug bar => _1; + let mut _0: bool; + let mut _2: isize; + + bb0: { + _2 = discriminant(_1); + switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1]; + } + + bb1: { + unreachable; + } + + bb2: { + _0 = const false; + goto -> bb4; + } + + bb3: { + _0 = const true; + goto -> bb4; + } + + bb4: { + return; + } + } + diff --git a/tests/mir-opt/matches_reduce_branches.rs b/tests/mir-opt/matches_reduce_branches.rs index 6787e5816a30..e2bd7b3459c1 100644 --- a/tests/mir-opt/matches_reduce_branches.rs +++ b/tests/mir-opt/matches_reduce_branches.rs @@ -19,6 +19,18 @@ fn foo(bar: Option<()>) { } } +// EMIT_MIR matches_reduce_branches.my_is_some.MatchBranchSimplification.diff +// Test for #131219. +fn my_is_some(bar: Option<()>) -> bool { + // CHECK-LABEL: fn my_is_some( + // CHECK: switchInt + // CHECK: return + match bar { + Some(_) => true, + None => false, + } +} + // EMIT_MIR matches_reduce_branches.bar.MatchBranchSimplification.diff fn bar(i: i32) -> (bool, bool, bool, bool) { // CHECK-LABEL: fn bar( @@ -651,4 +663,6 @@ fn main() { let _: u8 = match_trunc_u16_u8_failed(EnumAu16::u0_0x0000); let _ = match_i128_u128(EnumAi128::A); + + let _ = my_is_some(None); } From e32ec45c02d890e46e55af86163a6d1ba10a4b41 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 4 Oct 2024 20:26:41 +0000 Subject: [PATCH 2/2] MatchBranchSimplification: Consider empty-unreachable otherwise branch --- compiler/rustc_data_structures/src/packed.rs | 7 ++++ compiler/rustc_middle/src/mir/terminator.rs | 11 +++++ .../rustc_mir_transform/src/match_branches.rs | 30 ++++++++++---- ..._to_digit.PreCodegen.after.panic-abort.mir | 19 ++++----- ...to_digit.PreCodegen.after.panic-unwind.mir | 19 ++++----- ....my_is_some.MatchBranchSimplification.diff | 41 +++++++++++-------- tests/mir-opt/matches_reduce_branches.rs | 2 +- 7 files changed, 81 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_data_structures/src/packed.rs b/compiler/rustc_data_structures/src/packed.rs index f54b12b5b532..c8921536530a 100644 --- a/compiler/rustc_data_structures/src/packed.rs +++ b/compiler/rustc_data_structures/src/packed.rs @@ -18,6 +18,13 @@ impl Pu128 { } } +impl From for u128 { + #[inline] + fn from(value: Pu128) -> Self { + value.get() + } +} + impl From for Pu128 { #[inline] fn from(value: u128) -> Self { diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index b919f5726db5..473b817aed07 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -67,6 +67,17 @@ impl SwitchTargets { &mut self.targets } + /// Returns a slice with all considered values (not including the fallback). + #[inline] + pub fn all_values(&self) -> &[Pu128] { + &self.values + } + + #[inline] + pub fn all_values_mut(&mut self) -> &mut [Pu128] { + &mut self.values + } + /// Finds the `BasicBlock` to which this `SwitchInt` will branch given the /// specific value. This cannot fail, as it'll return the `otherwise` /// branch if there's not a specific match for the value. diff --git a/compiler/rustc_mir_transform/src/match_branches.rs b/compiler/rustc_mir_transform/src/match_branches.rs index 20e2a65b311c..534ba9917801 100644 --- a/compiler/rustc_mir_transform/src/match_branches.rs +++ b/compiler/rustc_mir_transform/src/match_branches.rs @@ -7,6 +7,7 @@ use rustc_middle::mir::*; use rustc_middle::ty::layout::{IntegerExt, TyAndLayout}; use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt}; use rustc_type_ir::TyKind::*; +use tracing::instrument; use super::simplify::simplify_cfg; @@ -51,7 +52,7 @@ impl<'tcx> crate::MirPass<'tcx> for MatchBranchSimplification { } trait SimplifyMatch<'tcx> { - /// Simplifies a match statement, returning true if the simplification succeeds, false + /// Simplifies a match statement, returning `Some` if the simplification succeeds, `None` /// otherwise. Generic code is written here, and we generally don't need a custom /// implementation. fn simplify( @@ -159,6 +160,7 @@ struct SimplifyToIf; /// } /// ``` impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf { + #[instrument(level = "debug", skip(self, tcx), ret)] fn can_simplify( &mut self, tcx: TyCtxt<'tcx>, @@ -167,12 +169,15 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf { bbs: &IndexSlice>, _discr_ty: Ty<'tcx>, ) -> Option<()> { - if targets.iter().len() != 1 { - return None; - } + let (first, second) = match targets.all_targets() { + &[first, otherwise] => (first, otherwise), + &[first, second, otherwise] if bbs[otherwise].is_empty_unreachable() => (first, second), + _ => { + return None; + } + }; + // We require that the possible target blocks all be distinct. - let (_, first) = targets.iter().next().unwrap(); - let second = targets.otherwise(); if first == second { return None; } @@ -221,8 +226,14 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf { discr_local: Local, discr_ty: Ty<'tcx>, ) { - let (val, first) = targets.iter().next().unwrap(); - let second = targets.otherwise(); + let ((val, first), second) = match (targets.all_targets(), targets.all_values()) { + (&[first, otherwise], &[val]) => ((val, first), otherwise), + (&[first, second, otherwise], &[val, _]) if bbs[otherwise].is_empty_unreachable() => { + ((val, first), second) + } + _ => unreachable!(), + }; + // We already checked that first and second are different blocks, // and bb_idx has a different terminator from both of them. let first = &bbs[first]; @@ -297,7 +308,7 @@ struct SimplifyToExp { transform_kinds: Vec, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] enum ExpectedTransformKind<'a, 'tcx> { /// Identical statements. Same(&'a StatementKind<'tcx>), @@ -362,6 +373,7 @@ impl From> for TransformKind { /// } /// ``` impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp { + #[instrument(level = "debug", skip(self, tcx), ret)] fn can_simplify( &mut self, tcx: TyCtxt<'tcx>, diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir index 573c0a12bc1a..5876c55c52b9 100644 --- a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir @@ -25,12 +25,12 @@ fn num_to_digit(_1: char) -> u32 { bb1: { StorageLive(_3); _3 = discriminant(_2); - switchInt(move _3) -> [1: bb2, 0: bb6, otherwise: bb8]; + StorageDead(_2); + switchInt(move _3) -> [1: bb2, otherwise: bb7]; } bb2: { StorageDead(_3); - StorageDead(_2); StorageLive(_4); _4 = char::methods::::to_digit(move _1, const 8_u32) -> [return: bb3, unwind unreachable]; } @@ -38,7 +38,7 @@ fn num_to_digit(_1: char) -> u32 { bb3: { StorageLive(_5); _5 = discriminant(_4); - switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb8]; + switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb6]; } bb4: { @@ -49,21 +49,20 @@ fn num_to_digit(_1: char) -> u32 { _0 = move ((_4 as Some).0: u32); StorageDead(_5); StorageDead(_4); - goto -> bb7; + goto -> bb8; } bb6: { - StorageDead(_3); - StorageDead(_2); - _0 = const 0_u32; - goto -> bb7; + unreachable; } bb7: { - return; + StorageDead(_3); + _0 = const 0_u32; + goto -> bb8; } bb8: { - unreachable; + return; } } diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir index 049803041d4e..f1185353a43c 100644 --- a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir +++ b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir @@ -25,12 +25,12 @@ fn num_to_digit(_1: char) -> u32 { bb1: { StorageLive(_3); _3 = discriminant(_2); - switchInt(move _3) -> [1: bb2, 0: bb6, otherwise: bb8]; + StorageDead(_2); + switchInt(move _3) -> [1: bb2, otherwise: bb7]; } bb2: { StorageDead(_3); - StorageDead(_2); StorageLive(_4); _4 = char::methods::::to_digit(move _1, const 8_u32) -> [return: bb3, unwind continue]; } @@ -38,7 +38,7 @@ fn num_to_digit(_1: char) -> u32 { bb3: { StorageLive(_5); _5 = discriminant(_4); - switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb8]; + switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb6]; } bb4: { @@ -49,21 +49,20 @@ fn num_to_digit(_1: char) -> u32 { _0 = move ((_4 as Some).0: u32); StorageDead(_5); StorageDead(_4); - goto -> bb7; + goto -> bb8; } bb6: { - StorageDead(_3); - StorageDead(_2); - _0 = const 0_u32; - goto -> bb7; + unreachable; } bb7: { - return; + StorageDead(_3); + _0 = const 0_u32; + goto -> bb8; } bb8: { - unreachable; + return; } } diff --git a/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff b/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff index d55e764e1c25..d255278ed300 100644 --- a/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff +++ b/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff @@ -5,27 +5,32 @@ debug bar => _1; let mut _0: bool; let mut _2: isize; ++ let mut _3: isize; bb0: { _2 = discriminant(_1); - switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1]; - } - - bb1: { - unreachable; - } - - bb2: { - _0 = const false; - goto -> bb4; - } - - bb3: { - _0 = const true; - goto -> bb4; - } - - bb4: { +- switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1]; +- } +- +- bb1: { +- unreachable; +- } +- +- bb2: { +- _0 = const false; +- goto -> bb4; +- } +- +- bb3: { +- _0 = const true; +- goto -> bb4; +- } +- +- bb4: { ++ StorageLive(_3); ++ _3 = move _2; ++ _0 = Ne(copy _3, const 0_isize); ++ StorageDead(_3); return; } } diff --git a/tests/mir-opt/matches_reduce_branches.rs b/tests/mir-opt/matches_reduce_branches.rs index e2bd7b3459c1..3372ae2f2a61 100644 --- a/tests/mir-opt/matches_reduce_branches.rs +++ b/tests/mir-opt/matches_reduce_branches.rs @@ -23,7 +23,7 @@ fn foo(bar: Option<()>) { // Test for #131219. fn my_is_some(bar: Option<()>) -> bool { // CHECK-LABEL: fn my_is_some( - // CHECK: switchInt + // CHECK: = Ne // CHECK: return match bar { Some(_) => true,