Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove both StorageLive and StorageDead in CopyProp. #107524

Merged
merged 3 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions compiler/rustc_mir_transform/src/copy_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,20 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
}

fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, loc: Location) {
if let StatementKind::StorageDead(l) = stmt.kind
&& self.storage_to_remove.contains(l)
{
stmt.make_nop();
} else if let StatementKind::Assign(box (ref place, ref mut rvalue)) = stmt.kind
&& place.as_local().is_some()
{
// Do not replace assignments.
self.visit_rvalue(rvalue, loc)
} else {
self.super_statement(stmt, loc);
match stmt.kind {
// When removing storage statements, we need to remove both (#107511).
StatementKind::StorageLive(l) | StatementKind::StorageDead(l)
if self.storage_to_remove.contains(l) =>
{
stmt.make_nop()
}
StatementKind::Assign(box (ref place, ref mut rvalue))
if place.as_local().is_some() =>
{
// Do not replace assignments.
self.visit_rvalue(rvalue, loc)
}
_ => self.super_statement(stmt, loc),
}
}
}
3 changes: 0 additions & 3 deletions tests/mir-opt/const_debuginfo.main.ConstDebugInfo.diff
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,8 @@
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/const_debuginfo.rs:+1:9: +1:10
_1 = const 1_u8; // scope 0 at $DIR/const_debuginfo.rs:+1:13: +1:16
StorageLive(_2); // scope 1 at $DIR/const_debuginfo.rs:+2:9: +2:10
_2 = const 2_u8; // scope 1 at $DIR/const_debuginfo.rs:+2:13: +2:16
StorageLive(_3); // scope 2 at $DIR/const_debuginfo.rs:+3:9: +3:10
_3 = const 3_u8; // scope 2 at $DIR/const_debuginfo.rs:+3:13: +3:16
StorageLive(_4); // scope 3 at $DIR/const_debuginfo.rs:+4:9: +4:12
StorageLive(_5); // scope 3 at $DIR/const_debuginfo.rs:+4:15: +4:20
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/bad_op_mod_by_zero.rs:+1:9: +1:10
_1 = const 0_i32; // scope 0 at $DIR/bad_op_mod_by_zero.rs:+1:13: +1:14
StorageLive(_2); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:9: +2:11
- _4 = Eq(_1, const 0_i32); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/scalar_literal_propagation.rs:+1:9: +1:10
_1 = const 1_u32; // scope 0 at $DIR/scalar_literal_propagation.rs:+1:13: +1:14
StorageLive(_2); // scope 1 at $DIR/scalar_literal_propagation.rs:+2:5: +2:15
- _2 = consume(_1) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:+2:5: +2:15
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/copy-prop/cycle.main.CopyProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
}

bb1: {
StorageLive(_2); // scope 1 at $DIR/cycle.rs:+2:9: +2:10
- StorageLive(_2); // scope 1 at $DIR/cycle.rs:+2:9: +2:10
_2 = _1; // scope 1 at $DIR/cycle.rs:+2:13: +2:14
- StorageLive(_3); // scope 2 at $DIR/cycle.rs:+3:9: +3:10
- _3 = _2; // scope 2 at $DIR/cycle.rs:+3:13: +3:14
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ fn f(_1: usize) -> usize {
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/dead_stores_79191.rs:+1:9: +1:10
_2 = _1; // scope 0 at $DIR/dead_stores_79191.rs:+1:13: +1:14
_1 = const 5_usize; // scope 1 at $DIR/dead_stores_79191.rs:+2:5: +2:10
_1 = _2; // scope 1 at $DIR/dead_stores_79191.rs:+3:5: +3:10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ fn f(_1: usize) -> usize {
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/dead_stores_better.rs:+1:9: +1:10
_2 = _1; // scope 0 at $DIR/dead_stores_better.rs:+1:13: +1:14
_1 = const 5_usize; // scope 1 at $DIR/dead_stores_better.rs:+2:5: +2:10
_1 = _2; // scope 1 at $DIR/dead_stores_better.rs:+3:5: +3:10
Expand Down
146 changes: 146 additions & 0 deletions tests/mir-opt/copy-prop/issue_107511.main.CopyProp.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
- // MIR for `main` before CopyProp
+ // MIR for `main` after CopyProp

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/issue_107511.rs:+0:11: +0:11
let mut _1: i32; // in scope 0 at $DIR/issue_107511.rs:+1:9: +1:16
let _3: (); // in scope 0 at $DIR/issue_107511.rs:+6:5: +8:6
let mut _4: std::ops::Range<usize>; // in scope 0 at $DIR/issue_107511.rs:+6:14: +6:24
let mut _5: std::ops::Range<usize>; // in scope 0 at $DIR/issue_107511.rs:+6:14: +6:24
let mut _6: usize; // in scope 0 at $DIR/issue_107511.rs:+6:17: +6:24
let mut _7: &[i32]; // in scope 0 at $DIR/issue_107511.rs:+6:17: +6:24
let mut _8: &[i32; 4]; // in scope 0 at $DIR/issue_107511.rs:+6:17: +6:24
let mut _10: (); // in scope 0 at $DIR/issue_107511.rs:+0:1: +11:2
let _11: (); // in scope 0 at $DIR/issue_107511.rs:+6:14: +6:24
let mut _12: std::option::Option<usize>; // in scope 0 at $DIR/issue_107511.rs:+6:14: +6:24
let mut _13: &mut std::ops::Range<usize>; // in scope 0 at $DIR/issue_107511.rs:+6:14: +6:24
let mut _14: &mut std::ops::Range<usize>; // in scope 0 at $DIR/issue_107511.rs:+6:14: +6:24
let mut _15: isize; // in scope 0 at $DIR/issue_107511.rs:+6:5: +8:6
let mut _16: !; // in scope 0 at $DIR/issue_107511.rs:+6:5: +8:6
let mut _18: i32; // in scope 0 at $DIR/issue_107511.rs:+7:16: +7:20
let _19: usize; // in scope 0 at $DIR/issue_107511.rs:+7:18: +7:19
let mut _20: usize; // in scope 0 at $DIR/issue_107511.rs:+7:16: +7:20
let mut _21: bool; // in scope 0 at $DIR/issue_107511.rs:+7:16: +7:20
scope 1 {
debug sum => _1; // in scope 1 at $DIR/issue_107511.rs:+1:9: +1:16
let _2: [i32; 4]; // in scope 1 at $DIR/issue_107511.rs:+2:9: +2:10
scope 2 {
debug a => _2; // in scope 2 at $DIR/issue_107511.rs:+2:9: +2:10
let mut _9: std::ops::Range<usize>; // in scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
scope 3 {
debug iter => _9; // in scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
let _17: usize; // in scope 3 at $DIR/issue_107511.rs:+6:9: +6:10
scope 4 {
debug i => _17; // in scope 4 at $DIR/issue_107511.rs:+6:9: +6:10
}
}
scope 5 {
}
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/issue_107511.rs:+1:9: +1:16
_1 = const 0_i32; // scope 0 at $DIR/issue_107511.rs:+1:19: +1:20
StorageLive(_2); // scope 1 at $DIR/issue_107511.rs:+2:9: +2:10
_2 = [const 0_i32, const 10_i32, const 20_i32, const 30_i32]; // scope 1 at $DIR/issue_107511.rs:+2:13: +2:28
- StorageLive(_3); // scope 2 at $DIR/issue_107511.rs:+6:5: +8:6
StorageLive(_4); // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
StorageLive(_5); // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
StorageLive(_6); // scope 2 at $DIR/issue_107511.rs:+6:17: +6:24
StorageLive(_7); // scope 2 at $DIR/issue_107511.rs:+6:17: +6:24
StorageLive(_8); // scope 2 at $DIR/issue_107511.rs:+6:17: +6:24
_8 = &_2; // scope 2 at $DIR/issue_107511.rs:+6:17: +6:24
_7 = move _8 as &[i32] (Pointer(Unsize)); // scope 2 at $DIR/issue_107511.rs:+6:17: +6:24
StorageDead(_8); // scope 2 at $DIR/issue_107511.rs:+6:17: +6:18
_6 = core::slice::<impl [i32]>::len(move _7) -> bb1; // scope 2 at $DIR/issue_107511.rs:+6:17: +6:24
// mir::Constant
// + span: $DIR/issue_107511.rs:10:19: 10:22
// + literal: Const { ty: for<'a> fn(&'a [i32]) -> usize {core::slice::<impl [i32]>::len}, val: Value(<ZST>) }
}

bb1: {
StorageDead(_7); // scope 2 at $DIR/issue_107511.rs:+6:23: +6:24
Deinit(_5); // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
(_5.0: usize) = const 0_usize; // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
(_5.1: usize) = move _6; // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
StorageDead(_6); // scope 2 at $DIR/issue_107511.rs:+6:23: +6:24
_4 = <std::ops::Range<usize> as IntoIterator>::into_iter(move _5) -> bb2; // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
// mir::Constant
// + span: $DIR/issue_107511.rs:10:14: 10:24
// + literal: Const { ty: fn(std::ops::Range<usize>) -> <std::ops::Range<usize> as IntoIterator>::IntoIter {<std::ops::Range<usize> as IntoIterator>::into_iter}, val: Value(<ZST>) }
}

bb2: {
StorageDead(_5); // scope 2 at $DIR/issue_107511.rs:+6:23: +6:24
StorageLive(_9); // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
_9 = move _4; // scope 2 at $DIR/issue_107511.rs:+6:14: +6:24
goto -> bb3; // scope 3 at $DIR/issue_107511.rs:+6:5: +8:6
}

bb3: {
- StorageLive(_11); // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
StorageLive(_12); // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
StorageLive(_13); // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
StorageLive(_14); // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
_14 = &mut _9; // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
_13 = &mut (*_14); // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
_12 = <std::ops::Range<usize> as Iterator>::next(move _13) -> bb4; // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
// mir::Constant
// + span: $DIR/issue_107511.rs:10:14: 10:24
// + literal: Const { ty: for<'a> fn(&'a mut std::ops::Range<usize>) -> Option<<std::ops::Range<usize> as Iterator>::Item> {<std::ops::Range<usize> as Iterator>::next}, val: Value(<ZST>) }
}

bb4: {
StorageDead(_13); // scope 3 at $DIR/issue_107511.rs:+6:23: +6:24
_15 = discriminant(_12); // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
switchInt(move _15) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
}

bb5: {
- StorageLive(_17); // scope 3 at $DIR/issue_107511.rs:+6:9: +6:10
_17 = ((_12 as Some).0: usize); // scope 3 at $DIR/issue_107511.rs:+6:9: +6:10
StorageLive(_18); // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
- StorageLive(_19); // scope 4 at $DIR/issue_107511.rs:+7:18: +7:19
- _19 = _17; // scope 4 at $DIR/issue_107511.rs:+7:18: +7:19
_20 = Len(_2); // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
- _21 = Lt(_19, _20); // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
- assert(move _21, "index out of bounds: the length is {} but the index is {}", move _20, _19) -> bb8; // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
+ _21 = Lt(_17, _20); // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
+ assert(move _21, "index out of bounds: the length is {} but the index is {}", move _20, _17) -> bb8; // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
}

bb6: {
unreachable; // scope 3 at $DIR/issue_107511.rs:+6:14: +6:24
}

bb7: {
- _3 = const (); // scope 3 at $DIR/issue_107511.rs:+6:5: +8:6
StorageDead(_14); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
StorageDead(_12); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
- StorageDead(_11); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
StorageDead(_9); // scope 2 at $DIR/issue_107511.rs:+8:5: +8:6
StorageDead(_4); // scope 2 at $DIR/issue_107511.rs:+8:5: +8:6
- StorageDead(_3); // scope 2 at $DIR/issue_107511.rs:+8:5: +8:6
_0 = const (); // scope 0 at $DIR/issue_107511.rs:+0:11: +11:2
StorageDead(_2); // scope 1 at $DIR/issue_107511.rs:+11:1: +11:2
StorageDead(_1); // scope 0 at $DIR/issue_107511.rs:+11:1: +11:2
return; // scope 0 at $DIR/issue_107511.rs:+11:2: +11:2
}

bb8: {
- _18 = _2[_19]; // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
+ _18 = _2[_17]; // scope 4 at $DIR/issue_107511.rs:+7:16: +7:20
_1 = Add(_1, move _18); // scope 4 at $DIR/issue_107511.rs:+7:9: +7:20
StorageDead(_18); // scope 4 at $DIR/issue_107511.rs:+7:19: +7:20
- StorageDead(_19); // scope 4 at $DIR/issue_107511.rs:+7:20: +7:21
- _11 = const (); // scope 4 at $DIR/issue_107511.rs:+6:25: +8:6
- StorageDead(_17); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
StorageDead(_14); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
StorageDead(_12); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
- StorageDead(_11); // scope 3 at $DIR/issue_107511.rs:+8:5: +8:6
- _10 = const (); // scope 3 at $DIR/issue_107511.rs:+6:5: +8:6
goto -> bb3; // scope 3 at $DIR/issue_107511.rs:+6:5: +8:6
}
}

15 changes: 15 additions & 0 deletions tests/mir-opt/copy-prop/issue_107511.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// unit-test: CopyProp

// EMIT_MIR issue_107511.main.CopyProp.diff
fn main() {
let mut sum = 0;
let a = [0, 10, 20, 30];

// `i` is assigned in a loop. Only removing its `StorageDead` would mean that
// execution sees repeated `StorageLive`. This would be UB.
for i in 0..a.len() {
sum += a[i];
}

let _ = sum;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let _val = sum might be better, _ patterns can be quite surprising...

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
_1 = const u8::MAX; // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
StorageLive(_2); // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
_2 = const 1_u8; // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
_5 = CheckedAdd(const u8::MAX, const 1_u8); // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
assert(!move (_5.1: bool), "attempt to compute `{} + {}`, which would overflow", const u8::MAX, const 1_u8) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
}

bb6: {
StorageLive(_10); // scope 3 at $DIR/funky_arms.rs:+13:17: +13:26
_10 = ((_7 as Some).0: usize); // scope 3 at $DIR/funky_arms.rs:+13:17: +13:26
StorageLive(_11); // scope 3 at $DIR/funky_arms.rs:+15:43: +15:46
_11 = &mut (*_1); // scope 3 at $DIR/funky_arms.rs:+15:43: +15:46
Expand Down
2 changes: 0 additions & 2 deletions tests/mir-opt/issue_101973.inner.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
bb0: {
StorageLive(_2); // scope 0 at $DIR/issue_101973.rs:+1:5: +1:65
StorageLive(_3); // scope 0 at $DIR/issue_101973.rs:+1:5: +1:58
StorageLive(_4); // scope 0 at $DIR/issue_101973.rs:+1:5: +1:17
StorageLive(_12); // scope 2 at $DIR/issue_101973.rs:7:12: 7:27
StorageLive(_13); // scope 2 at $DIR/issue_101973.rs:7:12: 7:20
_14 = CheckedShr(_1, const 0_i32); // scope 2 at $DIR/issue_101973.rs:7:12: 7:20
Expand Down Expand Up @@ -63,7 +62,6 @@
StorageDead(_13); // scope 2 at $DIR/issue_101973.rs:7:26: 7:27
_4 = BitOr(const 0_u32, move _12); // scope 2 at $DIR/issue_101973.rs:7:5: 7:27
StorageDead(_12); // scope 2 at $DIR/issue_101973.rs:7:26: 7:27
StorageLive(_6); // scope 0 at $DIR/issue_101973.rs:+1:31: +1:57
StorageLive(_7); // scope 0 at $DIR/issue_101973.rs:+1:31: +1:52
StorageLive(_8); // scope 0 at $DIR/issue_101973.rs:+1:32: +1:45
_10 = CheckedShr(_1, const 8_i32); // scope 0 at $DIR/issue_101973.rs:+1:32: +1:45
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

bb0: {
StorageLive(_2); // scope 0 at $DIR/issue_76432.rs:+1:9: +1:10
StorageLive(_4); // scope 0 at $DIR/issue_76432.rs:+1:19: +1:29
StorageLive(_5); // scope 0 at $DIR/issue_76432.rs:+1:20: +1:29
_5 = [_1, _1, _1]; // scope 0 at $DIR/issue_76432.rs:+1:20: +1:29
_4 = &_5; // scope 0 at $DIR/issue_76432.rs:+1:19: +1:29
Expand Down
1 change: 0 additions & 1 deletion tests/mir-opt/simplify_match.main.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/simplify_match.rs:+1:17: +1:18
_2 = const false; // scope 0 at $DIR/simplify_match.rs:+1:21: +1:26
- switchInt(_2) -> [0: bb1, otherwise: bb2]; // scope 0 at $DIR/simplify_match.rs:+1:5: +1:31
+ switchInt(const false) -> [0: bb1, otherwise: bb2]; // scope 0 at $DIR/simplify_match.rs:+1:5: +1:31
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,16 @@
}

bb0: {
StorageLive(_3); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28
- StorageLive(_3); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28
_25 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28
_3 = &((*_25).0: usize); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28
StorageLive(_4); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31
- StorageLive(_4); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31
_26 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31
_4 = &((*_26).1: usize); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31
StorageLive(_5); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34
- StorageLive(_5); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34
_27 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34
_5 = &((*_27).2: usize); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34
StorageLive(_6); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37
- StorageLive(_6); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37
_28 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37
_6 = &((*_28).3: usize); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37
StorageLive(_7); // scope 1 at $DIR/slice_filter.rs:+0:40: +0:56
Expand Down
Loading