From 31e74771f04126cdb753dcc1798ef024893431a3 Mon Sep 17 00:00:00 2001 From: DianQK Date: Wed, 21 Feb 2024 22:44:48 +0800 Subject: [PATCH] Resolve unsound hoisting of discriminant in `EarlyOtherwiseBranch` --- .../src/early_otherwise_branch.rs | 166 ++++++------------ ...wise_branch.opt1.EarlyOtherwiseBranch.diff | 39 ++-- ...wise_branch.opt2.EarlyOtherwiseBranch.diff | 57 +++--- ...wise_branch.opt3.EarlyOtherwiseBranch.diff | 106 +++++++---- ...wise_branch.opt4.EarlyOtherwiseBranch.diff | 104 +++++++++++ tests/mir-opt/early_otherwise_branch.rs | 32 +++- ...ement_tuple.opt1.EarlyOtherwiseBranch.diff | 91 +++++----- ...ement_tuple.opt2.EarlyOtherwiseBranch.diff | 138 +++++++++++++++ .../early_otherwise_branch_3_element_tuple.rs | 21 +++ tests/mir-opt/early_otherwise_branch_68867.rs | 1 + ...ch_68867.try_sum.EarlyOtherwiseBranch.diff | 6 +- tests/mir-opt/early_otherwise_branch_noopt.rs | 1 + ...ess.no_deref_ptr.EarlyOtherwiseBranch.diff | 8 +- ...ness.no_downcast.EarlyOtherwiseBranch.diff | 8 +- .../early_otherwise_branch_soundness.rs | 1 + 15 files changed, 507 insertions(+), 272 deletions(-) create mode 100644 tests/mir-opt/early_otherwise_branch.opt4.EarlyOtherwiseBranch.diff create mode 100644 tests/mir-opt/early_otherwise_branch_3_element_tuple.opt2.EarlyOtherwiseBranch.diff diff --git a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs index 0d600f0f937..3d38bcc6f13 100644 --- a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs +++ b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs @@ -1,6 +1,6 @@ use rustc_middle::mir::patch::MirPatch; use rustc_middle::mir::*; -use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::ty::{Ty, TyCtxt}; use std::fmt::Debug; use super::simplify::simplify_cfg; @@ -11,6 +11,7 @@ /// let y: Option<()>; /// match (x,y) { /// (Some(_), Some(_)) => {0}, +/// (None, None) => {2}, /// _ => {1} /// } /// ``` @@ -23,10 +24,10 @@ /// if discriminant_x == discriminant_y { /// match x { /// Some(_) => 0, -/// _ => 1, // <---- -/// } // | Actually the same bb -/// } else { // | -/// 1 // <-------------- +/// None => 2, +/// } +/// } else { +/// 1 /// } /// ``` /// @@ -47,18 +48,18 @@ /// | | | /// ================= | | | /// | BBU | <-| | | ============================ -/// |---------------| | \-------> | BBD | -/// |---------------| | | |--------------------------| -/// | unreachable | | | | _dl = discriminant(P) | -/// ================= | | |--------------------------| -/// | | | switchInt(_dl) | -/// ================= | | | d | ---> BBD.2 +/// |---------------| \-------> | BBD | +/// |---------------| | |--------------------------| +/// | unreachable | | | _dl = discriminant(P) | +/// ================= | |--------------------------| +/// | | switchInt(_dl) | +/// ================= | | d | ---> BBD.2 /// | BB9 | <--------------- | otherwise | /// |---------------| ============================ /// | ... | /// ================= /// ``` -/// Where the `otherwise` branch on `BB1` is permitted to either go to `BBU` or to `BB9`. In the +/// Where the `otherwise` branch on `BB1` is permitted to either go to `BBU`. In the /// code: /// - `BB1` is `parent` and `BBC, BBD` are children /// - `P` is `child_place` @@ -78,7 +79,7 @@ /// |---------------------| | | switchInt(Q) | /// | switchInt(_t) | | | c | ---> BBC.2 /// | false | --------/ | d | ---> BBD.2 -/// | otherwise | ---------------- | otherwise | +/// | otherwise | /--------- | otherwise | /// ======================= | ============================ /// | /// ================= | @@ -87,10 +88,6 @@ /// | ... | /// ================= /// ``` -/// -/// This is only correct for some `P`, since `P` is now computed outside the original `switchInt`. -/// The filter on which `P` are allowed (together with discussion of its correctness) is found in -/// `may_hoist`. pub struct EarlyOtherwiseBranch; impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch { @@ -217,85 +214,6 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { } } -/// Returns true if computing the discriminant of `place` may be hoisted out of the branch -fn may_hoist<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, place: Place<'tcx>) -> bool { - // FIXME(JakobDegen): This is unsound. Someone could write code like this: - // ```rust - // let Q = val; - // if discriminant(P) == otherwise { - // let ptr = &mut Q as *mut _ as *mut u8; - // unsafe { *ptr = 10; } // Any invalid value for the type - // } - // - // match P { - // A => match Q { - // A => { - // // code - // } - // _ => { - // // don't use Q - // } - // } - // _ => { - // // don't use Q - // } - // }; - // ``` - // - // Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant of an - // invalid value, which is UB. - // - // In order to fix this, we would either need to show that the discriminant computation of - // `place` is computed in all branches, including the `otherwise` branch, or we would need - // another analysis pass to determine that the place is fully initialized. It might even be best - // to have the hoisting be performed in a different pass and just do the CFG changing in this - // pass. - for (place, proj) in place.iter_projections() { - match proj { - // Dereferencing in the computation of `place` might cause issues from one of two - // categories. First, the referent might be invalid. We protect against this by - // dereferencing references only (not pointers). Second, the use of a reference may - // invalidate other references that are used later (for aliasing reasons). Consider - // where such an invalidated reference may appear: - // - In `Q`: Not possible since `Q` is used as the operand of a `SwitchInt` and so - // cannot contain referenced data. - // - In `BBU`: Not possible since that block contains only the `unreachable` terminator - // - In `BBC.2, BBD.2`: Not possible, since `discriminant(P)` was computed prior to - // reaching that block in the input to our transformation, and so any data - // invalidated by that computation could not have been used there. - // - In `BB9`: Not possible since control flow might have reached `BB9` via the - // `otherwise` branch in `BBC, BBD` in the input to our transformation, which would - // have invalidated the data when computing `discriminant(P)` - // So dereferencing here is correct. - ProjectionElem::Deref => match place.ty(body.local_decls(), tcx).ty.kind() { - ty::Ref(..) => {} - _ => return false, - }, - // Field projections are always valid - ProjectionElem::Field(..) => {} - // We cannot allow - // downcasts either, since the correctness of the downcast may depend on the parent - // branch being taken. An easy example of this is - // ``` - // Q = discriminant(_3) - // P = (_3 as Variant) - // ``` - // However, checking if the child and parent place are the same and only erroring then - // is not sufficient either, since the `discriminant(_3) == 1` (or whatever) check may - // be replaced by another optimization pass with any other condition that can be proven - // equivalent. - ProjectionElem::Downcast(..) => { - return false; - } - // We cannot allow indexing since the index may be out of bounds. - _ => { - return false; - } - } - } - true -} - #[derive(Debug)] struct OptimizationData<'tcx> { destination: BasicBlock, @@ -315,18 +233,40 @@ fn evaluate_candidate<'tcx>( return None; }; let parent_ty = parent_discr.ty(body.local_decls(), tcx); - let parent_dest = { - let poss = targets.otherwise(); - // If the fallthrough on the parent is trivially unreachable, we can let the - // children choose the destination - if bbs[poss].statements.len() == 0 - && bbs[poss].terminator().kind == TerminatorKind::Unreachable - { - None - } else { - Some(poss) - } - }; + if !bbs[targets.otherwise()].is_empty_unreachable() { + // Someone could write code like this: + // ```rust + // let Q = val; + // if discriminant(P) == otherwise { + // let ptr = &mut Q as *mut _ as *mut u8; + // // It may be difficult for us to effectively determine whether values are valid. + // // Invalid values can come from all sorts of corners. + // unsafe { *ptr = 10; } + // } + // + // match P { + // A => match Q { + // A => { + // // code + // } + // _ => { + // // don't use Q + // } + // } + // _ => { + // // don't use Q + // } + // }; + // ``` + // + // Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant of an + // invalid value, which is UB. + // In order to fix this, **we would either need to show that the discriminant computation of + // `place` is computed in all branches**. + // FIXME(#95162) For the moment, we adopt a conservative approach and + // consider only the `otherwise` branch has no statements and an unreachable terminator. + return None; + } let (_, child) = targets.iter().next()?; let child_terminator = &bbs[child].terminator(); let TerminatorKind::SwitchInt { targets: child_targets, discr: child_discr } = @@ -344,13 +284,7 @@ fn evaluate_candidate<'tcx>( let (_, Rvalue::Discriminant(child_place)) = &**boxed else { return None; }; - let destination = parent_dest.unwrap_or(child_targets.otherwise()); - - // Verify that the optimization is legal in general - // We can hoist evaluating the child discriminant out of the branch - if !may_hoist(tcx, body, *child_place) { - return None; - } + let destination = child_targets.otherwise(); // Verify that the optimization is legal for each branch for (value, child) in targets.iter() { @@ -411,5 +345,5 @@ fn verify_candidate_branch<'tcx>( if let Some(_) = iter.next() { return false; } - return true; + true } diff --git a/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff index 7a374c5675a..4af3ed3e1d1 100644 --- a/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff @@ -12,8 +12,6 @@ let mut _7: isize; let _8: u32; let _9: u32; -+ let mut _10: isize; -+ let mut _11: bool; scope 1 { debug a => _8; debug b => _9; @@ -29,28 +27,20 @@ StorageDead(_5); StorageDead(_4); _7 = discriminant((_3.0: std::option::Option)); -- switchInt(move _7) -> [1: bb2, otherwise: bb1]; -+ StorageLive(_10); -+ _10 = discriminant((_3.1: std::option::Option)); -+ StorageLive(_11); -+ _11 = Ne(_7, move _10); -+ StorageDead(_10); -+ switchInt(move _11) -> [0: bb4, otherwise: bb1]; + switchInt(move _7) -> [1: bb2, 0: bb1, otherwise: bb5]; } bb1: { -+ StorageDead(_11); _0 = const 1_u32; -- goto -> bb4; -+ goto -> bb3; + goto -> bb4; } bb2: { -- _6 = discriminant((_3.1: std::option::Option)); -- switchInt(move _6) -> [1: bb3, otherwise: bb1]; -- } -- -- bb3: { + _6 = discriminant((_3.1: std::option::Option)); + switchInt(move _6) -> [1: bb3, 0: bb1, otherwise: bb5]; + } + + bb3: { StorageLive(_8); _8 = (((_3.0: std::option::Option) as Some).0: u32); StorageLive(_9); @@ -58,19 +48,16 @@ _0 = const 0_u32; StorageDead(_9); StorageDead(_8); -- goto -> bb4; -+ goto -> bb3; + goto -> bb4; } -- bb4: { -+ bb3: { + bb4: { StorageDead(_3); return; -+ } -+ -+ bb4: { -+ StorageDead(_11); -+ switchInt(_7) -> [1: bb2, otherwise: bb1]; + } + + bb5: { + unreachable; } } diff --git a/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff index 1348bdd739a..7776ff0fde7 100644 --- a/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff @@ -13,8 +13,6 @@ let mut _8: isize; let _9: u32; let _10: u32; -+ let mut _11: isize; -+ let mut _12: bool; scope 1 { debug a => _9; debug b => _10; @@ -30,33 +28,25 @@ StorageDead(_5); StorageDead(_4); _8 = discriminant((_3.0: std::option::Option)); -- switchInt(move _8) -> [0: bb3, 1: bb2, otherwise: bb1]; -+ StorageLive(_11); -+ _11 = discriminant((_3.1: std::option::Option)); -+ StorageLive(_12); -+ _12 = Ne(_8, move _11); -+ StorageDead(_11); -+ switchInt(move _12) -> [0: bb5, otherwise: bb1]; + switchInt(move _8) -> [0: bb3, 1: bb2, otherwise: bb7]; } bb1: { -+ StorageDead(_12); _0 = const 1_u32; -- goto -> bb6; -+ goto -> bb4; + goto -> bb6; } bb2: { -- _6 = discriminant((_3.1: std::option::Option)); -- switchInt(move _6) -> [1: bb4, otherwise: bb1]; -- } -- -- bb3: { -- _7 = discriminant((_3.1: std::option::Option)); -- switchInt(move _7) -> [0: bb5, otherwise: bb1]; -- } -- -- bb4: { + _6 = discriminant((_3.1: std::option::Option)); + switchInt(move _6) -> [1: bb4, 0: bb1, otherwise: bb7]; + } + + bb3: { + _7 = discriminant((_3.1: std::option::Option)); + switchInt(move _7) -> [0: bb5, 1: bb1, otherwise: bb7]; + } + + bb4: { StorageLive(_9); _9 = (((_3.0: std::option::Option) as Some).0: u32); StorageLive(_10); @@ -64,26 +54,21 @@ _0 = const 0_u32; StorageDead(_10); StorageDead(_9); -- goto -> bb6; -+ goto -> bb4; + goto -> bb6; } -- bb5: { -+ bb3: { - _0 = const 0_u32; -- goto -> bb6; -+ goto -> bb4; + bb5: { + _0 = const 2_u32; + goto -> bb6; } -- bb6: { -+ bb4: { + bb6: { StorageDead(_3); return; -+ } -+ -+ bb5: { -+ StorageDead(_12); -+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb1]; + } + + bb7: { + unreachable; } } diff --git a/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff index e058c409cb5..d93c609c051 100644 --- a/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff @@ -1,76 +1,104 @@ - // MIR for `opt3` before EarlyOtherwiseBranch + // MIR for `opt3` after EarlyOtherwiseBranch - fn opt3(_1: Option, _2: Option) -> u32 { + fn opt3(_1: Option2, _2: Option2) -> u32 { debug x => _1; debug y => _2; let mut _0: u32; - let mut _3: (std::option::Option, std::option::Option); - let mut _4: std::option::Option; - let mut _5: std::option::Option; + let mut _3: (Option2, Option2); + let mut _4: Option2; + let mut _5: Option2; let mut _6: isize; let mut _7: isize; - let _8: u32; - let _9: bool; -+ let mut _10: isize; -+ let mut _11: bool; + let mut _8: isize; + let mut _9: isize; + let _10: u32; + let _11: bool; ++ let mut _12: isize; ++ let mut _13: bool; scope 1 { - debug a => _8; - debug b => _9; + debug a => _10; + debug b => _11; } bb0: { StorageLive(_3); StorageLive(_4); - _4 = _1; + _4 = move _1; StorageLive(_5); - _5 = _2; + _5 = move _2; _3 = (move _4, move _5); StorageDead(_5); StorageDead(_4); - _7 = discriminant((_3.0: std::option::Option)); -- switchInt(move _7) -> [1: bb2, otherwise: bb1]; -+ StorageLive(_10); -+ _10 = discriminant((_3.1: std::option::Option)); -+ StorageLive(_11); -+ _11 = Ne(_7, move _10); -+ StorageDead(_10); -+ switchInt(move _11) -> [0: bb4, otherwise: bb1]; + _9 = discriminant((_3.0: Option2)); +- switchInt(move _9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb9]; ++ StorageLive(_12); ++ _12 = discriminant((_3.1: Option2)); ++ StorageLive(_13); ++ _13 = Ne(_9, move _12); ++ StorageDead(_12); ++ switchInt(move _13) -> [0: bb6, otherwise: bb1]; } bb1: { -+ StorageDead(_11); ++ StorageDead(_13); _0 = const 1_u32; -- goto -> bb4; -+ goto -> bb3; +- goto -> bb8; ++ goto -> bb5; } bb2: { -- _6 = discriminant((_3.1: std::option::Option)); -- switchInt(move _6) -> [1: bb3, otherwise: bb1]; +- _6 = discriminant((_3.1: Option2)); +- switchInt(move _6) -> [0: bb5, otherwise: bb1]; - } - - bb3: { - StorageLive(_8); - _8 = (((_3.0: std::option::Option) as Some).0: u32); - StorageLive(_9); - _9 = (((_3.1: std::option::Option) as Some).0: bool); +- _7 = discriminant((_3.1: Option2)); +- switchInt(move _7) -> [1: bb6, otherwise: bb1]; +- } +- +- bb4: { +- _8 = discriminant((_3.1: Option2)); +- switchInt(move _8) -> [2: bb7, otherwise: bb1]; +- } +- +- bb5: { + StorageLive(_10); + _10 = (((_3.0: Option2) as Some).0: u32); + StorageLive(_11); + _11 = (((_3.1: Option2) as Some).0: bool); _0 = const 0_u32; - StorageDead(_9); - StorageDead(_8); -- goto -> bb4; -+ goto -> bb3; + StorageDead(_11); + StorageDead(_10); +- goto -> bb8; ++ goto -> bb5; } -- bb4: { +- bb6: { + bb3: { + _0 = const 2_u32; +- goto -> bb8; ++ goto -> bb5; + } + +- bb7: { ++ bb4: { + _0 = const 3_u32; +- goto -> bb8; ++ goto -> bb5; + } + +- bb8: { ++ bb5: { StorageDead(_3); return; -+ } -+ -+ bb4: { -+ StorageDead(_11); -+ switchInt(_7) -> [1: bb2, otherwise: bb1]; + } + +- bb9: { +- unreachable; ++ bb6: { ++ StorageDead(_13); ++ switchInt(_9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb1]; } } diff --git a/tests/mir-opt/early_otherwise_branch.opt4.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt4.EarlyOtherwiseBranch.diff new file mode 100644 index 00000000000..afdf7d3c576 --- /dev/null +++ b/tests/mir-opt/early_otherwise_branch.opt4.EarlyOtherwiseBranch.diff @@ -0,0 +1,104 @@ +- // MIR for `opt4` before EarlyOtherwiseBranch ++ // MIR for `opt4` after EarlyOtherwiseBranch + + fn opt4(_1: Option2, _2: Option2) -> u32 { + debug x => _1; + debug y => _2; + let mut _0: u32; + let mut _3: (Option2, Option2); + let mut _4: Option2; + let mut _5: Option2; + let mut _6: isize; + let mut _7: isize; + let mut _8: isize; + let mut _9: isize; + let _10: u32; + let _11: u32; ++ let mut _12: isize; ++ let mut _13: bool; + scope 1 { + debug a => _10; + debug b => _11; + } + + bb0: { + StorageLive(_3); + StorageLive(_4); + _4 = move _1; + StorageLive(_5); + _5 = move _2; + _3 = (move _4, move _5); + StorageDead(_5); + StorageDead(_4); + _9 = discriminant((_3.0: Option2)); +- switchInt(move _9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb9]; ++ StorageLive(_12); ++ _12 = discriminant((_3.1: Option2)); ++ StorageLive(_13); ++ _13 = Ne(_9, move _12); ++ StorageDead(_12); ++ switchInt(move _13) -> [0: bb6, otherwise: bb1]; + } + + bb1: { ++ StorageDead(_13); + _0 = const 1_u32; +- goto -> bb8; ++ goto -> bb5; + } + + bb2: { +- _6 = discriminant((_3.1: Option2)); +- switchInt(move _6) -> [0: bb5, otherwise: bb1]; +- } +- +- bb3: { +- _7 = discriminant((_3.1: Option2)); +- switchInt(move _7) -> [1: bb6, otherwise: bb1]; +- } +- +- bb4: { +- _8 = discriminant((_3.1: Option2)); +- switchInt(move _8) -> [2: bb7, otherwise: bb1]; +- } +- +- bb5: { + StorageLive(_10); + _10 = (((_3.0: Option2) as Some).0: u32); + StorageLive(_11); + _11 = (((_3.1: Option2) as Some).0: u32); + _0 = const 0_u32; + StorageDead(_11); + StorageDead(_10); +- goto -> bb8; ++ goto -> bb5; + } + +- bb6: { ++ bb3: { + _0 = const 2_u32; +- goto -> bb8; ++ goto -> bb5; + } + +- bb7: { ++ bb4: { + _0 = const 3_u32; +- goto -> bb8; ++ goto -> bb5; + } + +- bb8: { ++ bb5: { + StorageDead(_3); + return; + } + +- bb9: { +- unreachable; ++ bb6: { ++ StorageDead(_13); ++ switchInt(_9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb1]; + } + } + diff --git a/tests/mir-opt/early_otherwise_branch.rs b/tests/mir-opt/early_otherwise_branch.rs index c984c271ccd..d3a4134a012 100644 --- a/tests/mir-opt/early_otherwise_branch.rs +++ b/tests/mir-opt/early_otherwise_branch.rs @@ -1,5 +1,14 @@ // skip-filecheck //@ unit-test: EarlyOtherwiseBranch +//@ compile-flags: -Zmir-enable-passes=+UnreachableEnumBranching + +enum Option2 { + Some(T), + None, + Other, +} + +// We can't optimize it because y may be an invalid value. // EMIT_MIR early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff fn opt1(x: Option, y: Option) -> u32 { match (x, y) { @@ -8,20 +17,34 @@ fn opt1(x: Option, y: Option) -> u32 { } } +// FIXME: `switchInt` will have three targets after `UnreachableEnumBranching`, +// otherwise is unreachable. We can consume the UB fact to transform back to if else pattern. // EMIT_MIR early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff fn opt2(x: Option, y: Option) -> u32 { match (x, y) { (Some(a), Some(b)) => 0, - (None, None) => 0, + (None, None) => 2, _ => 1, } } // optimize despite different types // EMIT_MIR early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff -fn opt3(x: Option, y: Option) -> u32 { +fn opt3(x: Option2, y: Option2) -> u32 { match (x, y) { - (Some(a), Some(b)) => 0, + (Option2::Some(a), Option2::Some(b)) => 0, + (Option2::None, Option2::None) => 2, + (Option2::Other, Option2::Other) => 3, + _ => 1, + } +} + +// EMIT_MIR early_otherwise_branch.opt4.EarlyOtherwiseBranch.diff +fn opt4(x: Option2, y: Option2) -> u32 { + match (x, y) { + (Option2::Some(a), Option2::Some(b)) => 0, + (Option2::None, Option2::None) => 2, + (Option2::Other, Option2::Other) => 3, _ => 1, } } @@ -29,5 +52,6 @@ fn opt3(x: Option, y: Option) -> u32 { fn main() { opt1(None, Some(0)); opt2(None, Some(0)); - opt3(None, Some(false)); + opt3(Option2::None, Option2::Some(false)); + opt4(Option2::None, Option2::Some(0)); } diff --git a/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff index f98d68e6ffc..4c3c717b522 100644 --- a/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff @@ -13,17 +13,15 @@ let mut _8: isize; let mut _9: isize; let mut _10: isize; - let _11: u32; - let _12: u32; + let mut _11: isize; + let mut _12: isize; let _13: u32; -+ let mut _14: isize; -+ let mut _15: bool; -+ let mut _16: isize; -+ let mut _17: bool; + let _14: u32; + let _15: u32; scope 1 { - debug a => _11; - debug b => _12; - debug c => _13; + debug a => _13; + debug b => _14; + debug c => _15; } bb0: { @@ -38,60 +36,61 @@ StorageDead(_7); StorageDead(_6); StorageDead(_5); - _10 = discriminant((_4.0: std::option::Option)); -- switchInt(move _10) -> [1: bb2, otherwise: bb1]; -+ StorageLive(_14); -+ _14 = discriminant((_4.1: std::option::Option)); -+ StorageLive(_15); -+ _15 = Ne(_10, move _14); -+ StorageDead(_14); -+ switchInt(move _15) -> [0: bb5, otherwise: bb1]; + _12 = discriminant((_4.0: std::option::Option)); + switchInt(move _12) -> [0: bb4, 1: bb2, otherwise: bb9]; } bb1: { -+ StorageDead(_17); -+ StorageDead(_15); _0 = const 1_u32; -- goto -> bb5; -+ goto -> bb4; + goto -> bb8; } bb2: { -- _9 = discriminant((_4.1: std::option::Option)); -- switchInt(move _9) -> [1: bb3, otherwise: bb1]; -- } -- -- bb3: { + _9 = discriminant((_4.1: std::option::Option)); + switchInt(move _9) -> [1: bb3, 0: bb1, otherwise: bb9]; + } + + bb3: { _8 = discriminant((_4.2: std::option::Option)); -- switchInt(move _8) -> [1: bb4, otherwise: bb1]; -+ switchInt(move _8) -> [1: bb3, otherwise: bb1]; + switchInt(move _8) -> [1: bb6, 0: bb1, otherwise: bb9]; } -- bb4: { -+ bb3: { - StorageLive(_11); - _11 = (((_4.0: std::option::Option) as Some).0: u32); - StorageLive(_12); - _12 = (((_4.1: std::option::Option) as Some).0: u32); + bb4: { + _11 = discriminant((_4.1: std::option::Option)); + switchInt(move _11) -> [0: bb5, 1: bb1, otherwise: bb9]; + } + + bb5: { + _10 = discriminant((_4.2: std::option::Option)); + switchInt(move _10) -> [0: bb7, 1: bb1, otherwise: bb9]; + } + + bb6: { StorageLive(_13); - _13 = (((_4.2: std::option::Option) as Some).0: u32); + _13 = (((_4.0: std::option::Option) as Some).0: u32); + StorageLive(_14); + _14 = (((_4.1: std::option::Option) as Some).0: u32); + StorageLive(_15); + _15 = (((_4.2: std::option::Option) as Some).0: u32); _0 = const 0_u32; + StorageDead(_15); + StorageDead(_14); StorageDead(_13); - StorageDead(_12); - StorageDead(_11); -- goto -> bb5; -+ goto -> bb4; + goto -> bb8; } -- bb5: { -+ bb4: { + bb7: { + _0 = const 2_u32; + goto -> bb8; + } + + bb8: { StorageDead(_4); return; -+ } -+ -+ bb5: { -+ StorageDead(_15); -+ switchInt(_10) -> [1: bb2, otherwise: bb1]; + } + + bb9: { + unreachable; } } diff --git a/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt2.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt2.EarlyOtherwiseBranch.diff new file mode 100644 index 00000000000..89a06a7669d --- /dev/null +++ b/tests/mir-opt/early_otherwise_branch_3_element_tuple.opt2.EarlyOtherwiseBranch.diff @@ -0,0 +1,138 @@ +- // MIR for `opt2` before EarlyOtherwiseBranch ++ // MIR for `opt2` after EarlyOtherwiseBranch + + fn opt2(_1: Option2, _2: Option2, _3: Option2) -> u32 { + debug x => _1; + debug y => _2; + debug z => _3; + let mut _0: u32; + let mut _4: (Option2, Option2, Option2); + let mut _5: Option2; + let mut _6: Option2; + let mut _7: Option2; + let mut _8: isize; + let mut _9: isize; + let mut _10: isize; + let mut _11: isize; + let mut _12: isize; + let mut _13: isize; + let mut _14: isize; + let _15: u32; + let _16: u32; + let _17: u32; ++ let mut _18: isize; ++ let mut _19: bool; + scope 1 { + debug a => _15; + debug b => _16; + debug c => _17; + } + + bb0: { + StorageLive(_4); + StorageLive(_5); + _5 = move _1; + StorageLive(_6); + _6 = move _2; + StorageLive(_7); + _7 = move _3; + _4 = (move _5, move _6, move _7); + StorageDead(_7); + StorageDead(_6); + StorageDead(_5); + _14 = discriminant((_4.0: Option2)); +- switchInt(move _14) -> [0: bb2, 1: bb4, 2: bb6, otherwise: bb12]; ++ StorageLive(_18); ++ _18 = discriminant((_4.1: Option2)); ++ StorageLive(_19); ++ _19 = Ne(_14, move _18); ++ StorageDead(_18); ++ switchInt(move _19) -> [0: bb9, otherwise: bb1]; + } + + bb1: { ++ StorageDead(_19); + _0 = const 1_u32; +- goto -> bb11; ++ goto -> bb8; + } + + bb2: { +- _9 = discriminant((_4.1: Option2)); +- switchInt(move _9) -> [0: bb3, otherwise: bb1]; +- } +- +- bb3: { + _8 = discriminant((_4.2: Option2)); +- switchInt(move _8) -> [0: bb8, otherwise: bb1]; ++ switchInt(move _8) -> [0: bb5, otherwise: bb1]; + } + +- bb4: { +- _11 = discriminant((_4.1: Option2)); +- switchInt(move _11) -> [1: bb5, otherwise: bb1]; +- } +- +- bb5: { ++ bb3: { + _10 = discriminant((_4.2: Option2)); +- switchInt(move _10) -> [1: bb9, otherwise: bb1]; ++ switchInt(move _10) -> [1: bb6, otherwise: bb1]; + } + +- bb6: { +- _13 = discriminant((_4.1: Option2)); +- switchInt(move _13) -> [2: bb7, otherwise: bb1]; +- } +- +- bb7: { ++ bb4: { + _12 = discriminant((_4.2: Option2)); +- switchInt(move _12) -> [2: bb10, otherwise: bb1]; ++ switchInt(move _12) -> [2: bb7, otherwise: bb1]; + } + +- bb8: { ++ bb5: { + StorageLive(_15); + _15 = (((_4.0: Option2) as Some).0: u32); + StorageLive(_16); + _16 = (((_4.1: Option2) as Some).0: u32); + StorageLive(_17); + _17 = (((_4.2: Option2) as Some).0: u32); + _0 = const 0_u32; + StorageDead(_17); + StorageDead(_16); + StorageDead(_15); +- goto -> bb11; ++ goto -> bb8; + } + +- bb9: { ++ bb6: { + _0 = const 2_u32; +- goto -> bb11; ++ goto -> bb8; + } + +- bb10: { ++ bb7: { + _0 = const 3_u32; +- goto -> bb11; ++ goto -> bb8; + } + +- bb11: { ++ bb8: { + StorageDead(_4); + return; + } + +- bb12: { +- unreachable; ++ bb9: { ++ StorageDead(_19); ++ switchInt(_14) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb1]; + } + } + diff --git a/tests/mir-opt/early_otherwise_branch_3_element_tuple.rs b/tests/mir-opt/early_otherwise_branch_3_element_tuple.rs index 32081347558..195ca2b5935 100644 --- a/tests/mir-opt/early_otherwise_branch_3_element_tuple.rs +++ b/tests/mir-opt/early_otherwise_branch_3_element_tuple.rs @@ -1,14 +1,35 @@ // skip-filecheck //@ unit-test: EarlyOtherwiseBranch +//@ compile-flags: -Zmir-enable-passes=+UnreachableEnumBranching +enum Option2 { + Some(T), + None, + Other, +} + +// FIXME: `switchInt` will have three targets after `UnreachableEnumBranching`, +// otherwise is unreachable. We can consume the UB fact to transform back to if else pattern. // EMIT_MIR early_otherwise_branch_3_element_tuple.opt1.EarlyOtherwiseBranch.diff fn opt1(x: Option, y: Option, z: Option) -> u32 { match (x, y, z) { (Some(a), Some(b), Some(c)) => 0, + (None, None, None) => 2, + _ => 1, + } +} + +// EMIT_MIR early_otherwise_branch_3_element_tuple.opt2.EarlyOtherwiseBranch.diff +fn opt2(x: Option2, y: Option2, z: Option2) -> u32 { + match (x, y, z) { + (Option2::Some(a), Option2::Some(b), Option2::Some(c)) => 0, + (Option2::None, Option2::None, Option2::None) => 2, + (Option2::Other, Option2::Other, Option2::Other) => 3, _ => 1, } } fn main() { opt1(None, Some(0), None); + opt2(Option2::None, Option2::Some(0), Option2::None); } diff --git a/tests/mir-opt/early_otherwise_branch_68867.rs b/tests/mir-opt/early_otherwise_branch_68867.rs index 805d21533c5..36e02a64e29 100644 --- a/tests/mir-opt/early_otherwise_branch_68867.rs +++ b/tests/mir-opt/early_otherwise_branch_68867.rs @@ -1,5 +1,6 @@ // skip-filecheck //@ unit-test: EarlyOtherwiseBranch +//@ compile-flags: -Zmir-enable-passes=+UnreachableEnumBranching // FIXME: This test was broken by the derefer change. diff --git a/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff index a5b5659a31a..de12fe8f120 100644 --- a/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff @@ -78,7 +78,7 @@ StorageDead(_5); _34 = deref_copy (_4.0: &ViewportPercentageLength); _11 = discriminant((*_34)); - switchInt(move _11) -> [0: bb2, 1: bb3, 2: bb4, 3: bb5, otherwise: bb1]; + switchInt(move _11) -> [0: bb2, 1: bb3, 2: bb4, 3: bb5, otherwise: bb12]; } bb1: { @@ -213,5 +213,9 @@ bb11: { return; } + + bb12: { + unreachable; + } } diff --git a/tests/mir-opt/early_otherwise_branch_noopt.rs b/tests/mir-opt/early_otherwise_branch_noopt.rs index 648089e2df1..0d14f70c639 100644 --- a/tests/mir-opt/early_otherwise_branch_noopt.rs +++ b/tests/mir-opt/early_otherwise_branch_noopt.rs @@ -1,5 +1,6 @@ // skip-filecheck //@ unit-test: EarlyOtherwiseBranch +//@ compile-flags: -Zmir-enable-passes=+UnreachableEnumBranching // must not optimize as it does not follow the pattern of // left and right hand side being the same variant diff --git a/tests/mir-opt/early_otherwise_branch_soundness.no_deref_ptr.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_soundness.no_deref_ptr.EarlyOtherwiseBranch.diff index b24ff6ec74b..8eab59823f4 100644 --- a/tests/mir-opt/early_otherwise_branch_soundness.no_deref_ptr.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_soundness.no_deref_ptr.EarlyOtherwiseBranch.diff @@ -14,7 +14,7 @@ bb0: { _3 = discriminant(_1); - switchInt(move _3) -> [1: bb2, otherwise: bb1]; + switchInt(move _3) -> [1: bb2, 0: bb1, otherwise: bb6]; } bb1: { @@ -24,7 +24,7 @@ bb2: { _4 = discriminant((*_2)); - switchInt(move _4) -> [1: bb4, otherwise: bb3]; + switchInt(move _4) -> [1: bb4, 0: bb3, otherwise: bb6]; } bb3: { @@ -43,5 +43,9 @@ bb5: { return; } + + bb6: { + unreachable; + } } diff --git a/tests/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff index c3ea975ce03..6a4c947b882 100644 --- a/tests/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff +++ b/tests/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff @@ -12,13 +12,13 @@ bb0: { _3 = discriminant((*_1)); - switchInt(move _3) -> [1: bb1, otherwise: bb3]; + switchInt(move _3) -> [1: bb1, 0: bb3, otherwise: bb5]; } bb1: { _4 = deref_copy (((*_1) as Some).0: &E<'_>); _2 = discriminant((*_4)); - switchInt(move _2) -> [1: bb2, otherwise: bb3]; + switchInt(move _2) -> [1: bb2, 0: bb3, otherwise: bb5]; } bb2: { @@ -34,5 +34,9 @@ bb4: { return; } + + bb5: { + unreachable; + } } diff --git a/tests/mir-opt/early_otherwise_branch_soundness.rs b/tests/mir-opt/early_otherwise_branch_soundness.rs index b4f5821c420..d78922efad0 100644 --- a/tests/mir-opt/early_otherwise_branch_soundness.rs +++ b/tests/mir-opt/early_otherwise_branch_soundness.rs @@ -1,5 +1,6 @@ // skip-filecheck //@ unit-test: EarlyOtherwiseBranch +//@ compile-flags: -Zmir-enable-passes=+UnreachableEnumBranching // Tests various cases that the `early_otherwise_branch` opt should *not* optimize