GVN: Remove unused invalidate_derefs

Author: dianqk

compiler/rustc_middle/src/mir/terminator.rs

@@ -696,28 +696,6 @@ impl<'tcx> TerminatorKind<'tcx> {
             _ => None,
         }
     }
-
-    /// Returns true if the terminator can write to memory.
-    pub fn can_write_to_memory(&self) -> bool {
-        match self {
-            TerminatorKind::Goto { .. }
-            | TerminatorKind::SwitchInt { .. }
-            | TerminatorKind::UnwindResume
-            | TerminatorKind::UnwindTerminate(_)
-            | TerminatorKind::Return
-            | TerminatorKind::Assert { .. }
-            | TerminatorKind::CoroutineDrop
-            | TerminatorKind::FalseEdge { .. }
-            | TerminatorKind::FalseUnwind { .. }
-            | TerminatorKind::Unreachable => false,
-            TerminatorKind::Call { .. }
-            | TerminatorKind::Drop { .. }
-            | TerminatorKind::TailCall { .. }
-            // Yield writes to the resume_arg place.
-            | TerminatorKind::Yield { .. }
-            | TerminatorKind::InlineAsm { .. } => true,
-        }
-    }
 }
 
 #[derive(Copy, Clone, Debug)]

compiler/rustc_mir_transform/src/gvn.rs

@@ -129,7 +129,6 @@ impl<'tcx> crate::MirPass<'tcx> for GVN {
         let ssa = SsaLocals::new(tcx, body, typing_env);
         // Clone dominators because we need them while mutating the body.
         let dominators = body.basic_blocks.dominators().clone();
-        let maybe_loop_headers = loops::maybe_loop_headers(body);
 
         let arena = DroplessArena::default();
         let mut state =
@@ -142,11 +141,6 @@ impl<'tcx> crate::MirPass<'tcx> for GVN {
 
         let reverse_postorder = body.basic_blocks.reverse_postorder().to_vec();
         for bb in reverse_postorder {
-            // N.B. With loops, reverse postorder cannot produce a valid topological order.
-            // A statement or terminator from inside the loop, that is not processed yet, may have performed an indirect write.
-            if maybe_loop_headers.contains(bb) {
-                state.invalidate_derefs();
-            }
             let data = &mut body.basic_blocks.as_mut_preserves_cfg()[bb];
             state.visit_basic_block_data(bb, data);
         }
@@ -351,12 +345,6 @@ impl<'a, 'tcx> ValueSet<'a, 'tcx> {
     fn ty(&self, index: VnIndex) -> Ty<'tcx> {
         self.types[index]
     }
-
-    /// Replace the value associated with `index` with an opaque value.
-    #[inline]
-    fn forget(&mut self, index: VnIndex) {
-        self.values[index] = Value::Opaque(VnOpaque);
-    }
 }
 
 struct VnState<'body, 'a, 'tcx> {
@@ -375,8 +363,6 @@ struct VnState<'body, 'a, 'tcx> {
     /// - `Some(None)` are values for which computation has failed;
     /// - `Some(Some(op))` are successful computations.
     evaluated: IndexVec<VnIndex, Option<Option<&'a OpTy<'tcx>>>>,
-    /// Cache the deref values.
-    derefs: Vec<VnIndex>,
     ssa: &'body SsaLocals,
     dominators: Dominators<BasicBlock>,
     reused_locals: DenseBitSet<Local>,
@@ -409,7 +395,6 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
             rev_locals: IndexVec::with_capacity(num_values),
             values: ValueSet::new(num_values),
             evaluated: IndexVec::with_capacity(num_values),
-            derefs: Vec::new(),
             ssa,
             dominators,
             reused_locals: DenseBitSet::new_empty(local_decls.len()),
@@ -546,19 +531,8 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
         self.insert(ty, Value::Aggregate(VariantIdx::ZERO, self.arena.alloc_slice(values)))
     }
 
-    fn insert_deref(&mut self, ty: Ty<'tcx>, value: VnIndex, always_valid: bool) -> VnIndex {
-        let value = self.insert(ty, Value::Projection(value, ProjectionElem::Deref));
-        // If the borrow lifetime is the whole body, we don't need to invalidate it.
-        if !always_valid {
-            self.derefs.push(value);
-        }
-        value
-    }
-
-    fn invalidate_derefs(&mut self) {
-        for deref in std::mem::take(&mut self.derefs) {
-            self.values.forget(deref);
-        }
+    fn insert_deref(&mut self, ty: Ty<'tcx>, value: VnIndex) -> VnIndex {
+        self.insert(ty, Value::Projection(value, ProjectionElem::Deref))
     }
 
     #[instrument(level = "trace", skip(self), ret)]
@@ -831,28 +805,19 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
                     if let Value::Parameter(_) | Value::Constant { .. } = self.get(value) {
                         // An immutable borrow `_x` that is an parameter or a constant always points to the same value in the whole body,
                         // so we can merge all instances of `*_x`.
-                        return Some((
-                            projection_ty,
-                            self.insert_deref(projection_ty.ty, value, true),
-                        ));
+                        return Some((projection_ty, self.insert_deref(projection_ty.ty, value)));
                     }
 
                     // FIXME: We could introduce new deref that the immutable borrow lifetime scope is not the whole body,
-                    // but we must invalidate it when out of the lifetime scope.
+                    // but we cannot use it when out of the lifetime scope.
                     // The lifetime scope here is unrelated to storage markers, but rather does not violate the Stacked Borrows rules.
                     // Known related issues:
-                    // - https://github.com/rust-lang/rust/issues/130853 (Fixed by invalidating)
-                    // - https://github.com/rust-lang/rust/issues/132353 (Fixed by invalidating)
-                    // - https://github.com/rust-lang/rust/issues/141038 (Not fixed well)
-                    // - https://github.com/rust-lang/rust/issues/141251 (Fixed by #144477)
+                    // - https://github.com/rust-lang/rust/issues/130853
+                    // - https://github.com/rust-lang/rust/issues/132353
+                    // - https://github.com/rust-lang/rust/issues/141038
+                    // - https://github.com/rust-lang/rust/issues/141251
                     // - https://github.com/rust-lang/rust/issues/141313
-                    // - https://github.com/rust-lang/rust/pull/147607 (Fixed by invalidating)
-                    if self.tcx.sess.opts.unstable_opts.unsound_mir_opts {
-                        return Some((
-                            projection_ty,
-                            self.insert_deref(projection_ty.ty, value, false),
-                        ));
-                    }
+                    // - https://github.com/rust-lang/rust/pull/147607
                     return None;
                 } else {
                     return None;
@@ -1894,10 +1859,6 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
 
     fn visit_place(&mut self, place: &mut Place<'tcx>, context: PlaceContext, location: Location) {
         self.simplify_place_projection(place, location);
-        if context.is_mutating_use() && place.is_indirect() {
-            // Non-local mutation maybe invalidate deref.
-            self.invalidate_derefs();
-        }
         self.super_place(place, context, location);
     }
 
@@ -1927,11 +1888,6 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
             }
         }
 
-        if lhs.is_indirect() {
-            // Non-local mutation maybe invalidate deref.
-            self.invalidate_derefs();
-        }
-
         if let Some(local) = lhs.as_local()
             && self.ssa.is_ssa(local)
             && let rvalue_ty = rvalue.ty(self.local_decls, self.tcx)
@@ -1954,10 +1910,6 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
                 self.assign(local, opaque);
             }
         }
-        // Terminators that can write to memory may invalidate (nested) derefs.
-        if terminator.kind.can_write_to_memory() {
-            self.invalidate_derefs();
-        }
         self.super_terminator(terminator, location);
     }
 }