Invalidate all dereferences

Author: dianqk

compiler/rustc_data_structures/src/fx.rs

@@ -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) => {

compiler/rustc_mir_transform/src/gvn.rs

@@ -93,7 +93,7 @@ use rustc_const_eval::interpret::{
     ImmTy, Immediate, InterpCx, MemPlaceMeta, MemoryKind, OpTy, Projectable, Scalar,
     intern_const_alloc_for_constprop,
 };
-use rustc_data_structures::fx::FxIndexSet;
+use rustc_data_structures::fx::{FxIndexSet, MutableValues};
 use rustc_data_structures::graph::dominators::Dominators;
 use rustc_hir::def::DefKind;
 use rustc_index::bit_set::BitSet;
@@ -234,6 +234,8 @@ struct VnState<'body, 'tcx> {
     evaluated: IndexVec<VnIndex, Option<OpTy<'tcx>>>,
     /// Counter to generate different values.
     next_opaque: usize,
+    /// Cache the deref values.
+    derefs: Vec<VnIndex>,
     /// Cache the value of the `unsized_locals` features, to avoid fetching it repeatedly in a loop.
     feature_unsized_locals: bool,
     ssa: &'body SsaLocals,
@@ -266,6 +268,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()),
             evaluated: IndexVec::with_capacity(num_values),
             next_opaque: 1,
+            derefs: Vec::new(),
             feature_unsized_locals: tcx.features().unsized_locals(),
             ssa,
             dominators,
@@ -364,6 +367,21 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         self.insert(Value::Aggregate(AggregateTy::Tuple, VariantIdx::ZERO, values))
     }
 
+    fn insert_deref(&mut self, value: VnIndex) -> VnIndex {
+        let value = self.insert(Value::Projection(value, ProjectionElem::Deref));
+        self.derefs.push(value);
+        value
+    }
+
+    fn invalidate_derefs(&mut self) {
+        let mut derefs = Vec::new();
+        std::mem::swap(&mut derefs, &mut self.derefs);
+        for deref in derefs {
+            let opaque = self.next_opaque();
+            *self.values.get_index_mut2(deref.index()).unwrap() = Value::Opaque(opaque);
+        }
+    }
+
     #[instrument(level = "trace", skip(self), ret)]
     fn eval_to_const(&mut self, value: VnIndex) -> Option<OpTy<'tcx>> {
         use Value::*;
@@ -624,7 +642,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 {
                     // An immutable borrow `_x` always points to the same value for the
                     // lifetime of the borrow, so we can merge all instances of `*_x`.
-                    ProjectionElem::Deref
+                    return Some(self.insert_deref(value));
                 } else {
                     return None;
                 }
@@ -1626,6 +1644,8 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
                 self.assign(local, value);
                 Some(value)
             } else {
+                // Non-local assignments maybe invalidate deref.
+                self.invalidate_derefs();
                 value
             };
             let Some(value) = value else { return };
@@ -1645,12 +1665,16 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
     }
 
     fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) {
-        if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator
-            && let Some(local) = destination.as_local()
-            && self.ssa.is_ssa(local)
-        {
-            let opaque = self.new_opaque();
-            self.assign(local, opaque);
+        if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator {
+            if let Some(local) = destination.as_local()
+                && self.ssa.is_ssa(local)
+            {
+                let opaque = self.new_opaque();
+                self.assign(local, opaque);
+            }
+            // Function calls maybe invalidate nested deref, and non-local assignments maybe invalidate deref.
+            // Currently, no distinction is made between these two cases.
+            self.invalidate_derefs();
         }
         self.super_terminator(terminator, location);
     }

tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff

@@ -19,8 +19,7 @@
       }
 
       bb0: {
--         StorageLive(_2);
-+         nop;
+          StorageLive(_2);
           StorageLive(_3);
           StorageLive(_4);
           _4 = &_1;
@@ -50,13 +49,12 @@
           StorageLive(_9);
           _9 = copy _6;
 -         _0 = Option::<i32>::Some(move _9);
-+         _0 = copy (*_2);
++         _0 = Option::<i32>::Some(copy _6);
           StorageDead(_9);
 -         StorageDead(_6);
 +         nop;
           StorageDead(_4);
--         StorageDead(_2);
-+         nop;
+          StorageDead(_2);
           return;
       }