Auto merge of #146516 - cjgillot:dest-prop-aggregate, r=Amanieu

DestinationPropagation: avoid creating overlapping assignments.

r? `@Amanieu`

Fixes #146383

Author: bors

compiler/rustc_mir_transform/src/dest_prop.rs

@@ -567,13 +567,15 @@ fn save_as_intervals<'tcx>(
         // the written-to locals as live in the second half of the statement.
         // We also ensure that operands read by terminators conflict with writes by that terminator.
         // For instance a function call may read args after having written to the destination.
-        VisitPlacesWith(|place, ctxt| match DefUse::for_place(place, ctxt) {
-            DefUse::Def | DefUse::Use | DefUse::PartialWrite => {
-                if let Some(relevant) = relevant.shrink[place.local] {
-                    values.insert(relevant, twostep);
+        VisitPlacesWith(|place: Place<'tcx>, ctxt| {
+            if let Some(relevant) = relevant.shrink[place.local] {
+                match DefUse::for_place(place, ctxt) {
+                    DefUse::Def | DefUse::Use | DefUse::PartialWrite => {
+                        values.insert(relevant, twostep);
+                    }
+                    DefUse::NonUse => {}
                 }
             }
-            DefUse::NonUse => {}
         })
         .visit_terminator(term, loc);
 
@@ -588,15 +590,32 @@ fn save_as_intervals<'tcx>(
             twostep = TwoStepIndex::from_u32(twostep.as_u32() + 1);
             debug_assert_eq!(twostep, two_step_loc(loc, Effect::After));
             append_at(&mut values, &state, twostep);
-            // Ensure we have a non-zero live range even for dead stores. This is done by marking
-            // all the written-to locals as live in the second half of the statement.
-            VisitPlacesWith(|place, ctxt| match DefUse::for_place(place, ctxt) {
-                DefUse::Def | DefUse::PartialWrite => {
-                    if let Some(relevant) = relevant.shrink[place.local] {
-                        values.insert(relevant, twostep);
+            // Like terminators, ensure we have a non-zero live range even for dead stores.
+            // Some rvalues interleave reads and writes, for instance `Rvalue::Aggregate`, see
+            // https://github.com/rust-lang/rust/issues/146383. By precaution, treat statements
+            // as behaving so by default.
+            // We make an exception for simple assignments `_a.stuff = {copy|move} _b.stuff`,
+            // as marking `_b` live here would prevent unification.
+            let is_simple_assignment = match stmt.kind {
+                StatementKind::Assign(box (
+                    lhs,
+                    Rvalue::CopyForDeref(rhs)
+                    | Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)),
+                )) => lhs.projection == rhs.projection,
+                _ => false,
+            };
+            VisitPlacesWith(|place: Place<'tcx>, ctxt| {
+                if let Some(relevant) = relevant.shrink[place.local] {
+                    match DefUse::for_place(place, ctxt) {
+                        DefUse::Def | DefUse::PartialWrite => {
+                            values.insert(relevant, twostep);
+                        }
+                        DefUse::Use if !is_simple_assignment => {
+                            values.insert(relevant, twostep);
+                        }
+                        DefUse::Use | DefUse::NonUse => {}
                     }
                 }
-                DefUse::Use | DefUse::NonUse => {}
             })
             .visit_statement(stmt, loc);
 

tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff

@@ -0,0 +1,19 @@
+- // MIR for `rewrap` before DestinationPropagation
++ // MIR for `rewrap` after DestinationPropagation
+  
+  fn rewrap() -> (u8,) {
+      let mut _0: (u8,);
+      let mut _1: (u8,);
+      let mut _2: (u8,);
+  
+      bb0: {
+-         (_1.0: u8) = const 0_u8;
+-         _0 = copy _1;
++         (_0.0: u8) = const 0_u8;
++         nop;
+          _2 = (copy (_0.0: u8),);
+          _0 = copy _2;
+          return;
+      }
+  }
+  

tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff

@@ -0,0 +1,19 @@
+- // MIR for `rewrap` before DestinationPropagation
++ // MIR for `rewrap` after DestinationPropagation
+  
+  fn rewrap() -> (u8,) {
+      let mut _0: (u8,);
+      let mut _1: (u8,);
+      let mut _2: (u8,);
+  
+      bb0: {
+-         (_1.0: u8) = const 0_u8;
+-         _0 = copy _1;
++         (_0.0: u8) = const 0_u8;
++         nop;
+          _2 = (copy (_0.0: u8),);
+          _0 = copy _2;
+          return;
+      }
+  }
+  

tests/mir-opt/dest-prop/aggregate.rs

@@ -0,0 +1,51 @@
+//@ test-mir-pass: DestinationPropagation
+// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+
+#![feature(custom_mir, core_intrinsics)]
+#![allow(internal_features)]
+
+use std::intrinsics::mir::*;
+use std::mem::MaybeUninit;
+
+fn dump_var<T>(_: T) {}
+
+// EMIT_MIR aggregate.rewrap.DestinationPropagation.diff
+#[custom_mir(dialect = "runtime")]
+fn rewrap() -> (u8,) {
+    // CHECK-LABEL: fn rewrap(
+    // CHECK: (_0.0: u8) = const 0_u8;
+    // CHECK: _2 = (copy (_0.0: u8),);
+    // CHECK: _0 = copy _2;
+    mir! {
+        let _1: (u8,);
+        let _2: (u8,);
+        {
+            _1.0 = 0;
+            RET = _1;
+            _2 = (RET.0, );
+            RET = _2;
+            Return()
+        }
+    }
+}
+
+// EMIT_MIR aggregate.swap.DestinationPropagation.diff
+#[custom_mir(dialect = "runtime")]
+fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) {
+    // CHECK-LABEL: fn swap(
+    // CHECK: _0 = const
+    // CHECK: _2 = copy _0;
+    // CHECK: _0 = (copy (_2.1: {{.*}}), copy (_2.0: {{.*}}));
+    mir! {
+        let _1: (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>);
+        let _2: (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>);
+        let _3: ();
+        {
+            _1 = const { (MaybeUninit::new([0; 10]), MaybeUninit::new([1; 10])) };
+            _2 = _1;
+            _1 = (_2.1, _2.0);
+            RET = _1;
+            Return()
+        }
+    }
+}

tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff

@@ -0,0 +1,22 @@
+- // MIR for `swap` before DestinationPropagation
++ // MIR for `swap` after DestinationPropagation
+  
+  fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) {
+      let mut _0: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>);
+      let mut _1: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>);
+      let mut _2: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>);
+      let mut _3: ();
+  
+      bb0: {
+-         _1 = const swap::{constant#6};
+-         _2 = copy _1;
+-         _1 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>));
+-         _0 = copy _1;
++         _0 = const swap::{constant#6};
++         _2 = copy _0;
++         _0 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>));
++         nop;
+          return;
+      }
+  }
+  

tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff

@@ -0,0 +1,22 @@
+- // MIR for `swap` before DestinationPropagation
++ // MIR for `swap` after DestinationPropagation
+  
+  fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) {
+      let mut _0: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>);
+      let mut _1: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>);
+      let mut _2: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>);
+      let mut _3: ();
+  
+      bb0: {
+-         _1 = const swap::{constant#6};
+-         _2 = copy _1;
+-         _1 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>));
+-         _0 = copy _1;
++         _0 = const swap::{constant#6};
++         _2 = copy _0;
++         _0 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>));
++         nop;
+          return;
+      }
+  }
+