Remove disabled transformation from instcombine

Author: tmiasko

compiler/rustc_mir/src/transform/instcombine.rs

@@ -4,14 +4,9 @@ use crate::transform::MirPass;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::Mutability;
 use rustc_index::vec::Idx;
+use rustc_middle::mir::visit::{MutVisitor, Visitor};
 use rustc_middle::mir::{
-    visit::PlaceContext,
-    visit::{MutVisitor, Visitor},
-    Statement,
-};
-use rustc_middle::mir::{
-    BinOp, Body, BorrowKind, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem,
-    Rvalue,
+    BinOp, Body, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue,
 };
 use rustc_middle::ty::{self, TyCtxt};
 use std::mem;
@@ -90,38 +85,10 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> {
             }
         }
 
-        if let Some(place) = self.optimizations.unneeded_deref.remove(&location) {
-            if self.should_combine(rvalue, location) {
-                debug!("unneeded_deref: replacing {:?} with {:?}", rvalue, place);
-                *rvalue = Rvalue::Use(Operand::Copy(place));
-            }
-        }
-
         // We do not call super_rvalue as we are not interested in any other parts of the tree
     }
 }
 
-struct MutatingUseVisitor {
-    has_mutating_use: bool,
-    local_to_look_for: Local,
-}
-
-impl MutatingUseVisitor {
-    fn has_mutating_use_in_stmt(local: Local, stmt: &Statement<'tcx>, location: Location) -> bool {
-        let mut _self = Self { has_mutating_use: false, local_to_look_for: local };
-        _self.visit_statement(stmt, location);
-        _self.has_mutating_use
-    }
-}
-
-impl<'tcx> Visitor<'tcx> for MutatingUseVisitor {
-    fn visit_local(&mut self, local: &Local, context: PlaceContext, _: Location) {
-        if *local == self.local_to_look_for {
-            self.has_mutating_use |= context.is_mutating_use();
-        }
-    }
-}
-
 /// Finds optimization opportunities on the MIR.
 struct OptimizationFinder<'b, 'tcx> {
     body: &'b Body<'tcx>,
@@ -134,103 +101,6 @@ impl OptimizationFinder<'b, 'tcx> {
         OptimizationFinder { body, tcx, optimizations: OptimizationList::default() }
     }
 
-    fn find_deref_of_address(&mut self, rvalue: &Rvalue<'tcx>, location: Location) -> Option<()> {
-        // FIXME(#78192): This optimization can result in unsoundness.
-        if !self.tcx.sess.opts.debugging_opts.unsound_mir_opts {
-            return None;
-        }
-
-        // Look for the sequence
-        //
-        // _2 = &_1;
-        // ...
-        // _5 = (*_2);
-        //
-        // which we can replace the last statement with `_5 = _1;` to avoid the load of `_2`.
-        if let Rvalue::Use(op) = rvalue {
-            let local_being_derefed = match op.place()?.as_ref() {
-                PlaceRef { local, projection: [ProjectionElem::Deref] } => Some(local),
-                _ => None,
-            }?;
-
-            let mut dead_locals_seen = vec![];
-
-            let stmt_index = location.statement_index;
-            // Look behind for statement that assigns the local from a address of operator.
-            // 6 is chosen as a heuristic determined by seeing the number of times
-            // the optimization kicked in compiling rust std.
-            let lower_index = stmt_index.saturating_sub(6);
-            let statements_to_look_in = self.body.basic_blocks()[location.block].statements
-                [lower_index..stmt_index]
-                .iter()
-                .rev();
-            for stmt in statements_to_look_in {
-                match &stmt.kind {
-                    // Exhaustive match on statements to detect conditions that warrant we bail out of the optimization.
-                    rustc_middle::mir::StatementKind::Assign(box (l, r))
-                        if l.local == local_being_derefed =>
-                    {
-                        match r {
-                            // Looking for immutable reference e.g _local_being_deref = &_1;
-                            Rvalue::Ref(
-                                _,
-                                // Only apply the optimization if it is an immutable borrow.
-                                BorrowKind::Shared,
-                                place_taken_address_of,
-                            ) => {
-                                // Make sure that the place has not been marked dead
-                                if dead_locals_seen.contains(&place_taken_address_of.local) {
-                                    return None;
-                                }
-
-                                self.optimizations
-                                    .unneeded_deref
-                                    .insert(location, *place_taken_address_of);
-                                return Some(());
-                            }
-
-                            // We found an assignment of `local_being_deref` that is not an immutable ref, e.g the following sequence
-                            // _2 = &_1;
-                            // _3 = &5
-                            // _2 = _3;  <-- this means it is no longer valid to replace the last statement with `_5 = _1;`
-                            // _5 = (*_2);
-                            _ => return None,
-                        }
-                    }
-
-                    // Inline asm can do anything, so bail out of the optimization.
-                    rustc_middle::mir::StatementKind::LlvmInlineAsm(_) => return None,
-
-                    // Remember `StorageDead`s, as the local being marked dead could be the
-                    // place RHS we are looking for, in which case we need to abort to avoid UB
-                    // using an uninitialized place
-                    rustc_middle::mir::StatementKind::StorageDead(dead) => {
-                        dead_locals_seen.push(*dead)
-                    }
-
-                    // Check that `local_being_deref` is not being used in a mutating way which can cause misoptimization.
-                    rustc_middle::mir::StatementKind::Assign(box (_, _))
-                    | rustc_middle::mir::StatementKind::Coverage(_)
-                    | rustc_middle::mir::StatementKind::Nop
-                    | rustc_middle::mir::StatementKind::FakeRead(_, _)
-                    | rustc_middle::mir::StatementKind::StorageLive(_)
-                    | rustc_middle::mir::StatementKind::Retag(_, _)
-                    | rustc_middle::mir::StatementKind::AscribeUserType(_, _)
-                    | rustc_middle::mir::StatementKind::SetDiscriminant { .. } => {
-                        if MutatingUseVisitor::has_mutating_use_in_stmt(
-                            local_being_derefed,
-                            stmt,
-                            location,
-                        ) {
-                            return None;
-                        }
-                    }
-                }
-            }
-        }
-        Some(())
-    }
-
     fn find_unneeded_equality_comparison(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
         // find Ne(_place, false) or Ne(false, _place)
         // or   Eq(_place, true) or Eq(true, _place)
@@ -295,8 +165,6 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
             }
         }
 
-        let _ = self.find_deref_of_address(rvalue, location);
-
         self.find_unneeded_equality_comparison(rvalue, location);
 
         // We do not call super_rvalue as we are not interested in any other parts of the tree
@@ -308,22 +176,15 @@ struct OptimizationList<'tcx> {
     and_stars: FxHashSet<Location>,
     arrays_lengths: FxHashMap<Location, Constant<'tcx>>,
     unneeded_equality_comparison: FxHashMap<Location, Operand<'tcx>>,
-    unneeded_deref: FxHashMap<Location, Place<'tcx>>,
 }
 
 impl<'tcx> OptimizationList<'tcx> {
     fn is_empty(&self) -> bool {
         match self {
-            OptimizationList {
-                and_stars,
-                arrays_lengths,
-                unneeded_equality_comparison,
-                unneeded_deref,
-            } => {
+            OptimizationList { and_stars, arrays_lengths, unneeded_equality_comparison } => {
                 and_stars.is_empty()
                     && arrays_lengths.is_empty()
                     && unneeded_equality_comparison.is_empty()
-                    && unneeded_deref.is_empty()
             }
         }
     }

src/test/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir

@@ -40,7 +40,7 @@ fn foo(_1: T, _2: &i32) -> i32 {
         _9 = move (_5.1: &i32);          // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
         StorageLive(_10);                // scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
         _10 = _8;                        // scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
-        _0 = (*_8);                      // scope 3 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
+        _0 = (*_10);                     // scope 3 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
         StorageDead(_10);                // scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
         StorageDead(_9);                 // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
         StorageDead(_8);                 // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12