Skip to content

Commit 0754bc9

Browse files
borsbors
committed
Auto merge of rust-lang#132527 - DianQK:gvn-stmt-iter, r=<try>
Invalidate all dereferences when encountering non-local assignments Fixes rust-lang#132353. This PR removes the computation value by traversing SSA locals through `for_each_assignment_mut`. Because the `for_each_assignment_mut` traversal skips statements which have side effects, such as dereference assignments, the computation may be unsound. Instead of `for_each_assignment_mut`, we compute values by traversing in reverse postorder. Because we compute and use the symbolic representation of values on the fly, I invalidate all old values when encountering a dereference assignment. The current approach does not prevent the optimization of a clone to a copy. In the future, we may add an alias model, or dominance information for dereference assignments, or SSA form to help GVN. r? cjgillot cc `@jieyouxu` rust-lang#132356 cc `@RalfJung` rust-lang#133474 try-job: x86_64-mingw-1
2 parents 5f23ef7 + 199b7f9 commit 0754bc9

38 files changed

+506
-549
lines changed

‎compiler/rustc_data_structures/src/fx.rs

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ pub type FxIndexSet<V> = indexmap::IndexSet<V, BuildHasherDefault<FxHasher>>;
99
pub type IndexEntry<'a, K, V> = indexmap::map::Entry<'a, K, V>;
1010
pub type IndexOccupiedEntry<'a, K, V> = indexmap::map::OccupiedEntry<'a, K, V>;
1111

12+
pub use indexmap::set::MutableValues;
13+
1214
#[macro_export]
1315
macro_rules! define_id_collections {
1416
($map_name:ident, $set_name:ident, $entry_name:ident, $key:ty) => {

‎compiler/rustc_mir_transform/src/gvn.rs

+95-85
Large diffs are not rendered by default.

‎compiler/rustc_mir_transform/src/ssa.rs

-37
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ pub(super) struct SsaLocals {
3232
borrowed_locals: BitSet<Local>,
3333
}
3434

35-
pub(super) enum AssignedValue<'a, 'tcx> {
36-
Arg,
37-
Rvalue(&'a mut Rvalue<'tcx>),
38-
Terminator,
39-
}
40-
4135
impl SsaLocals {
4236
pub(super) fn new<'tcx>(
4337
tcx: TyCtxt<'tcx>,
@@ -152,37 +146,6 @@ impl SsaLocals {
152146
})
153147
}
154148

155-
pub(super) fn for_each_assignment_mut<'tcx>(
156-
&self,
157-
basic_blocks: &mut IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
158-
mut f: impl FnMut(Local, AssignedValue<'_, 'tcx>, Location),
159-
) {
160-
for &local in &self.assignment_order {
161-
match self.assignments[local] {
162-
Set1::One(DefLocation::Argument) => f(local, AssignedValue::Arg, Location {
163-
block: START_BLOCK,
164-
statement_index: 0,
165-
}),
166-
Set1::One(DefLocation::Assignment(loc)) => {
167-
let bb = &mut basic_blocks[loc.block];
168-
// `loc` must point to a direct assignment to `local`.
169-
let stmt = &mut bb.statements[loc.statement_index];
170-
let StatementKind::Assign(box (target, ref mut rvalue)) = stmt.kind else {
171-
bug!()
172-
};
173-
assert_eq!(target.as_local(), Some(local));
174-
f(local, AssignedValue::Rvalue(rvalue), loc)
175-
}
176-
Set1::One(DefLocation::CallReturn { call, .. }) => {
177-
let bb = &mut basic_blocks[call];
178-
let loc = Location { block: call, statement_index: bb.statements.len() };
179-
f(local, AssignedValue::Terminator, loc)
180-
}
181-
_ => {}
182-
}
183-
}
184-
}
185-
186149
/// Compute the equivalence classes for locals, based on copy statements.
187150
///
188151
/// The returned vector maps each local to the one it copies. In the following case:

‎tests/codegen/clone_as_copy.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
//@ revisions: DEBUGINFO NODEBUGINFO
2-
//@ compile-flags: -Zunsound-mir-opts
3-
// FIXME: see <https://github.com/rust-lang/rust/issues/132353>
42
//@ compile-flags: -O -Cno-prepopulate-passes
53
//@ [DEBUGINFO] compile-flags: -Cdebuginfo=full
64

‎tests/codegen/try_question_mark_nop.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,17 @@ use std::ptr::NonNull;
1616
#[no_mangle]
1717
pub fn option_nop_match_32(x: Option<u32>) -> Option<u32> {
1818
// CHECK: start:
19-
// TWENTY-NEXT: %trunc = trunc nuw i32 %0 to i1
20-
// TWENTY-NEXT: %.2 = select i1 %trunc, i32 %1, i32 undef
21-
// CHECK-NEXT: [[REG1:%.*]] = insertvalue { i32, i32 } poison, i32 %0, 0
22-
// NINETEEN-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %1, 1
23-
// TWENTY-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %.2, 1
24-
// CHECK-NEXT: ret { i32, i32 } [[REG2]]
19+
// CHECK-NEXT: [[TRUNC:%.*]] = trunc nuw i32 %0 to i1
20+
21+
// NINETEEN-NEXT: [[SELECT:%.*]] = select i1 [[TRUNC]], i32 %0, i32 0
22+
// NINETEEN-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } poison, i32 [[SELECT]], 0
23+
// NINETEEN-NEXT: [[REG3:%.*]] = insertvalue { i32, i32 } [[REG2]], i32 %1, 1
24+
25+
// TWENTY-NEXT: [[SELECT:%.*]] = select i1 [[TRUNC]], i32 %1, i32 undef
26+
// TWENTY-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } poison, i32 %0, 0
27+
// TWENTY-NEXT: [[REG3:%.*]] = insertvalue { i32, i32 } [[REG2]], i32 [[SELECT]], 1
28+
29+
// CHECK-NEXT: ret { i32, i32 } [[REG3]]
2530
match x {
2631
Some(x) => Some(x),
2732
None => None,

‎tests/coverage/issue-84561.cov-map

+93-109
Large diffs are not rendered by default.

‎tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-abort.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88

99
bb0: {
1010
StorageLive(_1);
11-
_1 = const <bool as NeedsDrop>::NEEDS;
11+
- _1 = const <bool as NeedsDrop>::NEEDS;
1212
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
13+
+ _1 = const false;
1314
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
1415
}
1516

‎tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-unwind.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88

99
bb0: {
1010
StorageLive(_1);
11-
_1 = const <bool as NeedsDrop>::NEEDS;
11+
- _1 = const <bool as NeedsDrop>::NEEDS;
1212
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
13+
+ _1 = const false;
1314
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
1415
}
1516

‎tests/mir-opt/const_prop/read_immutable_static.main.GVN.diff

+9-5
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,23 @@
1414

1515
bb0: {
1616
StorageLive(_1);
17-
StorageLive(_2);
17+
- StorageLive(_2);
1818
- StorageLive(_3);
19+
+ nop;
1920
+ nop;
2021
_3 = const {ALLOC0: &u8};
21-
_2 = copy (*_3);
22+
- _2 = copy (*_3);
23+
+ _2 = const 2_u8;
2224
StorageLive(_4);
2325
StorageLive(_5);
2426
_5 = const {ALLOC0: &u8};
2527
- _4 = copy (*_5);
26-
+ _4 = copy (*_3);
27-
_1 = Add(move _2, move _4);
28+
- _1 = Add(move _2, move _4);
29+
+ _4 = const 2_u8;
30+
+ _1 = const 4_u8;
2831
StorageDead(_4);
29-
StorageDead(_2);
32+
- StorageDead(_2);
33+
+ nop;
3034
StorageDead(_5);
3135
- StorageDead(_3);
3236
+ nop;

‎tests/mir-opt/const_prop/read_immutable_static.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ static FOO: u8 = 2;
66
fn main() {
77
// CHECK-LABEL: fn main(
88
// CHECK: debug x => [[x:_.*]];
9-
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
10-
// COM: CHECK: [[x]] = const 4_u8;
9+
// CHECK: [[x]] = const 4_u8;
1110
let x = FOO + FOO;
1211
}

‎tests/mir-opt/const_prop/ref_deref.main.GVN.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
StorageLive(_2);
1717
_4 = const main::promoted[0];
1818
_2 = &(*_4);
19-
_1 = copy (*_2);
19+
- _1 = copy (*_2);
20+
+ _1 = const 4_i32;
2021
StorageDead(_2);
2122
_0 = const ();
2223
StorageDead(_1);

‎tests/mir-opt/const_prop/ref_deref_project.main.GVN.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
StorageLive(_2);
1717
_4 = const main::promoted[0];
1818
_2 = &((*_4).1: i32);
19-
_1 = copy (*_2);
19+
- _1 = copy (*_2);
20+
+ _1 = const 5_i32;
2021
StorageDead(_2);
2122
_0 = const ();
2223
StorageDead(_1);

‎tests/mir-opt/const_prop/ref_deref_project.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
fn main() {
66
// CHECK-LABEL: fn main(
77
// CHECK: debug a => [[a:_.*]];
8-
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
9-
// COM: CHECK: [[a]] = const 5_i32;
8+
// CHECK: [[a]] = const 5_i32;
109
let a = *(&(4, 5).1);
1110
}

‎tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040

4141
bb1: {
4242
- _1 = copy (*_2)[_6];
43-
+ _1 = copy (*_2)[1 of 2];
43+
+ _1 = const 2_u32;
4444
StorageDead(_6);
4545
StorageDead(_4);
4646
StorageDead(_2);

‎tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040

4141
bb1: {
4242
- _1 = copy (*_2)[_6];
43-
+ _1 = copy (*_2)[1 of 2];
43+
+ _1 = const 2_u32;
4444
StorageDead(_6);
4545
StorageDead(_4);
4646
StorageDead(_2);

‎tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040

4141
bb1: {
4242
- _1 = copy (*_2)[_6];
43-
+ _1 = copy (*_2)[1 of 2];
43+
+ _1 = const 2_u32;
4444
StorageDead(_6);
4545
StorageDead(_4);
4646
StorageDead(_2);

‎tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040

4141
bb1: {
4242
- _1 = copy (*_2)[_6];
43-
+ _1 = copy (*_2)[1 of 2];
43+
+ _1 = const 2_u32;
4444
StorageDead(_6);
4545
StorageDead(_4);
4646
StorageDead(_2);

‎tests/mir-opt/const_prop/slice_len.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ fn main() {
88
// CHECK-LABEL: fn main(
99
// CHECK: debug a => [[a:_.*]];
1010
// CHECK: [[slice:_.*]] = copy {{.*}} as &[u32] (PointerCoercion(Unsize, AsCast));
11-
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
12-
// COM: CHECK: assert(const true,
13-
// COM: CHECK: [[a]] = const 2_u32;
11+
// CHECK: assert(const true,
12+
// CHECK: [[a]] = const 2_u32;
1413
let a = (&[1u32, 2, 3] as &[u32])[1];
1514
}

‎tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
}
1919

2020
bb2: {
21-
_0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind unreachable];
21+
- _0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind unreachable];
22+
+ _0 = opaque::<T>(copy _1) -> [return: bb3, unwind unreachable];
2223
}
2324

2425
bb3: {

‎tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
}
1919

2020
bb2: {
21-
_0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind continue];
21+
- _0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind continue];
22+
+ _0 = opaque::<T>(copy _1) -> [return: bb3, unwind continue];
2223
}
2324

2425
bb3: {

‎tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@
88
let mut _3: fn(u8) -> u8;
99
let _5: ();
1010
let mut _6: fn(u8) -> u8;
11-
let mut _9: {closure@$DIR/gvn.rs:615:19: 615:21};
11+
let mut _9: {closure@$DIR/gvn.rs:620:19: 620:21};
1212
let _10: ();
1313
let mut _11: fn();
14-
let mut _13: {closure@$DIR/gvn.rs:615:19: 615:21};
14+
let mut _13: {closure@$DIR/gvn.rs:620:19: 620:21};
1515
let _14: ();
1616
let mut _15: fn();
1717
scope 1 {
1818
debug f => _1;
1919
let _4: fn(u8) -> u8;
2020
scope 2 {
2121
debug g => _4;
22-
let _7: {closure@$DIR/gvn.rs:615:19: 615:21};
22+
let _7: {closure@$DIR/gvn.rs:620:19: 620:21};
2323
scope 3 {
2424
debug closure => _7;
2525
let _8: fn();
@@ -62,16 +62,16 @@
6262
StorageDead(_6);
6363
StorageDead(_5);
6464
- StorageLive(_7);
65-
- _7 = {closure@$DIR/gvn.rs:615:19: 615:21};
65+
- _7 = {closure@$DIR/gvn.rs:620:19: 620:21};
6666
- StorageLive(_8);
6767
+ nop;
68-
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
68+
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
6969
+ nop;
7070
StorageLive(_9);
7171
- _9 = copy _7;
7272
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
73-
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
74-
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
73+
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
74+
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
7575
StorageDead(_9);
7676
StorageLive(_10);
7777
StorageLive(_11);
@@ -88,8 +88,8 @@
8888
StorageLive(_13);
8989
- _13 = copy _7;
9090
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
91-
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
92-
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
91+
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
92+
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
9393
StorageDead(_13);
9494
StorageLive(_14);
9595
StorageLive(_15);

‎tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@
88
let mut _3: fn(u8) -> u8;
99
let _5: ();
1010
let mut _6: fn(u8) -> u8;
11-
let mut _9: {closure@$DIR/gvn.rs:615:19: 615:21};
11+
let mut _9: {closure@$DIR/gvn.rs:620:19: 620:21};
1212
let _10: ();
1313
let mut _11: fn();
14-
let mut _13: {closure@$DIR/gvn.rs:615:19: 615:21};
14+
let mut _13: {closure@$DIR/gvn.rs:620:19: 620:21};
1515
let _14: ();
1616
let mut _15: fn();
1717
scope 1 {
1818
debug f => _1;
1919
let _4: fn(u8) -> u8;
2020
scope 2 {
2121
debug g => _4;
22-
let _7: {closure@$DIR/gvn.rs:615:19: 615:21};
22+
let _7: {closure@$DIR/gvn.rs:620:19: 620:21};
2323
scope 3 {
2424
debug closure => _7;
2525
let _8: fn();
@@ -62,16 +62,16 @@
6262
StorageDead(_6);
6363
StorageDead(_5);
6464
- StorageLive(_7);
65-
- _7 = {closure@$DIR/gvn.rs:615:19: 615:21};
65+
- _7 = {closure@$DIR/gvn.rs:620:19: 620:21};
6666
- StorageLive(_8);
6767
+ nop;
68-
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
68+
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
6969
+ nop;
7070
StorageLive(_9);
7171
- _9 = copy _7;
7272
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
73-
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
74-
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
73+
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
74+
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
7575
StorageDead(_9);
7676
StorageLive(_10);
7777
StorageLive(_11);
@@ -88,8 +88,8 @@
8888
StorageLive(_13);
8989
- _13 = copy _7;
9090
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
91-
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
92-
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
91+
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
92+
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
9393
StorageDead(_13);
9494
StorageLive(_14);
9595
StorageLive(_15);

0 commit comments

Comments
 (0)