Auto merge of #96451 - JakobDegen:dest-prop, r=tmiasko

Fix Dest Prop

Closes #82678, #79191 .

This was not originally a total re-write of the pass but is has gradually turned into one. Notable changes:

 1. Significant improvements to documentation all around. The top of the file has been extended with a more precise argument for soundness. The code should be fairly readable, and I've done my best to add useful comments wherever possible. I would very much like for the bus factor to not be one on this code.
 3. Improved handling of conflicts that are not visible in normal dataflow.  This was the cause of #79191. Handling this correctly requires us to make decision about the semantics and specifically evaluation order of basically all MIR constructs (see specifically #68364 #71117.  The way this is implemented is based on my preferred resolution to these questions around the semantics of assignment statements.
 4. Some re-architecting to improve performance. More details below.
 5. Possible future improvements to this optimization are documented, and the code is written with the needs of those improvements in mind. The hope is that adding support for more precise analyses will not require a full re-write of this opt, but just localized changes.

### Regarding Performance

The previous approach had some performance issues; letting `l` be the number of locals and `s` be the number of statements/terminators, the runtime of the pass was `O(l^2 * s)`, both in theory and in practice. This version is smarter about not calculating unnecessary things and doing more caching. Our runtime is now dominated by one invocation of `MaybeLiveLocals` for each "round," and the number of rounds is less than 5 in over 90% of cases. This means it's linear-ish in practice.

r? `@oli-obk` who reviewed the last version of this, but review from anyone else would be more than welcome

Author: bors

compiler/rustc_mir_dataflow/src/impls/init_locals.rs

@@ -19,13 +19,11 @@ use crate::{drop_flag_effects, on_all_children_bits};
 use crate::{lattice, AnalysisDomain, GenKill, GenKillAnalysis};
 
 mod borrowed_locals;
-mod init_locals;
 mod liveness;
 mod storage_liveness;
 
 pub use self::borrowed_locals::borrowed_locals;
 pub use self::borrowed_locals::MaybeBorrowedLocals;
-pub use self::init_locals::MaybeInitializedLocals;
 pub use self::liveness::MaybeLiveLocals;
 pub use self::liveness::MaybeTransitiveLiveLocals;
 pub use self::storage_liveness::{MaybeRequiresStorage, MaybeStorageLive};

compiler/rustc_mir_dataflow/src/impls/mod.rs

@@ -70,6 +70,8 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS
     for Location { block, statement_index } in patch {
         bbs[block].statements[statement_index].make_nop();
     }
+
+    crate::simplify::SimplifyLocals.run_pass(tcx, body)
 }
 
 pub struct DeadStoreElimination;

compiler/rustc_mir_transform/src/dead_store_elimination.rs

@@ -110,15 +110,16 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly {
 
         if let TerminatorKind::Call { ref args, .. } = terminator.kind {
             for arg in args {
-                if let Operand::Move(_) = *arg {
-                    // ArgumentChecker panics if a direct move of an argument from a caller to a
-                    // callee was detected.
-                    //
-                    // If, in the future, MIR optimizations cause arguments to be moved directly
-                    // from callers to callees, change the panic to instead add the argument in
-                    // question to `mutating_uses`.
-                    ArgumentChecker::new(self.mutable_args.domain_size())
-                        .visit_operand(arg, location)
+                if let Operand::Move(place) = *arg {
+                    let local = place.local;
+                    if place.is_indirect()
+                        || local == RETURN_PLACE
+                        || local.index() > self.mutable_args.domain_size()
+                    {
+                        continue;
+                    }
+
+                    self.mutable_args.insert(local.index() - 1);
                 }
             }
         };
@@ -127,35 +128,6 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly {
     }
 }
 
-/// A visitor that simply panics if a direct move of an argument from a caller to a callee was
-/// detected.
-struct ArgumentChecker {
-    /// The number of arguments to the calling function.
-    arg_count: usize,
-}
-
-impl ArgumentChecker {
-    /// Creates a new ArgumentChecker.
-    fn new(arg_count: usize) -> Self {
-        Self { arg_count }
-    }
-}
-
-impl<'tcx> Visitor<'tcx> for ArgumentChecker {
-    fn visit_local(&mut self, local: Local, context: PlaceContext, _: Location) {
-        // Check to make sure that, if this local is an argument, we didn't move directly from it.
-        if matches!(context, PlaceContext::NonMutatingUse(NonMutatingUseContext::Move))
-            && local != RETURN_PLACE
-            && local.index() <= self.arg_count
-        {
-            // If, in the future, MIR optimizations cause arguments to be moved directly from
-            // callers to callees, change this panic to instead add the argument in question to
-            // `mutating_uses`.
-            panic!("Detected a direct move from a caller's argument to a callee's argument!")
-        }
-    }
-}
-
 /// Returns true if values of a given type will never be passed indirectly, regardless of ABI.
 fn type_will_always_be_passed_directly<'tcx>(ty: Ty<'tcx>) -> bool {
     matches!(