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.

Sign up for GitHub

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

Do not unify dereferences of shared borrows in GVN #132461

Closed
wants to merge 3 commits into from
Closed

Do not unify dereferences of shared borrows in GVN #132461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67ac9bf
Add test.
cjgillot Nov 1, 2024
013c9b9
Do not unify dereferences in GVN.
cjgillot Nov 1, 2024
1cecef1
Bless coverage.
cjgillot Nov 1, 2024
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
4 changes: 3 additions & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let proj = match proj {
ProjectionElem::Deref => {
let ty = place.ty(self.local_decls, self.tcx).ty;
if let Some(Mutability::Not) = ty.ref_mutability()
// unsound: https://github.com/rust-lang/rust/issues/130853
if self.tcx.sess.opts.unstable_opts.unsound_mir_opts
&& let Some(Mutability::Not) = ty.ref_mutability()
&& let Some(pointee_ty) = ty.builtin_deref(true)
&& pointee_ty.is_freeze(self.tcx, self.param_env)
{
Expand Down
18 changes: 10 additions & 8 deletions tests/coverage/closure.cov-map
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,19 @@ Number of file 0 mappings: 6
- Code(Counter(0)) at (prev + 2, 9) to (start + 0, 10)
Highest counter ID seen: c1

Function name: closure::main::{closure#18} (unused)
Raw bytes (24): 0x[01, 01, 00, 04, 00, 19, 0d, 02, 1c, 00, 02, 1d, 02, 12, 00, 02, 12, 00, 13, 00, 01, 11, 01, 0e]
Function name: closure::main::{closure#18}
Raw bytes (26): 0x[01, 01, 01, 01, 05, 04, 01, 19, 0d, 02, 1c, 05, 02, 1d, 02, 12, 02, 02, 12, 00, 13, 01, 01, 11, 01, 0e]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of expressions: 1
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
Number of file 0 mappings: 4
- Code(Zero) at (prev + 25, 13) to (start + 2, 28)
- Code(Zero) at (prev + 2, 29) to (start + 2, 18)
- Code(Zero) at (prev + 2, 18) to (start + 0, 19)
- Code(Zero) at (prev + 1, 17) to (start + 1, 14)
Highest counter ID seen: (none)
- Code(Counter(0)) at (prev + 25, 13) to (start + 2, 28)
- Code(Counter(1)) at (prev + 2, 29) to (start + 2, 18)
- Code(Expression(0, Sub)) at (prev + 2, 18) to (start + 0, 19)
= (c0 - c1)
- Code(Counter(0)) at (prev + 1, 17) to (start + 1, 14)
Highest counter ID seen: c1

Function name: closure::main::{closure#19}
Raw bytes (26): 0x[01, 01, 01, 01, 05, 04, 01, 43, 0d, 02, 1c, 05, 02, 1d, 02, 12, 02, 02, 12, 00, 13, 01, 01, 11, 01, 0e]
Expand Down
205 changes: 110 additions & 95 deletions tests/coverage/issue-84561.cov-map
View file
Open in desktop

Large diffs are not rendered by default.

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

bb0: {
StorageLive(_1);
- StorageLive(_2);
StorageLive(_2);
- StorageLive(_3);
+ nop;
+ nop;
_3 = const {ALLOC0: &u8};
- _2 = copy (*_3);
+ _2 = const 2_u8;
_2 = copy (*_3);
StorageLive(_4);
StorageLive(_5);
_5 = const {ALLOC0: &u8};
- _4 = copy (*_5);
- _1 = Add(move _2, move _4);
+ _4 = const 2_u8;
+ _1 = const 4_u8;
+ _4 = copy (*_3);
_1 = Add(move _2, move _4);
StorageDead(_4);
- StorageDead(_2);
+ nop;
StorageDead(_2);
StorageDead(_5);
- StorageDead(_3);
+ nop;
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/const_prop/read_immutable_static.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ static FOO: u8 = 2;
fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug x => [[x:_.*]];
// CHECK: [[x]] = const 4_u8;
// CHECK-NOT: [[x]] = const 4_u8;
let x = FOO + FOO;
}
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/ref_deref.main.GVN.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
StorageLive(_2);
_4 = const main::promoted[0];
_2 = &(*_4);
- _1 = copy (*_2);
+ _1 = const 4_i32;
_1 = copy (*_2);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/ref_deref_project.main.GVN.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
StorageLive(_2);
_4 = const main::promoted[0];
_2 = &((*_4).1: i32);
- _1 = copy (*_2);
+ _1 = const 5_i32;
_1 = copy (*_2);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/const_prop/ref_deref_project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug a => [[a:_.*]];
// CHECK: [[a]] = const 5_i32;
// CHECK-NOT: [[a]] = const 5_i32;
let a = *(&(4, 5).1);
}
9 changes: 4 additions & 5 deletions tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind unreachable];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind unreachable];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
9 changes: 4 additions & 5 deletions tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind continue];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind continue];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind continue];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
9 changes: 4 additions & 5 deletions tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind unreachable];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind unreachable];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
9 changes: 4 additions & 5 deletions tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind continue];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind continue];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind continue];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
4 changes: 2 additions & 2 deletions tests/mir-opt/const_prop/slice_len.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug a => [[a:_.*]];
// CHECK: [[slice:_.*]] = copy {{.*}} as &[u32] (PointerCoercion(Unsize, AsCast));
// CHECK: assert(const true,
// CHECK: [[a]] = const 2_u32;
// CHECK-NOT: assert(const true,
// CHECK-NOT: [[a]] = const 2_u32;
Copy link
Member

Choose a reason for hiding this comment

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

Seems odd to have tests asserting that an optimization does not happen. Shouldn't we instead keep the old assertions but comment them out?

Copy link
Contributor

@klensy klensy Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is COM: prefix for that, i.e. COM: CHECK: assert(const true, but it nice to add fixme for that, to not to forget what it was about.

let a = (&[1u32, 2, 3] as &[u32])[1];
}
3 changes: 1 addition & 2 deletions tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
}

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

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

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

bb3: {
Expand Down
30 changes: 10 additions & 20 deletions tests/mir-opt/gvn.dereferences.GVN.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,18 @@
StorageLive(_18);
_18 = &(*_1);
StorageLive(_19);
- StorageLive(_20);
+ nop;
StorageLive(_20);
_20 = copy (*_18);
- _19 = opaque::<u32>(move _20) -> [return: bb7, unwind unreachable];
+ _19 = opaque::<u32>(copy _20) -> [return: bb7, unwind unreachable];
_19 = opaque::<u32>(move _20) -> [return: bb7, unwind unreachable];
}

bb7: {
- StorageDead(_20);
+ nop;
StorageDead(_20);
StorageDead(_19);
StorageLive(_21);
StorageLive(_22);
- _22 = copy (*_18);
- _21 = opaque::<u32>(move _22) -> [return: bb8, unwind unreachable];
+ _22 = copy _20;
+ _21 = opaque::<u32>(copy _20) -> [return: bb8, unwind unreachable];
_22 = copy (*_18);
_21 = opaque::<u32>(move _22) -> [return: bb8, unwind unreachable];
}

bb8: {
Expand Down Expand Up @@ -157,23 +152,18 @@
StorageDead(_28);
StorageDead(_27);
StorageLive(_29);
- StorageLive(_30);
+ nop;
StorageLive(_30);
_30 = copy ((*_3).0: u32);
- _29 = opaque::<u32>(move _30) -> [return: bb12, unwind unreachable];
+ _29 = opaque::<u32>(copy _30) -> [return: bb12, unwind unreachable];
_29 = opaque::<u32>(move _30) -> [return: bb12, unwind unreachable];
}

bb12: {
- StorageDead(_30);
+ nop;
StorageDead(_30);
StorageDead(_29);
StorageLive(_31);
StorageLive(_32);
- _32 = copy ((*_3).0: u32);
- _31 = opaque::<u32>(move _32) -> [return: bb13, unwind unreachable];
+ _32 = copy _30;
+ _31 = opaque::<u32>(copy _30) -> [return: bb13, unwind unreachable];
_32 = copy ((*_3).0: u32);
_31 = opaque::<u32>(move _32) -> [return: bb13, unwind unreachable];
}

bb13: {
Expand Down
30 changes: 10 additions & 20 deletions tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,18 @@
StorageLive(_18);
_18 = &(*_1);
StorageLive(_19);
- StorageLive(_20);
+ nop;
StorageLive(_20);
_20 = copy (*_18);
- _19 = opaque::<u32>(move _20) -> [return: bb7, unwind continue];
+ _19 = opaque::<u32>(copy _20) -> [return: bb7, unwind continue];
_19 = opaque::<u32>(move _20) -> [return: bb7, unwind continue];
}

bb7: {
- StorageDead(_20);
+ nop;
StorageDead(_20);
StorageDead(_19);
StorageLive(_21);
StorageLive(_22);
- _22 = copy (*_18);
- _21 = opaque::<u32>(move _22) -> [return: bb8, unwind continue];
+ _22 = copy _20;
+ _21 = opaque::<u32>(copy _20) -> [return: bb8, unwind continue];
_22 = copy (*_18);
_21 = opaque::<u32>(move _22) -> [return: bb8, unwind continue];
}

bb8: {
Expand Down Expand Up @@ -157,23 +152,18 @@
StorageDead(_28);
StorageDead(_27);
StorageLive(_29);
- StorageLive(_30);
+ nop;
StorageLive(_30);
_30 = copy ((*_3).0: u32);
- _29 = opaque::<u32>(move _30) -> [return: bb12, unwind continue];
+ _29 = opaque::<u32>(copy _30) -> [return: bb12, unwind continue];
_29 = opaque::<u32>(move _30) -> [return: bb12, unwind continue];
}

bb12: {
- StorageDead(_30);
+ nop;
StorageDead(_30);
StorageDead(_29);
StorageLive(_31);
StorageLive(_32);
- _32 = copy ((*_3).0: u32);
- _31 = opaque::<u32>(move _32) -> [return: bb13, unwind continue];
+ _32 = copy _30;
+ _31 = opaque::<u32>(copy _30) -> [return: bb13, unwind continue];
_32 = copy ((*_3).0: u32);
_31 = opaque::<u32>(move _32) -> [return: bb13, unwind continue];
}

bb13: {
Expand Down
18 changes: 9 additions & 9 deletions tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff
View file
Open in desktop
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:614:19: 614: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:614:19: 614: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:614:19: 614: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:614:19: 614:21};
- _7 = {closure@$DIR/gvn.rs:620:19: 620:21};
- StorageLive(_8);
+ nop;
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614: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:614:19: 614:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614: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:614:19: 614:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614: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
Loading