From 66edf317f7d5499e89bd77eba5d2bb9e05791949 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 4 Oct 2024 20:26:41 +0000 Subject: [PATCH] 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-unwind.mir | 20 ++++----- ....my_is_some.MatchBranchSimplification.diff | 41 +++++++++++-------- tests/mir-opt/matches_reduce_branches.rs | 2 +- 6 files changed, 71 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_data_structures/src/packed.rs b/compiler/rustc_data_structures/src/packed.rs index f54b12b5b532d..c8921536530a2 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 783952fb9cb8a..baa457939c6ff 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 ad3126f66a67e..b0a08b22b2ee7 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::ty::layout::{IntegerExt, TyAndLayout}; use rustc_middle::ty::{ParamEnv, ScalarInt, Ty, TyCtxt}; use rustc_target::abi::Integer; use rustc_type_ir::TyKind::*; +use tracing::instrument; use super::simplify::simplify_cfg; @@ -57,7 +58,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( @@ -156,6 +157,7 @@ struct SimplifyToIf; /// } /// ``` impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf { + #[instrument(level = "debug", skip(self, tcx), ret)] fn can_simplify( &mut self, tcx: TyCtxt<'tcx>, @@ -164,12 +166,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; } @@ -218,8 +223,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]; @@ -294,7 +305,7 @@ struct SimplifyToExp { transform_kinds: Vec, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] enum ExpectedTransformKind<'a, 'tcx> { /// Identical statements. Same(&'a StatementKind<'tcx>), @@ -359,6 +370,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-unwind.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir index 049803041d4e3..cc99f279d8a4a 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 @@ -23,14 +23,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 +36,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 +47,19 @@ 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; + _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 50e7596db54f0..310f74786bfaf 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 @@ -4,27 +4,32 @@ fn my_is_some(_1: Option) -> bool { let mut _0: bool; let mut _2: isize; ++ let mut _3: isize; bb0: { _2 = discriminant(_1); - switchInt(copy _2) -> [0: bb1, 1: bb2, otherwise: bb3]; - } - - bb1: { - _0 = const false; - goto -> bb4; - } - - bb2: { - _0 = const true; - goto -> bb4; - } - - bb3: { - unreachable; - } - - bb4: { +- switchInt(copy _2) -> [0: bb1, 1: bb2, otherwise: bb3]; +- } +- +- bb1: { +- _0 = const false; +- goto -> bb4; +- } +- +- bb2: { +- _0 = const true; +- goto -> bb4; +- } +- +- bb3: { +- unreachable; +- } +- +- bb4: { ++ StorageLive(_3); ++ _3 = copy _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 cf96dcf9a73e2..7f843e4085112 100644 --- a/tests/mir-opt/matches_reduce_branches.rs +++ b/tests/mir-opt/matches_reduce_branches.rs @@ -25,7 +25,7 @@ fn foo(bar: Option<()>) { #[custom_mir(dialect = "built")] fn my_is_some(bar: Option) -> bool { // CHECK-LABEL: fn my_is_some( - // CHECK: switchInt + // CHECK: = Ne // CHECK: return mir! { {