Skip to content

Commit af2bcaf

Browse files
dianqkdianqk
committed
GVN: Avoid merging borrows from the dereferenced value
1 parent 65d6893 commit af2bcaf

8 files changed

Lines changed: 175 additions & 125 deletions

‎compiler/rustc_mir_transform/src/gvn.rs‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,13 @@ enum Value<'a, 'tcx> {
237237

238238
// Extractions.
239239
/// This is the *value* obtained by projecting another value.
240-
Projection(VnIndex, ProjectionElem<VnIndex, ()>),
240+
Projection {
241+
base: VnIndex,
242+
elem: ProjectionElem<VnIndex, ()>,
243+
/// Some values may be a borrow or pointer.
244+
/// Give them a different provenance, so we don't merge them.
245+
provenance: Option<VnOpaque>,
246+
},
241247
/// Discriminant of the given value.
242248
Discriminant(VnIndex),
243249

@@ -286,6 +292,7 @@ impl<'a, 'tcx> ValueSet<'a, 'tcx> {
286292
debug_assert!(match value {
287293
Value::Opaque(_) | Value::Address { .. } => true,
288294
Value::Constant { disambiguator, .. } => disambiguator.is_some(),
295+
Value::Projection { provenance, .. } => provenance.is_some(),
289296
_ => false,
290297
});
291298

@@ -527,7 +534,21 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
527534
}
528535

529536
fn insert_deref(&mut self, ty: Ty<'tcx>, value: VnIndex) -> VnIndex {
530-
self.insert(ty, Value::Projection(value, ProjectionElem::Deref))
537+
if ty.is_any_ptr() {
538+
// Give each borrow and pointer a different provenance, so we don't merge them.
539+
let index = self.insert_unique(ty, |provenance| Value::Projection {
540+
base: value,
541+
elem: ProjectionElem::Deref,
542+
provenance: Some(provenance),
543+
});
544+
self.evaluated[index] = Some(None);
545+
index
546+
} else {
547+
self.insert(
548+
ty,
549+
Value::Projection { base: value, elem: ProjectionElem::Deref, provenance: None },
550+
)
551+
}
531552
}
532553

533554
#[instrument(level = "trace", skip(self), ret)]
@@ -625,7 +646,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
625646
ImmTy::from_immediate(ptr_imm, ty).into()
626647
}
627648

628-
Projection(base, elem) => {
649+
Projection { base, elem, .. } => {
629650
let base = self.eval_to_const(base)?;
630651
// `Index` by constants should have been replaced by `ConstantIndex` by
631652
// `simplify_place_projection`.
@@ -803,8 +824,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
803824
ProjectionElem::Field(f, _) => match self.get(value) {
804825
Value::Aggregate(_, fields) => return Some((projection_ty, fields[f.as_usize()])),
805826
Value::Union(active, field) if active == f => return Some((projection_ty, field)),
806-
Value::Projection(outer_value, ProjectionElem::Downcast(_, read_variant))
807-
if let Value::Aggregate(written_variant, fields) = self.get(outer_value)
827+
Value::Projection {
828+
base, elem: ProjectionElem::Downcast(_, read_variant), ..
829+
} if let Value::Aggregate(written_variant, fields) = self.get(base)
808830
// This pass is not aware of control-flow, so we do not know whether the
809831
// replacement we are doing is actually reachable. We could be in any arm of
810832
// ```
@@ -857,7 +879,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
857879
ProjectionElem::UnwrapUnsafeBinder(_) => ProjectionElem::UnwrapUnsafeBinder(()),
858880
};
859881

860-
let value = self.insert(projection_ty.ty, Value::Projection(value, proj));
882+
let value = self.insert(
883+
projection_ty.ty,
884+
Value::Projection { base: value, elem: proj, provenance: None },
885+
);
861886
Some((projection_ty, value))
862887
}
863888

@@ -1090,11 +1115,17 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
10901115
fields: &[VnIndex],
10911116
) -> Option<VnIndex> {
10921117
let Some(&first_field) = fields.first() else { return None };
1093-
let Value::Projection(copy_from_value, _) = self.get(first_field) else { return None };
1118+
let Value::Projection { base: copy_from_value, .. } = self.get(first_field) else {
1119+
return None;
1120+
};
10941121

10951122
// All fields must correspond one-to-one and come from the same aggregate value.
10961123
if fields.iter().enumerate().any(|(index, &v)| {
1097-
if let Value::Projection(pointer, ProjectionElem::Field(from_index, _)) = self.get(v)
1124+
if let Value::Projection {
1125+
base: pointer,
1126+
elem: ProjectionElem::Field(from_index, _),
1127+
..
1128+
} = self.get(v)
10981129
&& copy_from_value == pointer
10991130
&& from_index.index() == index
11001131
{
@@ -1106,7 +1137,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
11061137
}
11071138

11081139
let mut copy_from_local_value = copy_from_value;
1109-
if let Value::Projection(pointer, proj) = self.get(copy_from_value)
1140+
if let Value::Projection { base: pointer, elem: proj, .. } = self.get(copy_from_value)
11101141
&& let ProjectionElem::Downcast(_, read_variant) = proj
11111142
{
11121143
if variant_index == read_variant {
@@ -1819,7 +1850,7 @@ impl<'tcx> VnState<'_, '_, 'tcx> {
18191850
// If we are here, we failed to find a local, and we already have a `Deref`.
18201851
// Trying to add projections will only result in an ill-formed place.
18211852
return None;
1822-
} else if let Value::Projection(pointer, proj) = self.get(index)
1853+
} else if let Value::Projection { base: pointer, elem: proj, .. } = self.get(index)
18231854
&& (allow_complex_projection || proj.is_stable_offset())
18241855
&& let Some(proj) = self.try_as_place_elem(self.ty(index), proj, loc)
18251856
{

‎tests/mir-opt/gvn_copy_aggregate.all_copy_2.GVN.diff‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,21 @@
2929
_8 = copy (*_1);
3030
_2 = copy ((*_8).0: i32);
3131
- StorageLive(_3);
32-
- _9 = copy (*_1);
33-
- _3 = copy ((*_9).1: u64);
34-
- StorageLive(_4);
35-
- _10 = copy (*_1);
36-
- _4 = copy ((*_10).2: [i8; 3]);
3732
+ nop;
38-
+ _9 = copy _8;
39-
+ _3 = copy ((*_8).1: u64);
33+
_9 = copy (*_1);
34+
_3 = copy ((*_9).1: u64);
35+
- StorageLive(_4);
4036
+ nop;
41-
+ _10 = copy _8;
42-
+ _4 = copy ((*_8).2: [i8; 3]);
37+
_10 = copy (*_1);
38+
_4 = copy ((*_10).2: [i8; 3]);
4339
StorageLive(_5);
4440
_5 = copy _2;
4541
StorageLive(_6);
4642
_6 = copy _3;
4743
StorageLive(_7);
4844
_7 = copy _4;
4945
- _0 = AllCopy { a: move _5, b: move _6, c: move _7 };
50-
+ _0 = copy (*_8);
46+
+ _0 = AllCopy { a: copy _2, b: copy _3, c: copy _4 };
5147
StorageDead(_7);
5248
StorageDead(_6);
5349
StorageDead(_5);

‎tests/mir-opt/gvn_copy_aggregate.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,12 @@ fn all_copy(v: &AllCopy) -> AllCopy {
2424
AllCopy { a, b, c }
2525
}
2626

27+
// FIXME: For nested references, we need knowing each borrows can be merged.
2728
// EMIT_MIR gvn_copy_aggregate.all_copy_2.GVN.diff
2829
fn all_copy_2(v: &&AllCopy) -> AllCopy {
2930
// CHECK-LABEL: fn all_copy_2(
3031
// CHECK: bb0: {
31-
// CHECK-NOT: = AllCopy { {{.*}} };
32-
// CHECK: [[V1:_.*]] = copy (*_1);
33-
// CHECK: _0 = copy (*[[V1]]);
32+
// CHECK: _0 = AllCopy { {{.*}} };
3433
let a = v.a;
3534
let b = v.b;
3635
let c = v.c;

‎tests/mir-opt/pre-codegen/deref_nested_borrows.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
fn src(x: &&u8) -> bool {
44
// CHECK-LABEL: fn src(
5+
// CHECK: debug y => [[Y:_.*]];
6+
// CHECK: bb0:
7+
// CHECK: [[BORROW_u8:_.*]] = copy (*_1);
8+
// CHECK: [[Y]] = copy (*[[BORROW_u8]]);
9+
// CHECK: bb1:
10+
// BORROW_u8 outside its lifetime in bb1.
11+
// CHECK-NOT: copy (*[[BORROW_u8]]);
12+
// CHECK: copy (*_1);
513
// CHECK-NOT: _0 = const true;
614
// CHECK: _0 = Eq({{.*}}, {{.*}});
715
// CHECK-NOT: _0 = const true;

‎tests/mir-opt/pre-codegen/deref_nested_borrows.src.GVN.panic-unwind.diff‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,12 @@
2424

2525
bb1: {
2626
StorageLive(_4);
27-
- _7 = copy (*_1);
28-
- _4 = copy (*_7);
29-
+ _7 = copy _6;
30-
+ _4 = copy _2;
27+
_7 = copy (*_1);
28+
_4 = copy (*_7);
3129
StorageLive(_5);
3230
_5 = copy _2;
3331
- _0 = Eq(move _4, move _5);
34-
+ _0 = const true;
32+
+ _0 = Eq(move _4, copy _2);
3533
StorageDead(_5);
3634
StorageDead(_4);
3735
- StorageDead(_2);

‎tests/mir-opt/pre-codegen/deref_nested_borrows.src.PreCodegen.after.panic-unwind.mir‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ fn src(_1: &&u8) -> bool {
66
let mut _2: &u8;
77
let _3: u8;
88
let _4: ();
9+
let mut _5: &u8;
10+
let mut _6: u8;
911
scope 1 {
1012
debug y => _3;
1113
}
@@ -17,7 +19,11 @@ fn src(_1: &&u8) -> bool {
1719
}
1820

1921
bb1: {
20-
_0 = const true;
22+
StorageLive(_6);
23+
_5 = copy (*_1);
24+
_6 = copy (*_5);
25+
_0 = Eq(move _6, copy _3);
26+
StorageDead(_6);
2127
return;
2228
}
2329
}

0 commit comments

Comments
 (0)