From e5447a22222ecc6a650e75282cb9931b910854b2 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Sun, 13 Sep 2020 10:47:20 +0200 Subject: [PATCH] Fix #76432 Only insert StorageDeads if we actually removed one. Fixes an issue where we added StorageDead to a place with no StorageLive --- .../transform/simplify_comparison_integral.rs | 43 +++---- src/test/mir-opt/issue_76432.rs | 16 +++ ...76432.test.SimplifyComparisonIntegral.diff | 116 ++++++++++++++++++ 3 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 src/test/mir-opt/issue_76432.rs create mode 100644 src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff diff --git a/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs b/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs index a450a75d091ef..9b460c9ecb1be 100644 --- a/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs +++ b/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs @@ -61,26 +61,6 @@ impl<'tcx> MirPass<'tcx> for SimplifyComparisonIntegral { _ => unreachable!(), } - let terminator = bb.terminator_mut(); - - // add StorageDead for the place switched on at the top of each target - for bb_idx in new_targets.iter() { - storage_deads_to_insert.push(( - *bb_idx, - Statement { - source_info: terminator.source_info, - kind: StatementKind::StorageDead(opt.to_switch_on.local), - }, - )); - } - - terminator.kind = TerminatorKind::SwitchInt { - discr: Operand::Move(opt.to_switch_on), - switch_ty: opt.branch_value_ty, - values: vec![new_value].into(), - targets: new_targets, - }; - // delete comparison statement if it the value being switched on was moved, which means it can not be user later on if opt.can_remove_bin_op_stmt { bb.statements[opt.bin_op_stmt_idx].make_nop(); @@ -106,14 +86,35 @@ impl<'tcx> MirPass<'tcx> for SimplifyComparisonIntegral { } } + let terminator = bb.terminator(); + // remove StorageDead (if it exists) being used in the assign of the comparison for (stmt_idx, stmt) in bb.statements.iter().enumerate() { if !matches!(stmt.kind, StatementKind::StorageDead(local) if local == opt.to_switch_on.local) { continue; } - storage_deads_to_remove.push((stmt_idx, opt.bb_idx)) + storage_deads_to_remove.push((stmt_idx, opt.bb_idx)); + // if we have StorageDeads to remove then make sure to insert them at the top of each target + for bb_idx in new_targets.iter() { + storage_deads_to_insert.push(( + *bb_idx, + Statement { + source_info: terminator.source_info, + kind: StatementKind::StorageDead(opt.to_switch_on.local), + }, + )); + } } + + let terminator = bb.terminator_mut(); + + terminator.kind = TerminatorKind::SwitchInt { + discr: Operand::Move(opt.to_switch_on), + switch_ty: opt.branch_value_ty, + values: vec![new_value].into(), + targets: new_targets, + }; } for (idx, bb_idx) in storage_deads_to_remove { diff --git a/src/test/mir-opt/issue_76432.rs b/src/test/mir-opt/issue_76432.rs new file mode 100644 index 0000000000000..c8b405ca8eaaf --- /dev/null +++ b/src/test/mir-opt/issue_76432.rs @@ -0,0 +1,16 @@ +// Check that we do not insert StorageDead at each target if StorageDead was never seen + +// EMIT_MIR issue_76432.test.SimplifyComparisonIntegral.diff +use std::fmt::Debug; + +fn test(x: T) { + let v: &[T] = &[x, x, x]; + match v { + [ref v1, ref v2, ref v3] => [v1 as *const _, v2 as *const _, v3 as *const _], + _ => unreachable!(), + }; +} + +fn main() { + test(0u32); +} diff --git a/src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff b/src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff new file mode 100644 index 0000000000000..499134b69919f --- /dev/null +++ b/src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff @@ -0,0 +1,116 @@ +- // MIR for `test` before SimplifyComparisonIntegral ++ // MIR for `test` after SimplifyComparisonIntegral + + fn test(_1: T) -> () { + debug x => _1; // in scope 0 at $DIR/issue_76432.rs:6:38: 6:39 + let mut _0: (); // return place in scope 0 at $DIR/issue_76432.rs:6:44: 6:44 + let _2: &[T]; // in scope 0 at $DIR/issue_76432.rs:7:9: 7:10 + let mut _3: &[T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + let _4: &[T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + let _5: [T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:20: 7:29 + let mut _6: T; // in scope 0 at $DIR/issue_76432.rs:7:21: 7:22 + let mut _7: T; // in scope 0 at $DIR/issue_76432.rs:7:24: 7:25 + let mut _8: T; // in scope 0 at $DIR/issue_76432.rs:7:27: 7:28 + let _9: [*const T; 3]; // in scope 0 at $DIR/issue_76432.rs:8:5: 11:6 + let mut _10: usize; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33 + let mut _11: usize; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33 + let mut _12: bool; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33 + let mut _16: *const T; // in scope 0 at $DIR/issue_76432.rs:9:38: 9:52 + let mut _17: *const T; // in scope 0 at $DIR/issue_76432.rs:9:38: 9:52 + let mut _18: *const T; // in scope 0 at $DIR/issue_76432.rs:9:54: 9:68 + let mut _19: *const T; // in scope 0 at $DIR/issue_76432.rs:9:54: 9:68 + let mut _20: *const T; // in scope 0 at $DIR/issue_76432.rs:9:70: 9:84 + let mut _21: *const T; // in scope 0 at $DIR/issue_76432.rs:9:70: 9:84 + let mut _22: !; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL + scope 1 { + debug v => _2; // in scope 1 at $DIR/issue_76432.rs:7:9: 7:10 + let _13: &T; // in scope 1 at $DIR/issue_76432.rs:9:10: 9:16 + let _14: &T; // in scope 1 at $DIR/issue_76432.rs:9:18: 9:24 + let _15: &T; // in scope 1 at $DIR/issue_76432.rs:9:26: 9:32 + scope 2 { + debug v1 => _13; // in scope 2 at $DIR/issue_76432.rs:9:10: 9:16 + debug v2 => _14; // in scope 2 at $DIR/issue_76432.rs:9:18: 9:24 + debug v3 => _15; // in scope 2 at $DIR/issue_76432.rs:9:26: 9:32 + } + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/issue_76432.rs:7:9: 7:10 + StorageLive(_3); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + StorageLive(_4); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + StorageLive(_5); // scope 0 at $DIR/issue_76432.rs:7:20: 7:29 + StorageLive(_6); // scope 0 at $DIR/issue_76432.rs:7:21: 7:22 + _6 = _1; // scope 0 at $DIR/issue_76432.rs:7:21: 7:22 + StorageLive(_7); // scope 0 at $DIR/issue_76432.rs:7:24: 7:25 + _7 = _1; // scope 0 at $DIR/issue_76432.rs:7:24: 7:25 + StorageLive(_8); // scope 0 at $DIR/issue_76432.rs:7:27: 7:28 + _8 = _1; // scope 0 at $DIR/issue_76432.rs:7:27: 7:28 + _5 = [move _6, move _7, move _8]; // scope 0 at $DIR/issue_76432.rs:7:20: 7:29 + StorageDead(_8); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + StorageDead(_7); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + StorageDead(_6); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + _4 = &_5; // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + _3 = _4; // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + _2 = move _3 as &[T] (Pointer(Unsize)); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + StorageDead(_3); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + StorageDead(_4); // scope 0 at $DIR/issue_76432.rs:7:29: 7:30 + StorageLive(_9); // scope 1 at $DIR/issue_76432.rs:8:5: 11:6 + _10 = Len((*_2)); // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 + _11 = const 3_usize; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 +- _12 = Eq(move _10, const 3_usize); // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 +- switchInt(move _12) -> [false: bb1, otherwise: bb2]; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 ++ nop; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 ++ switchInt(move _10) -> [3_usize: bb2, otherwise: bb1]; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 + } + + bb1: { + StorageLive(_22); // scope 1 at $SRC_DIR/std/src/macros.rs:LL:COL + begin_panic::<&str>(const "internal error: entered unreachable code"); // scope 1 at $SRC_DIR/std/src/macros.rs:LL:COL + // mir::Constant + // + span: $SRC_DIR/std/src/macros.rs:LL:COL + // + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar()) } + // ty::Const + // + ty: &str + // + val: Value(Slice { data: Allocation { bytes: [105, 110, 116, 101, 114, 110, 97, 108, 32, 101, 114, 114, 111, 114, 58, 32, 101, 110, 116, 101, 114, 101, 100, 32, 117, 110, 114, 101, 97, 99, 104, 97, 98, 108, 101, 32, 99, 111, 100, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [1099511627775], len: Size { raw: 40 } }, size: Size { raw: 40 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 40 }) + // mir::Constant + // + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL + // + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [105, 110, 116, 101, 114, 110, 97, 108, 32, 101, 114, 114, 111, 114, 58, 32, 101, 110, 116, 101, 114, 101, 100, 32, 117, 110, 114, 101, 97, 99, 104, 97, 98, 108, 101, 32, 99, 111, 100, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [1099511627775], len: Size { raw: 40 } }, size: Size { raw: 40 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 40 }) } + } + + bb2: { + StorageLive(_13); // scope 1 at $DIR/issue_76432.rs:9:10: 9:16 + _13 = &(*_2)[0 of 3]; // scope 1 at $DIR/issue_76432.rs:9:10: 9:16 + StorageLive(_14); // scope 1 at $DIR/issue_76432.rs:9:18: 9:24 + _14 = &(*_2)[1 of 3]; // scope 1 at $DIR/issue_76432.rs:9:18: 9:24 + StorageLive(_15); // scope 1 at $DIR/issue_76432.rs:9:26: 9:32 + _15 = &(*_2)[2 of 3]; // scope 1 at $DIR/issue_76432.rs:9:26: 9:32 + StorageLive(_16); // scope 2 at $DIR/issue_76432.rs:9:38: 9:52 + StorageLive(_17); // scope 2 at $DIR/issue_76432.rs:9:38: 9:52 + _17 = &raw const (*_13); // scope 2 at $DIR/issue_76432.rs:9:38: 9:40 + _16 = _17; // scope 2 at $DIR/issue_76432.rs:9:38: 9:52 + StorageLive(_18); // scope 2 at $DIR/issue_76432.rs:9:54: 9:68 + StorageLive(_19); // scope 2 at $DIR/issue_76432.rs:9:54: 9:68 + _19 = &raw const (*_14); // scope 2 at $DIR/issue_76432.rs:9:54: 9:56 + _18 = _19; // scope 2 at $DIR/issue_76432.rs:9:54: 9:68 + StorageLive(_20); // scope 2 at $DIR/issue_76432.rs:9:70: 9:84 + StorageLive(_21); // scope 2 at $DIR/issue_76432.rs:9:70: 9:84 + _21 = &raw const (*_15); // scope 2 at $DIR/issue_76432.rs:9:70: 9:72 + _20 = _21; // scope 2 at $DIR/issue_76432.rs:9:70: 9:84 + _9 = [move _16, move _18, move _20]; // scope 2 at $DIR/issue_76432.rs:9:37: 9:85 + StorageDead(_21); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_20); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_19); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_18); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_17); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_16); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_15); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_14); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_13); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_9); // scope 1 at $DIR/issue_76432.rs:11:6: 11:7 + _0 = const (); // scope 0 at $DIR/issue_76432.rs:6:44: 12:2 + StorageDead(_5); // scope 0 at $DIR/issue_76432.rs:12:1: 12:2 + StorageDead(_2); // scope 0 at $DIR/issue_76432.rs:12:1: 12:2 + return; // scope 0 at $DIR/issue_76432.rs:12:2: 12:2 + } + } +