Skip to content

Commit

Permalink
Auto merge of rust-lang#132527 - DianQK:gvn-stmt-iter, r=<try>
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@bors
bors committed Dec 21, 2024
2 parents 5f23ef7 + 199b7f9 commit 0754bc9
Show file tree
Hide file tree
Showing 38 changed files with 506 additions and 549 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_data_structures/src/fx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ pub type FxIndexSet<V> = indexmap::IndexSet<V, BuildHasherDefault<FxHasher>>;
pub type IndexEntry<'a, K, V> = indexmap::map::Entry<'a, K, V>;
pub type IndexOccupiedEntry<'a, K, V> = indexmap::map::OccupiedEntry<'a, K, V>;

pub use indexmap::set::MutableValues;

#[macro_export]
macro_rules! define_id_collections {
($map_name:ident, $set_name:ident, $entry_name:ident, $key:ty) => {
Expand Down
180 changes: 95 additions & 85 deletions compiler/rustc_mir_transform/src/gvn.rs
View file

Large diffs are not rendered by default.

37 changes: 0 additions & 37 deletions compiler/rustc_mir_transform/src/ssa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ pub(super) struct SsaLocals {
borrowed_locals: BitSet<Local>,
}

pub(super) enum AssignedValue<'a, 'tcx> {
Arg,
Rvalue(&'a mut Rvalue<'tcx>),
Terminator,
}

impl SsaLocals {
pub(super) fn new<'tcx>(
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -152,37 +146,6 @@ impl SsaLocals {
})
}

pub(super) fn for_each_assignment_mut<'tcx>(
&self,
basic_blocks: &mut IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
mut f: impl FnMut(Local, AssignedValue<'_, 'tcx>, Location),
) {
for &local in &self.assignment_order {
match self.assignments[local] {
Set1::One(DefLocation::Argument) => f(local, AssignedValue::Arg, Location {
block: START_BLOCK,
statement_index: 0,
}),
Set1::One(DefLocation::Assignment(loc)) => {
let bb = &mut basic_blocks[loc.block];
// `loc` must point to a direct assignment to `local`.
let stmt = &mut bb.statements[loc.statement_index];
let StatementKind::Assign(box (target, ref mut rvalue)) = stmt.kind else {
bug!()
};
assert_eq!(target.as_local(), Some(local));
f(local, AssignedValue::Rvalue(rvalue), loc)
}
Set1::One(DefLocation::CallReturn { call, .. }) => {
let bb = &mut basic_blocks[call];
let loc = Location { block: call, statement_index: bb.statements.len() };
f(local, AssignedValue::Terminator, loc)
}
_ => {}
}
}
}

/// Compute the equivalence classes for locals, based on copy statements.
///
/// The returned vector maps each local to the one it copies. In the following case:
Expand Down
2 changes: 0 additions & 2 deletions tests/codegen/clone_as_copy.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
//@ revisions: DEBUGINFO NODEBUGINFO
//@ compile-flags: -Zunsound-mir-opts
// FIXME: see <https://github.com/rust-lang/rust/issues/132353>
//@ compile-flags: -O -Cno-prepopulate-passes
//@ [DEBUGINFO] compile-flags: -Cdebuginfo=full

Expand Down
17 changes: 11 additions & 6 deletions tests/codegen/try_question_mark_nop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@ use std::ptr::NonNull;
#[no_mangle]
pub fn option_nop_match_32(x: Option<u32>) -> Option<u32> {
// CHECK: start:
// TWENTY-NEXT: %trunc = trunc nuw i32 %0 to i1
// TWENTY-NEXT: %.2 = select i1 %trunc, i32 %1, i32 undef
// CHECK-NEXT: [[REG1:%.*]] = insertvalue { i32, i32 } poison, i32 %0, 0
// NINETEEN-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %1, 1
// TWENTY-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %.2, 1
// CHECK-NEXT: ret { i32, i32 } [[REG2]]
// CHECK-NEXT: [[TRUNC:%.*]] = trunc nuw i32 %0 to i1

// NINETEEN-NEXT: [[SELECT:%.*]] = select i1 [[TRUNC]], i32 %0, i32 0
// NINETEEN-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } poison, i32 [[SELECT]], 0
// NINETEEN-NEXT: [[REG3:%.*]] = insertvalue { i32, i32 } [[REG2]], i32 %1, 1

// TWENTY-NEXT: [[SELECT:%.*]] = select i1 [[TRUNC]], i32 %1, i32 undef
// TWENTY-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } poison, i32 %0, 0
// TWENTY-NEXT: [[REG3:%.*]] = insertvalue { i32, i32 } [[REG2]], i32 [[SELECT]], 1

// CHECK-NEXT: ret { i32, i32 } [[REG3]]
match x {
Some(x) => Some(x),
None => None,
Expand Down
202 changes: 93 additions & 109 deletions tests/coverage/issue-84561.cov-map
View file

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-abort.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

bb0: {
StorageLive(_1);
_1 = const <bool as NeedsDrop>::NEEDS;
- _1 = const <bool as NeedsDrop>::NEEDS;
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
+ _1 = const false;
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
}

Expand Down
3 changes: 2 additions & 1 deletion tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-unwind.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

bb0: {
StorageLive(_1);
_1 = const <bool as NeedsDrop>::NEEDS;
- _1 = const <bool as NeedsDrop>::NEEDS;
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
+ _1 = const false;
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
}

Expand Down
14 changes: 9 additions & 5 deletions tests/mir-opt/const_prop/read_immutable_static.main.GVN.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,23 @@

bb0: {
StorageLive(_1);
StorageLive(_2);
- StorageLive(_2);
- StorageLive(_3);
+ nop;
+ nop;
_3 = const {ALLOC0: &u8};
_2 = copy (*_3);
- _2 = copy (*_3);
+ _2 = const 2_u8;
StorageLive(_4);
StorageLive(_5);
_5 = const {ALLOC0: &u8};
- _4 = copy (*_5);
+ _4 = copy (*_3);
_1 = Add(move _2, move _4);
- _1 = Add(move _2, move _4);
+ _4 = const 2_u8;
+ _1 = const 4_u8;
StorageDead(_4);
StorageDead(_2);
- StorageDead(_2);
+ nop;
StorageDead(_5);
- StorageDead(_3);
+ nop;
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/read_immutable_static.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ static FOO: u8 = 2;
fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug x => [[x:_.*]];
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
// COM: CHECK: [[x]] = const 4_u8;
// CHECK: [[x]] = const 4_u8;
let x = FOO + FOO;
}
3 changes: 2 additions & 1 deletion tests/mir-opt/const_prop/ref_deref.main.GVN.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
StorageLive(_2);
_4 = const main::promoted[0];
_2 = &(*_4);
_1 = copy (*_2);
- _1 = copy (*_2);
+ _1 = const 4_i32;
StorageDead(_2);
_0 = const ();
StorageDead(_1);
Expand Down
3 changes: 2 additions & 1 deletion tests/mir-opt/const_prop/ref_deref_project.main.GVN.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
StorageLive(_2);
_4 = const main::promoted[0];
_2 = &((*_4).1: i32);
_1 = copy (*_2);
- _1 = copy (*_2);
+ _1 = const 5_i32;
StorageDead(_2);
_0 = const ();
StorageDead(_1);
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/ref_deref_project.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug a => [[a:_.*]];
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
// COM: CHECK: [[a]] = const 5_i32;
// CHECK: [[a]] = const 5_i32;
let a = *(&(4, 5).1);
}
2 changes: 1 addition & 1 deletion tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = copy (*_2)[1 of 2];
+ _1 = const 2_u32;
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = copy (*_2)[1 of 2];
+ _1 = const 2_u32;
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = copy (*_2)[1 of 2];
+ _1 = const 2_u32;
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = copy (*_2)[1 of 2];
+ _1 = const 2_u32;
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
5 changes: 2 additions & 3 deletions tests/mir-opt/const_prop/slice_len.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug a => [[a:_.*]];
// CHECK: [[slice:_.*]] = copy {{.*}} as &[u32] (PointerCoercion(Unsize, AsCast));
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
// COM: CHECK: assert(const true,
// COM: CHECK: [[a]] = const 2_u32;
// CHECK: assert(const true,
// CHECK: [[a]] = const 2_u32;
let a = (&[1u32, 2, 3] as &[u32])[1];
}
3 changes: 2 additions & 1 deletion tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
}

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

bb3: {
Expand Down
3 changes: 2 additions & 1 deletion tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
}

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

bb3: {
Expand Down
18 changes: 9 additions & 9 deletions tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
let mut _3: fn(u8) -> u8;
let _5: ();
let mut _6: fn(u8) -> u8;
let mut _9: {closure@$DIR/gvn.rs:615:19: 615:21};
let mut _9: {closure@$DIR/gvn.rs:620:19: 620:21};
let _10: ();
let mut _11: fn();
let mut _13: {closure@$DIR/gvn.rs:615:19: 615:21};
let mut _13: {closure@$DIR/gvn.rs:620:19: 620:21};
let _14: ();
let mut _15: fn();
scope 1 {
debug f => _1;
let _4: fn(u8) -> u8;
scope 2 {
debug g => _4;
let _7: {closure@$DIR/gvn.rs:615:19: 615:21};
let _7: {closure@$DIR/gvn.rs:620:19: 620:21};
scope 3 {
debug closure => _7;
let _8: fn();
Expand Down Expand Up @@ -62,16 +62,16 @@
StorageDead(_6);
StorageDead(_5);
- StorageLive(_7);
- _7 = {closure@$DIR/gvn.rs:615:19: 615:21};
- _7 = {closure@$DIR/gvn.rs:620:19: 620:21};
- StorageLive(_8);
+ nop;
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
+ nop;
StorageLive(_9);
- _9 = copy _7;
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
StorageDead(_9);
StorageLive(_10);
StorageLive(_11);
Expand All @@ -88,8 +88,8 @@
StorageLive(_13);
- _13 = copy _7;
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
StorageDead(_13);
StorageLive(_14);
StorageLive(_15);
Expand Down
18 changes: 9 additions & 9 deletions tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
let mut _3: fn(u8) -> u8;
let _5: ();
let mut _6: fn(u8) -> u8;
let mut _9: {closure@$DIR/gvn.rs:615:19: 615:21};
let mut _9: {closure@$DIR/gvn.rs:620:19: 620:21};
let _10: ();
let mut _11: fn();
let mut _13: {closure@$DIR/gvn.rs:615:19: 615:21};
let mut _13: {closure@$DIR/gvn.rs:620:19: 620:21};
let _14: ();
let mut _15: fn();
scope 1 {
debug f => _1;
let _4: fn(u8) -> u8;
scope 2 {
debug g => _4;
let _7: {closure@$DIR/gvn.rs:615:19: 615:21};
let _7: {closure@$DIR/gvn.rs:620:19: 620:21};
scope 3 {
debug closure => _7;
let _8: fn();
Expand Down Expand Up @@ -62,16 +62,16 @@
StorageDead(_6);
StorageDead(_5);
- StorageLive(_7);
- _7 = {closure@$DIR/gvn.rs:615:19: 615:21};
- _7 = {closure@$DIR/gvn.rs:620:19: 620:21};
- StorageLive(_8);
+ nop;
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
+ nop;
StorageLive(_9);
- _9 = copy _7;
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
StorageDead(_9);
StorageLive(_10);
StorageLive(_11);
Expand All @@ -88,8 +88,8 @@
StorageLive(_13);
- _13 = copy _7;
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:620:19: 620:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
StorageDead(_13);
StorageLive(_14);
StorageLive(_15);
Expand Down
Loading

0 comments on commit 0754bc9

Please sign in to comment.