GVN: Avoid merging borrows from the dereferenced value

Author: dianqk

compiler/rustc_mir_transform/src/gvn.rs

@@ -246,7 +246,13 @@ enum Value<'a, 'tcx> {
 
     // Extractions.
     /// This is the *value* obtained by projecting another value.
-    Projection(VnIndex, ProjectionElem<VnIndex, ()>),
+    Projection {
+        base: VnIndex,
+        elem: ProjectionElem<VnIndex, ()>,
+        /// Some values may be a borrow or pointer.
+        /// Give them a different provenance, so we don't merge them.
+        provenance: Option<VnOpaque>,
+    },
     /// Discriminant of the given value.
     Discriminant(VnIndex),
 
@@ -295,6 +301,7 @@ impl<'a, 'tcx> ValueSet<'a, 'tcx> {
         debug_assert!(match value {
             Value::Opaque(_) | Value::Address { .. } => true,
             Value::Constant { disambiguator, .. } => disambiguator.is_some(),
+            Value::Projection { provenance, .. } => provenance.is_some(),
             _ => false,
         });
 
@@ -539,7 +546,18 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
     }
 
     fn insert_deref(&mut self, ty: Ty<'tcx>, value: VnIndex) -> VnIndex {
-        self.insert(ty, Value::Projection(value, ProjectionElem::Deref))
+        if ty.is_any_ptr() {
+            // Give each borrow and pointer a different provenance, so we don't merge them.
+            return self.insert_unique(ty, true, |provenance| Value::Projection {
+                base: value,
+                elem: ProjectionElem::Deref,
+                provenance: Some(provenance),
+            });
+        }
+        self.insert(
+            ty,
+            Value::Projection { base: value, elem: ProjectionElem::Deref, provenance: None },
+        )
     }
 
     #[instrument(level = "trace", skip(self), ret)]
@@ -636,7 +654,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
                 ImmTy::from_immediate(ptr_imm, ty).into()
             }
 
-            Projection(base, elem) => {
+            Projection { base, elem, .. } => {
                 let base = self.eval_to_const(base)?;
                 // `Index` by constants should have been replaced by `ConstantIndex` by
                 // `simplify_place_projection`.
@@ -816,8 +834,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
             ProjectionElem::Field(f, _) => match self.get(value) {
                 Value::Aggregate(_, fields) => return Some((projection_ty, fields[f.as_usize()])),
                 Value::Union(active, field) if active == f => return Some((projection_ty, field)),
-                Value::Projection(outer_value, ProjectionElem::Downcast(_, read_variant))
-                    if let Value::Aggregate(written_variant, fields) = self.get(outer_value)
+                Value::Projection {
+                    base, elem: ProjectionElem::Downcast(_, read_variant), ..
+                } if let Value::Aggregate(written_variant, fields) = self.get(base)
                     // This pass is not aware of control-flow, so we do not know whether the
                     // replacement we are doing is actually reachable. We could be in any arm of
                     // ```
@@ -870,7 +889,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
             ProjectionElem::UnwrapUnsafeBinder(_) => ProjectionElem::UnwrapUnsafeBinder(()),
         };
 
-        let value = self.insert(projection_ty.ty, Value::Projection(value, proj));
+        let value = self.insert(
+            projection_ty.ty,
+            Value::Projection { base: value, elem: proj, provenance: None },
+        );
         Some((projection_ty, value))
     }
 
@@ -1099,11 +1121,17 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
         fields: &[VnIndex],
     ) -> Option<VnIndex> {
         let Some(&first_field) = fields.first() else { return None };
-        let Value::Projection(copy_from_value, _) = self.get(first_field) else { return None };
+        let Value::Projection { base: copy_from_value, .. } = self.get(first_field) else {
+            return None;
+        };
 
         // All fields must correspond one-to-one and come from the same aggregate value.
         if fields.iter().enumerate().any(|(index, &v)| {
-            if let Value::Projection(pointer, ProjectionElem::Field(from_index, _)) = self.get(v)
+            if let Value::Projection {
+                base: pointer,
+                elem: ProjectionElem::Field(from_index, _),
+                ..
+            } = self.get(v)
                 && copy_from_value == pointer
                 && from_index.index() == index
             {
@@ -1115,7 +1143,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
         }
 
         let mut copy_from_local_value = copy_from_value;
-        if let Value::Projection(pointer, proj) = self.get(copy_from_value)
+        if let Value::Projection { base: pointer, elem: proj, .. } = self.get(copy_from_value)
             && let ProjectionElem::Downcast(_, read_variant) = proj
         {
             if variant_index == read_variant {
@@ -1824,7 +1852,7 @@ impl<'tcx> VnState<'_, '_, 'tcx> {
                 // If we are here, we failed to find a local, and we already have a `Deref`.
                 // Trying to add projections will only result in an ill-formed place.
                 return None;
-            } else if let Value::Projection(pointer, proj) = self.get(index)
+            } else if let Value::Projection { base: pointer, elem: proj, .. } = self.get(index)
                 && (allow_complex_projection || proj.is_stable_offset())
                 && let Some(proj) = self.try_as_place_elem(self.ty(index), proj, loc)
             {

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

@@ -2,6 +2,14 @@
 
 fn src(x: &&u8) -> bool {
     // CHECK-LABEL: fn src(
+    // CHECK: debug y => [[Y:_.*]];
+    // CHECK: bb0:
+    // CHECK: [[BORROW_u8:_.*]] = copy (*_1);
+    // CHECK: [[Y]] = copy (*[[BORROW_u8]]);
+    // CHECK: bb1:
+    // BORROW_u8 outside its lifetime in bb1.
+    // CHECK-NOT: copy (*[[BORROW_u8]]);
+    // CHECK: copy (*_1);
     // CHECK-NOT: _0 = const true;
     // CHECK: _0 = Eq({{.*}}, {{.*}});
     // CHECK-NOT: _0 = const true;

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

@@ -24,10 +24,8 @@
 
       bb1: {
           StorageLive(_4);
--         _7 = copy (*_1);
--         _4 = copy (*_7);
-+         _7 = copy _6;
-+         _4 = copy (*_6);
+          _7 = copy (*_1);
+          _4 = copy (*_7);
           StorageLive(_5);
           _5 = copy _2;
 -         _0 = Eq(move _4, move _5);

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

@@ -25,12 +25,11 @@
       bb1: {
           StorageLive(_4);
           _7 = copy (*_1);
--         _4 = copy (*_7);
-+         _4 = copy _2;
+          _4 = copy (*_7);
           StorageLive(_5);
           _5 = copy _2;
 -         _0 = Eq(move _4, move _5);
-+         _0 = const true;
++         _0 = Eq(move _4, copy _2);
           StorageDead(_5);
           StorageDead(_4);
 -         StorageDead(_2);

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

@@ -6,7 +6,8 @@ fn src(_1: &&u8) -> bool {
     let mut _2: &u8;
     let _3: u8;
     let _4: ();
-    let mut _5: u8;
+    let mut _5: &u8;
+    let mut _6: u8;
     scope 1 {
         debug y => _3;
     }
@@ -18,10 +19,11 @@ fn src(_1: &&u8) -> bool {
     }
 
     bb1: {
-        StorageLive(_5);
-        _5 = copy (*_2);
-        _0 = Eq(move _5, copy _3);
-        StorageDead(_5);
+        StorageLive(_6);
+        _5 = copy (*_1);
+        _6 = copy (*_5);
+        _0 = Eq(move _6, copy _3);
+        StorageDead(_6);
         return;
     }
 }

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

@@ -6,6 +6,8 @@ fn src(_1: &&u8) -> bool {
     let mut _2: &u8;
     let _3: u8;
     let _4: ();
+    let mut _5: &u8;
+    let mut _6: u8;
     scope 1 {
         debug y => _3;
     }
@@ -17,7 +19,11 @@ fn src(_1: &&u8) -> bool {
     }
 
     bb1: {
-        _0 = const true;
+        StorageLive(_6);
+        _5 = copy (*_1);
+        _6 = copy (*_5);
+        _0 = Eq(move _6, copy _3);
+        StorageDead(_6);
         return;
     }
 }