Auto merge of rust-lang#128299 - DianQK:clone-copy, r=<try>

Simplify the canonical clone method and the copy-like forms to copy Fixes rust-lang#128081. Currently being blocked by rust-lang#128265. `@rustbot` label +S-blocked r? `@saethlin`
rust-lang-ci · Jul 28, 2024 · e4c8ee7 · e4c8ee7
2 parents 0bb6fec + 31a1608
commit e4c8ee7
Show file tree

Hide file tree

Showing 101 changed files with 1,109 additions and 183 deletions.
diff --git a/compiler/rustc_index/src/vec.rs b/compiler/rustc_index/src/vec.rs
@@ -190,6 +190,16 @@ impl<I: Idx, T> IndexVec<I, T> {
         let min_new_len = elem.index() + 1;
         self.raw.resize_with(min_new_len, fill_value);
     }
+
+    #[inline]
+    pub fn reset_all(&mut self, elem: T)
+    where
+        T: Copy,
+    {
+        for e in self.raw.iter_mut() {
+            *e = elem;
+        }
+    }
 }
 
 /// `IndexVec` is often used as a map, so it provides some map-like APIs.

diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs
@@ -4,6 +4,7 @@ use crate::simplify::simplify_duplicate_switch_targets;
 use crate::take_array;
 use rustc_ast::attr;
 use rustc_hir::LangItem;
+use rustc_index::IndexVec;
 use rustc_middle::bug;
 use rustc_middle::mir::*;
 use rustc_middle::ty::layout;
@@ -13,9 +14,25 @@ use rustc_span::sym;
 use rustc_span::symbol::Symbol;
 use rustc_target::spec::abi::Abi;
 
-pub struct InstSimplify;
+pub enum InstSimplify {
+    BeforeInline,
+    AfterSimplifyCfg,
+}
+
+impl InstSimplify {
+    pub fn name(&self) -> &'static str {
+        match self {
+            InstSimplify::BeforeInline => "InstSimplify-before-inline",
+            InstSimplify::AfterSimplifyCfg => "InstSimplify-after-simplifycfg",
+        }
+    }
+}
 
 impl<'tcx> MirPass<'tcx> for InstSimplify {
+    fn name(&self) -> &'static str {
+        self.name()
+    }
+
     fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
         sess.mir_opt_level() > 0
     }
@@ -44,8 +61,8 @@ impl<'tcx> MirPass<'tcx> for InstSimplify {
                     _ => {}
                 }
             }
-
             ctx.simplify_primitive_clone(block.terminator.as_mut().unwrap(), &mut block.statements);
+            ctx.simplify_copy_like(&mut block.statements);
             ctx.simplify_intrinsic_assert(block.terminator.as_mut().unwrap());
             ctx.simplify_nounwind_call(block.terminator.as_mut().unwrap());
             simplify_duplicate_switch_targets(block.terminator.as_mut().unwrap());
@@ -191,6 +208,95 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
         }
     }
 
+    /// Transform `Aggregate(Adt, [(*_1).0, (*_1).1])` ==> `Copy(*_1)`.
+    /// This comes from the simplification of the clone method by `simplify_primitive_clone`.
+    fn simplify_copy_like(&self, statements: &mut Vec<Statement<'tcx>>) {
+        let mut assignments = IndexVec::from_elem(None::<Place<'tcx>>, self.local_decls);
+        for statement in statements {
+            match statement.kind {
+                StatementKind::Assign(box (dest, ref mut rvalue)) => {
+                    if let Rvalue::Aggregate(_, fields) = rvalue {
+                        let mut from_local = None;
+                        if fields.iter_enumerated().all(|(index, field)| {
+                            let Some(from_place) = field
+                                .place()
+                                .and_then(|p| p.as_local())
+                                .and_then(|l| assignments[l])
+                            else {
+                                return false;
+                            };
+                            // All fields must come from the same local.
+                            if let Some(from_local) = from_local {
+                                if from_place.local != from_local {
+                                    return false;
+                                }
+                            } else {
+                                // We can only copy the same type.
+                                let Some(from_ty) =
+                                    self.local_decls[from_place.local].ty.builtin_deref(false)
+                                else {
+                                    return false;
+                                };
+                                let dest_ty = dest.ty(self.local_decls, self.tcx).ty;
+                                if dest_ty != from_ty {
+                                    return false;
+                                };
+                                from_local = Some(from_place.local);
+                            }
+                            // For more complex scenarios, we expect to get this simplified projection within a complete pipeline.
+                            let [ProjectionElem::Deref, ProjectionElem::Field(from_index, _)] =
+                                *from_place.projection.as_slice()
+                            else {
+                                return false;
+                            };
+                            from_index == index
+                        }) {
+                            if let Some(local) = from_local {
+                                if self.should_simplify(&statement.source_info, rvalue) {
+                                    *rvalue = Rvalue::Use(Operand::Copy(Place {
+                                        local,
+                                        projection: self
+                                            .tcx
+                                            .mk_place_elems(&[ProjectionElem::Deref]),
+                                    }));
+                                }
+                            }
+                        }
+                    }
+                    // Collect available assignments, including those transformed from `Aggregate`.
+                    if let Some(local) = dest.as_local() {
+                        assignments[local] = if let Rvalue::Use(operand) = rvalue
+                            && let Some(place) = operand.place()
+                        {
+                            if let Some(rvalue_local) = place.as_local() {
+                                let place = assignments[rvalue_local];
+                                if operand.is_move() {
+                                    assignments[rvalue_local] = None;
+                                }
+                                place
+                            } else {
+                                Some(place)
+                            }
+                        } else {
+                            // This assignment generally comes from debuginfo (e.g., Ref),
+                            // but we still need to check if a local is being overridden.
+                            None
+                        };
+                    } else {
+                        // We don't handle projection, so we drop all previous assignments.
+                        assignments.reset_all(None);
+                    }
+                }
+                StatementKind::StorageLive(_)
+                | StatementKind::StorageDead(_)
+                | StatementKind::Nop => {}
+                _ => {
+                    assignments.reset_all(None);
+                }
+            }
+        }
+    }
+
     fn simplify_cast(&self, rvalue: &mut Rvalue<'tcx>) {
         if let Rvalue::Cast(kind, operand, cast_ty) = rvalue {
             let operand_ty = operand.ty(self.local_decls, self.tcx);

diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs
@@ -572,6 +572,8 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
             // Has to be done before inlining, otherwise actual call will be almost always inlined.
             // Also simple, so can just do first
             &lower_slice_len::LowerSliceLenCalls,
+            // Perform instsimplify before inline to eliminate some trivial calls (like clone shims).
+            &instsimplify::InstSimplify::BeforeInline,
             // Perform inlining, which may add a lot of code.
             &inline::Inline,
             // Code from other crates may have storage markers, so this needs to happen after inlining.
@@ -591,7 +593,8 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
             &match_branches::MatchBranchSimplification,
             // inst combine is after MatchBranchSimplification to clean up Ne(_1, false)
             &multiple_return_terminators::MultipleReturnTerminators,
-            &instsimplify::InstSimplify,
+            // After simplifycfg, it allows us to discover new opportunities for peephole optimizations.
+            &instsimplify::InstSimplify::AfterSimplifyCfg,
             &simplify::SimplifyLocals::BeforeConstProp,
             &dead_store_elimination::DeadStoreElimination::Initial,
             &gvn::GVN,

diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs
@@ -154,7 +154,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceKind<'tcx>) -> Body<
             &deref_separator::Derefer,
             &remove_noop_landing_pads::RemoveNoopLandingPads,
             &simplify::SimplifyCfg::MakeShim,
-            &instsimplify::InstSimplify,
+            &instsimplify::InstSimplify::BeforeInline,
             &abort_unwinding_calls::AbortUnwindingCalls,
             &add_call_guards::CriticalCallEdges,
         ],

diff --git a/tests/codegen/clone_as_copy.rs b/tests/codegen/clone_as_copy.rs
@@ -0,0 +1,40 @@
+//@ revisions: DEBUGINFO NODEBUGINFO
+//@ compile-flags: -O -Cno-prepopulate-passes
+//@ [DEBUGINFO] compile-flags: -Cdebuginfo=full
+
+// From https://github.com/rust-lang/rust/issues/128081.
+// Ensure that we only generate a memcpy instruction.
+
+#![crate_type = "lib"]
+
+#[derive(Clone)]
+struct SubCloneAndCopy {
+    v1: u32,
+    v2: u32,
+}
+
+#[derive(Clone)]
+struct CloneOnly {
+    v1: u8,
+    v2: u8,
+    v3: u8,
+    v4: u8,
+    v5: u8,
+    v6: u8,
+    v7: u8,
+    v8: u8,
+    v9: u8,
+    v_sub: SubCloneAndCopy,
+    v_large: [u8; 256],
+}
+
+// CHECK-LABEL: define {{.*}}@clone_only(
+#[no_mangle]
+pub fn clone_only(v: &CloneOnly) -> CloneOnly {
+    // CHECK-NOT: call {{.*}}clone
+    // CHECK-NOT: store i8
+    // CHECK-NOT: store i32
+    // CHECK: call void @llvm.memcpy
+    // CHECK-NEXT: ret void
+    v.clone()
+}
diff --git a/tests/incremental/hashes/call_expressions.rs b/tests/incremental/hashes/call_expressions.rs
@@ -162,7 +162,7 @@ pub fn change_to_ufcs() {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
+#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,optimized_mir,typeck")]
 #[rustc_clean(cfg="cfail3")]
 #[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,optimized_mir,typeck")]
 #[rustc_clean(cfg="cfail6")]

diff --git a/tests/mir-opt/const_prop/slice_len.rs b/tests/mir-opt/const_prop/slice_len.rs
@@ -1,5 +1,5 @@
 //@ test-mir-pass: GVN
-//@ compile-flags: -Zmir-enable-passes=+InstSimplify -Zdump-mir-exclude-alloc-bytes
+//@ compile-flags: -Zmir-enable-passes=+InstSimplify-after-simplifycfg -Zdump-mir-exclude-alloc-bytes
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
 // EMIT_MIR_FOR_EACH_BIT_WIDTH
 

diff --git a/tests/mir-opt/dataflow-const-prop/slice_len.rs b/tests/mir-opt/dataflow-const-prop/slice_len.rs
@@ -1,6 +1,6 @@
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
 //@ test-mir-pass: DataflowConstProp
-//@ compile-flags: -Zmir-enable-passes=+InstSimplify
+//@ compile-flags: -Zmir-enable-passes=+InstSimplify-after-simplifycfg
 // EMIT_MIR_FOR_EACH_BIT_WIDTH
 
 // EMIT_MIR slice_len.main.DataflowConstProp.diff

diff --git a/tests/mir-opt/inline/dyn_trait.get_query.Inline.panic-abort.diff b/tests/mir-opt/inline/dyn_trait.get_query.Inline.panic-abort.diff
@@ -21,14 +21,14 @@
       bb0: {
           StorageLive(_2);
           StorageLive(_3);
-          _3 = &(*_1);
+          _3 = _1;
           _2 = <Q as Query>::cache::<T>(move _3) -> [return: bb1, unwind unreachable];
       }
 
       bb1: {
           StorageDead(_3);
           StorageLive(_4);
-          _4 = &(*_2);
+          _4 = _2;
 -         _0 = try_execute_query::<<Q as Query>::C>(move _4) -> [return: bb2, unwind unreachable];
 +         StorageLive(_5);
 +         _5 = _4 as &dyn Cache<V = <Q as Query>::V> (PointerCoercion(Unsize));

diff --git a/tests/mir-opt/inline/dyn_trait.get_query.Inline.panic-unwind.diff b/tests/mir-opt/inline/dyn_trait.get_query.Inline.panic-unwind.diff
@@ -21,14 +21,14 @@
       bb0: {
           StorageLive(_2);
           StorageLive(_3);
-          _3 = &(*_1);
+          _3 = _1;
           _2 = <Q as Query>::cache::<T>(move _3) -> [return: bb1, unwind continue];
       }
 
       bb1: {
           StorageDead(_3);
           StorageLive(_4);
-          _4 = &(*_2);
+          _4 = _2;
 -         _0 = try_execute_query::<<Q as Query>::C>(move _4) -> [return: bb2, unwind continue];
 +         StorageLive(_5);
 +         _5 = _4 as &dyn Cache<V = <Q as Query>::V> (PointerCoercion(Unsize));

diff --git a/tests/mir-opt/inline/dyn_trait.mk_cycle.Inline.panic-abort.diff b/tests/mir-opt/inline/dyn_trait.mk_cycle.Inline.panic-abort.diff
@@ -8,7 +8,7 @@
 
       bb0: {
           StorageLive(_2);
-          _2 = &(*_1);
+          _2 = _1;
           _0 = <dyn Cache<V = V> as Cache>::store_nocache(move _2) -> [return: bb1, unwind unreachable];
       }
 

diff --git a/tests/mir-opt/inline/dyn_trait.mk_cycle.Inline.panic-unwind.diff b/tests/mir-opt/inline/dyn_trait.mk_cycle.Inline.panic-unwind.diff
@@ -8,7 +8,7 @@
 
       bb0: {
           StorageLive(_2);
-          _2 = &(*_1);
+          _2 = _1;
           _0 = <dyn Cache<V = V> as Cache>::store_nocache(move _2) -> [return: bb1, unwind continue];
       }
 

diff --git a/tests/mir-opt/inline/dyn_trait.try_execute_query.Inline.panic-abort.diff b/tests/mir-opt/inline/dyn_trait.try_execute_query.Inline.panic-abort.diff
@@ -13,7 +13,7 @@
       bb0: {
           StorageLive(_2);
           StorageLive(_3);
-          _3 = &(*_1);
+          _3 = _1;
           _2 = move _3 as &dyn Cache<V = <C as Cache>::V> (PointerCoercion(Unsize));
           StorageDead(_3);
 -         _0 = mk_cycle::<<C as Cache>::V>(move _2) -> [return: bb1, unwind unreachable];

diff --git a/tests/mir-opt/inline/dyn_trait.try_execute_query.Inline.panic-unwind.diff b/tests/mir-opt/inline/dyn_trait.try_execute_query.Inline.panic-unwind.diff
@@ -13,7 +13,7 @@
       bb0: {
           StorageLive(_2);
           StorageLive(_3);
-          _3 = &(*_1);
+          _3 = _1;
           _2 = move _3 as &dyn Cache<V = <C as Cache>::V> (PointerCoercion(Unsize));
           StorageDead(_3);
 -         _0 = mk_cycle::<<C as Cache>::V>(move _2) -> [return: bb1, unwind continue];

diff --git a/tests/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir b/tests/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir
@@ -26,9 +26,9 @@ fn foo(_1: T, _2: &i32) -> i32 {
         _4 = &_3;
         StorageLive(_5);
         StorageLive(_6);
-        _6 = &(*_2);
+        _6 = _2;
         StorageLive(_7);
-        _7 = &(*_2);
+        _7 = _2;
         _5 = (move _6, move _7);
         StorageLive(_8);
         _8 = move (_5.0: &i32);

diff --git a/tests/mir-opt/inline/inline_retag.bar.Inline.after.mir b/tests/mir-opt/inline/inline_retag.bar.Inline.after.mir
@@ -31,14 +31,14 @@ fn bar() -> bool {
         StorageLive(_4);
         _10 = const bar::promoted[1];
         Retag(_10);
-        _4 = &(*_10);
-        _3 = &(*_4);
+        _4 = _10;
+        _3 = _4;
         StorageLive(_6);
         StorageLive(_7);
         _9 = const bar::promoted[0];
         Retag(_9);
-        _7 = &(*_9);
-        _6 = &(*_7);
+        _7 = _9;
+        _6 = _7;
         Retag(_3);
         Retag(_6);
         StorageLive(_11);

diff --git a/tests/mir-opt/inline/inline_trait_method.test.Inline.after.panic-abort.mir b/tests/mir-opt/inline/inline_trait_method.test.Inline.after.panic-abort.mir
@@ -7,7 +7,7 @@ fn test(_1: &dyn X) -> u32 {
 
     bb0: {
         StorageLive(_2);
-        _2 = &(*_1);
+        _2 = _1;
         _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind unreachable];
     }
 

diff --git a/tests/mir-opt/inline/inline_trait_method.test.Inline.after.panic-unwind.mir b/tests/mir-opt/inline/inline_trait_method.test.Inline.after.panic-unwind.mir
@@ -7,7 +7,7 @@ fn test(_1: &dyn X) -> u32 {
 
     bb0: {
         StorageLive(_2);
-        _2 = &(*_1);
+        _2 = _1;
         _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind continue];
     }
 

diff --git a/tests/mir-opt/inline/inline_trait_method_2.test2.Inline.after.panic-abort.mir b/tests/mir-opt/inline/inline_trait_method_2.test2.Inline.after.panic-abort.mir
@@ -12,8 +12,8 @@ fn test2(_1: &dyn X) -> bool {
     bb0: {
         StorageLive(_2);
         StorageLive(_3);
-        _3 = &(*_1);
-        _2 = move _3 as &dyn X (PointerCoercion(Unsize));
+        _3 = _1;
+        _2 = move _3;
         StorageDead(_3);
         _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind unreachable];
     }

diff --git a/tests/mir-opt/inline/inline_trait_method_2.test2.Inline.after.panic-unwind.mir b/tests/mir-opt/inline/inline_trait_method_2.test2.Inline.after.panic-unwind.mir
@@ -12,8 +12,8 @@ fn test2(_1: &dyn X) -> bool {
     bb0: {
         StorageLive(_2);
         StorageLive(_3);
-        _3 = &(*_1);
-        _2 = move _3 as &dyn X (PointerCoercion(Unsize));
+        _3 = _1;
+        _2 = move _3;
         StorageDead(_3);
         _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind continue];
     }