Skip to content

Commit 8bf926b

Browse files
dianqkdianqk
committed
Avoid merging borrows by a dereferenced value
1 parent eac4cc9 commit 8bf926b

9 files changed

+180
-136
lines changed

‎compiler/rustc_mir_transform/src/gvn.rs‎

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

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

@@ -287,6 +293,7 @@ impl<'a, 'tcx> ValueSet<'a, 'tcx> {
287293
debug_assert!(match value {
288294
Value::Opaque(_) | Value::Parameter(_) | Value::Address { .. } => true,
289295
Value::Constant { disambiguator, .. } => disambiguator.is_some(),
296+
Value::Projection { provenance, .. } => provenance.is_some(),
290297
_ => false,
291298
});
292299

@@ -532,7 +539,18 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
532539
}
533540

534541
fn insert_deref(&mut self, ty: Ty<'tcx>, value: VnIndex) -> VnIndex {
535-
self.insert(ty, Value::Projection(value, ProjectionElem::Deref))
542+
if ty.is_any_ptr() {
543+
// Give each borrow and pointer a different provenance, so we don't merge them.
544+
return self.insert_unique(ty, true, |provenance| Value::Projection {
545+
base: value,
546+
elem: ProjectionElem::Deref,
547+
provenance: Some(provenance),
548+
});
549+
}
550+
self.insert(
551+
ty,
552+
Value::Projection { base: value, elem: ProjectionElem::Deref, provenance: None },
553+
)
536554
}
537555

538556
#[instrument(level = "trace", skip(self), ret)]
@@ -615,7 +633,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
615633
ImmTy::from_immediate(ptr_imm, ty).into()
616634
}
617635

618-
Projection(base, elem) => {
636+
Projection { base, elem, .. } => {
619637
let base = self.eval_to_const(base)?;
620638
// `Index` by constants should have been replaced by `ConstantIndex` by
621639
// `simplify_place_projection`.
@@ -827,8 +845,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
827845
ProjectionElem::Field(f, _) => match self.get(value) {
828846
Value::Aggregate(_, fields) => return Some((projection_ty, fields[f.as_usize()])),
829847
Value::Union(active, field) if active == f => return Some((projection_ty, field)),
830-
Value::Projection(outer_value, ProjectionElem::Downcast(_, read_variant))
831-
if let Value::Aggregate(written_variant, fields) = self.get(outer_value)
848+
Value::Projection {
849+
base, elem: ProjectionElem::Downcast(_, read_variant), ..
850+
} if let Value::Aggregate(written_variant, fields) = self.get(base)
832851
// This pass is not aware of control-flow, so we do not know whether the
833852
// replacement we are doing is actually reachable. We could be in any arm of
834853
// ```
@@ -881,7 +900,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
881900
ProjectionElem::UnwrapUnsafeBinder(_) => ProjectionElem::UnwrapUnsafeBinder(()),
882901
};
883902

884-
let value = self.insert(projection_ty.ty, Value::Projection(value, proj));
903+
let value = self.insert(
904+
projection_ty.ty,
905+
Value::Projection { base: value, elem: proj, provenance: None },
906+
);
885907
Some((projection_ty, value))
886908
}
887909

@@ -1108,11 +1130,17 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
11081130
fields: &[VnIndex],
11091131
) -> Option<VnIndex> {
11101132
let Some(&first_field) = fields.first() else { return None };
1111-
let Value::Projection(copy_from_value, _) = self.get(first_field) else { return None };
1133+
let Value::Projection { base: copy_from_value, .. } = self.get(first_field) else {
1134+
return None;
1135+
};
11121136

11131137
// All fields must correspond one-to-one and come from the same aggregate value.
11141138
if fields.iter().enumerate().any(|(index, &v)| {
1115-
if let Value::Projection(pointer, ProjectionElem::Field(from_index, _)) = self.get(v)
1139+
if let Value::Projection {
1140+
base: pointer,
1141+
elem: ProjectionElem::Field(from_index, _),
1142+
..
1143+
} = self.get(v)
11161144
&& copy_from_value == pointer
11171145
&& from_index.index() == index
11181146
{
@@ -1124,7 +1152,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
11241152
}
11251153

11261154
let mut copy_from_local_value = copy_from_value;
1127-
if let Value::Projection(pointer, proj) = self.get(copy_from_value)
1155+
if let Value::Projection { base: pointer, elem: proj, .. } = self.get(copy_from_value)
11281156
&& let ProjectionElem::Downcast(_, read_variant) = proj
11291157
{
11301158
if variant_index == read_variant {
@@ -1829,7 +1857,7 @@ impl<'tcx> VnState<'_, '_, 'tcx> {
18291857
// If we are here, we failed to find a local, and we already have a `Deref`.
18301858
// Trying to add projections will only result in an ill-formed place.
18311859
return None;
1832-
} else if let Value::Projection(pointer, proj) = self.get(index)
1860+
} else if let Value::Projection { base: pointer, elem: proj, .. } = self.get(index)
18331861
&& (allow_complex_projection || proj.is_stable_offset())
18341862
&& let Some(proj) = self.try_as_place_elem(self.ty(index), proj, loc)
18351863
{

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,13 @@
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);

‎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-abort.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@
2424

2525
bb1: {
2626
StorageLive(_4);
27-
- _7 = copy (*_1);
28-
- _4 = copy (*_7);
29-
+ _7 = copy _6;
30-
+ _4 = copy (*_6);
27+
_7 = copy (*_1);
28+
_4 = copy (*_7);
3129
StorageLive(_5);
3230
_5 = copy _2;
3331
- _0 = Eq(move _4, move _5);

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@
2424

2525
bb1: {
2626
StorageLive(_4);
27-
- _7 = copy (*_1);
28-
- _4 = copy (*_7);
29-
+ _7 = copy _6;
30-
+ _4 = copy (*_6);
27+
_7 = copy (*_1);
28+
_4 = copy (*_7);
3129
StorageLive(_5);
3230
_5 = copy _2;
3331
- _0 = Eq(move _4, move _5);

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ fn src(_1: &&u8) -> bool {
66
let mut _2: &u8;
77
let _3: u8;
88
let _4: ();
9-
let mut _5: u8;
9+
let mut _5: &u8;
10+
let mut _6: u8;
1011
scope 1 {
1112
debug y => _3;
1213
}
@@ -18,10 +19,11 @@ fn src(_1: &&u8) -> bool {
1819
}
1920

2021
bb1: {
21-
StorageLive(_5);
22-
_5 = copy (*_2);
23-
_0 = Eq(move _5, copy _3);
24-
StorageDead(_5);
22+
StorageLive(_6);
23+
_5 = copy (*_1);
24+
_6 = copy (*_5);
25+
_0 = Eq(move _6, copy _3);
26+
StorageDead(_6);
2527
return;
2628
}
2729
}

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ fn src(_1: &&u8) -> bool {
66
let mut _2: &u8;
77
let _3: u8;
88
let _4: ();
9-
let mut _5: u8;
9+
let mut _5: &u8;
10+
let mut _6: u8;
1011
scope 1 {
1112
debug y => _3;
1213
}
@@ -18,10 +19,11 @@ fn src(_1: &&u8) -> bool {
1819
}
1920

2021
bb1: {
21-
StorageLive(_5);
22-
_5 = copy (*_2);
23-
_0 = Eq(move _5, copy _3);
24-
StorageDead(_5);
22+
StorageLive(_6);
23+
_5 = copy (*_1);
24+
_6 = copy (*_5);
25+
_0 = Eq(move _6, copy _3);
26+
StorageDead(_6);
2527
return;
2628
}
2729
}

0 commit comments

Comments
 (0)