From 0d4a3f11e2170c3c35b1faf4bd0ba86c73a56221 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 22 Feb 2022 23:19:57 -0800 Subject: [PATCH] mir-opt: Replace clone on primitives with copy We can't do it for everything, but it would be nice to at least stop making calls to clone methods in debug from things like derived-clones. --- compiler/rustc_middle/src/mir/mod.rs | 30 +++++++ .../rustc_mir_transform/src/instcombine.rs | 85 ++++++++++++++++++- src/test/codegen/inline-hint.rs | 4 +- .../mir-opt/combine_clone_of_primitives.rs | 19 +++++ ...primitives.{impl#0}-clone.InstCombine.diff | 80 +++++++++++++++++ 5 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 src/test/mir-opt/combine_clone_of_primitives.rs create mode 100644 src/test/mir-opt/combine_clone_of_primitives.{impl#0}-clone.InstCombine.diff diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 39d6b1267a5..fc438edc722 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1915,6 +1915,27 @@ impl<'tcx> Place<'tcx> { (base, proj) }) } + + /// Generates a new place by appending `more_projections` to the existing ones + /// and interning the result. + pub fn project_deeper(self, more_projections: &[PlaceElem<'tcx>], tcx: TyCtxt<'tcx>) -> Self { + if more_projections.is_empty() { + return self; + } + + let mut v: Vec>; + + let new_projections = if self.projection.is_empty() { + more_projections + } else { + v = Vec::with_capacity(self.projection.len() + more_projections.len()); + v.extend(self.projection); + v.extend(more_projections); + &v + }; + + Place { local: self.local, projection: tcx.intern_place_elems(new_projections) } + } } impl From for Place<'_> { @@ -2187,6 +2208,15 @@ impl<'tcx> Operand<'tcx> { Operand::Copy(_) | Operand::Move(_) => None, } } + + /// Gets the `ty::FnDef` from an operand if it's a constant function item. + /// + /// While this is unlikely in general, it's the normal case of what you'll + /// find as the `func` in a [`TerminatorKind::Call`]. + pub fn const_fn_def(&self) -> Option<(DefId, SubstsRef<'tcx>)> { + let const_ty = self.constant()?.literal.const_for_ty()?.ty(); + if let ty::FnDef(def_id, substs) = *const_ty.kind() { Some((def_id, substs)) } else { None } + } } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_mir_transform/src/instcombine.rs b/compiler/rustc_mir_transform/src/instcombine.rs index 385fcc43496..30e55d7e2fa 100644 --- a/compiler/rustc_mir_transform/src/instcombine.rs +++ b/compiler/rustc_mir_transform/src/instcombine.rs @@ -4,9 +4,9 @@ use crate::MirPass; use rustc_hir::Mutability; use rustc_middle::mir::{ BinOp, Body, Constant, LocalDecls, Operand, Place, ProjectionElem, Rvalue, SourceInfo, - StatementKind, UnOp, + Statement, StatementKind, Terminator, TerminatorKind, UnOp, }; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self, Ty, TyCtxt, TyKind}; pub struct InstCombine; @@ -29,6 +29,11 @@ impl<'tcx> MirPass<'tcx> for InstCombine { _ => {} } } + + ctx.combine_primitive_clone( + &mut block.terminator.as_mut().unwrap(), + &mut block.statements, + ); } } } @@ -130,4 +135,80 @@ impl<'tcx> InstCombineContext<'tcx, '_> { } } } + + fn combine_primitive_clone( + &self, + terminator: &mut Terminator<'tcx>, + statements: &mut Vec>, + ) { + let TerminatorKind::Call { func, args, destination, .. } = &mut terminator.kind + else { return }; + + // It's definitely not a clone if there are multiple arguments + if args.len() != 1 { + return; + } + + let Some((destination_place, destination_block)) = *destination + else { return }; + + // Only bother looking more if it's easy to know what we're calling + let Some((fn_def_id, fn_substs)) = func.const_fn_def() + else { return }; + + // Clone needs one subst, so we can cheaply rule out other stuff + if fn_substs.len() != 1 { + return; + } + + // These types are easily available from locals, so check that before + // doing DefId lookups to figure out what we're actually calling. + let arg_ty = args[0].ty(self.local_decls, self.tcx); + + let ty::Ref(_region, inner_ty, Mutability::Not) = *arg_ty.kind() + else { return }; + + if !is_trivially_pure_copy(self.tcx, inner_ty) { + return; + } + + let trait_def_id = self.tcx.trait_of_item(fn_def_id); + if trait_def_id.is_none() || trait_def_id != self.tcx.lang_items().clone_trait() { + return; + } + + if !self.tcx.consider_optimizing(|| { + format!( + "InstCombine - Call: {:?} SourceInfo: {:?}", + (fn_def_id, fn_substs), + terminator.source_info + ) + }) { + return; + } + + let Some(arg_place) = args.pop().unwrap().place() + else { return }; + + statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::Assign(box ( + destination_place, + Rvalue::Use(Operand::Copy( + arg_place.project_deeper(&[ProjectionElem::Deref], self.tcx), + )), + )), + }); + terminator.kind = TerminatorKind::Goto { target: destination_block }; + } +} + +fn is_trivially_pure_copy<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { + use TyKind::*; + match *ty.kind() { + Bool | Char | Int(..) | Uint(..) | Float(..) => true, + Array(element_ty, _len) => is_trivially_pure_copy(tcx, element_ty), + Tuple(field_tys) => field_tys.iter().all(|x| is_trivially_pure_copy(tcx, x)), + _ => false, + } } diff --git a/src/test/codegen/inline-hint.rs b/src/test/codegen/inline-hint.rs index ad41badf381..d3ea1915a8b 100644 --- a/src/test/codegen/inline-hint.rs +++ b/src/test/codegen/inline-hint.rs @@ -6,7 +6,7 @@ pub fn f() { let a = A; - let b = (0i32, 1i32, 2i32, 3i32); + let b = (0i32, 1i32, 2i32, 3 as *const i32); let c = || {}; a(String::new(), String::new()); @@ -21,7 +21,7 @@ struct A(String, String); // CHECK-NOT: inlinehint // CHECK-SAME: {{$}} -// CHECK: ; <(i32, i32, i32, i32) as core::clone::Clone>::clone +// CHECK: ; <(i32, i32, i32, *const i{{16|32|64}}) as core::clone::Clone>::clone // CHECK-NEXT: ; Function Attrs: inlinehint // CHECK: ; inline_hint::f::{closure#0} diff --git a/src/test/mir-opt/combine_clone_of_primitives.rs b/src/test/mir-opt/combine_clone_of_primitives.rs new file mode 100644 index 00000000000..894d9281d5d --- /dev/null +++ b/src/test/mir-opt/combine_clone_of_primitives.rs @@ -0,0 +1,19 @@ +// compile-flags: -C opt-level=0 -Z inline_mir=no + +// EMIT_MIR combine_clone_of_primitives.{impl#0}-clone.InstCombine.diff + +#[derive(Clone)] +struct MyThing { + v: T, + i: u64, + a: [f32; 3], +} + +fn main() { + let x = MyThing:: { v: 2, i: 3, a: [0.0; 3] }; + let y = x.clone(); + + assert_eq!(y.v, 2); + assert_eq!(y.i, 3); + assert_eq!(y.a, [0.0; 3]); +} diff --git a/src/test/mir-opt/combine_clone_of_primitives.{impl#0}-clone.InstCombine.diff b/src/test/mir-opt/combine_clone_of_primitives.{impl#0}-clone.InstCombine.diff new file mode 100644 index 00000000000..a19b92d9084 --- /dev/null +++ b/src/test/mir-opt/combine_clone_of_primitives.{impl#0}-clone.InstCombine.diff @@ -0,0 +1,80 @@ +- // MIR for `::clone` before InstCombine ++ // MIR for `::clone` after InstCombine + + fn ::clone(_1: &MyThing) -> MyThing { + debug self => _1; // in scope 0 at $DIR/combine_clone_of_primitives.rs:5:10: 5:15 + let mut _0: MyThing; // return place in scope 0 at $DIR/combine_clone_of_primitives.rs:5:10: 5:15 + let _2: &T; // in scope 0 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + let _3: &u64; // in scope 0 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + let _4: &[f32; 3]; // in scope 0 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 + let mut _5: T; // in scope 0 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + let mut _6: &T; // in scope 0 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + let _7: &T; // in scope 0 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + let mut _8: u64; // in scope 0 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + let mut _9: &u64; // in scope 0 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + let _10: &u64; // in scope 0 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + let mut _11: [f32; 3]; // in scope 0 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 + let mut _12: &[f32; 3]; // in scope 0 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 + let _13: &[f32; 3]; // in scope 0 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 + scope 1 { + debug __self_0_0 => _2; // in scope 1 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + debug __self_0_1 => _3; // in scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + debug __self_0_2 => _4; // in scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 + } + + bb0: { + _2 = &((*_1).0: T); // scope 0 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + _3 = &((*_1).1: u64); // scope 0 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + _4 = &((*_1).2: [f32; 3]); // scope 0 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 +- _7 = &(*_2); // scope 1 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 +- _6 = &(*_7); // scope 1 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 ++ _7 = _2; // scope 1 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 ++ _6 = _7; // scope 1 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + _5 = ::clone(move _6) -> bb1; // scope 1 at $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + // mir::Constant + // + span: $DIR/combine_clone_of_primitives.rs:7:5: 7:9 + // + literal: Const { ty: for<'r> fn(&'r T) -> T {::clone}, val: Value(Scalar()) } + } + + bb1: { +- _10 = &(*_3); // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 +- _9 = &(*_10); // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 +- _8 = ::clone(move _9) -> [return: bb2, unwind: bb4]; // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 +- // mir::Constant +- // + span: $DIR/combine_clone_of_primitives.rs:8:5: 8:11 +- // + literal: Const { ty: for<'r> fn(&'r u64) -> u64 {::clone}, val: Value(Scalar()) } ++ _10 = _3; // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 ++ _9 = _10; // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 ++ _8 = (*_9); // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 ++ goto -> bb2; // scope 1 at $DIR/combine_clone_of_primitives.rs:8:5: 8:11 + } + + bb2: { +- _13 = &(*_4); // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 +- _12 = &(*_13); // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 +- _11 = <[f32; 3] as Clone>::clone(move _12) -> [return: bb3, unwind: bb4]; // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 +- // mir::Constant +- // + span: $DIR/combine_clone_of_primitives.rs:9:5: 9:16 +- // + literal: Const { ty: for<'r> fn(&'r [f32; 3]) -> [f32; 3] {<[f32; 3] as Clone>::clone}, val: Value(Scalar()) } ++ _13 = _4; // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 ++ _12 = _13; // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 ++ _11 = (*_12); // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 ++ goto -> bb3; // scope 1 at $DIR/combine_clone_of_primitives.rs:9:5: 9:16 + } + + bb3: { + (_0.0: T) = move _5; // scope 1 at $DIR/combine_clone_of_primitives.rs:5:10: 5:15 + (_0.1: u64) = move _8; // scope 1 at $DIR/combine_clone_of_primitives.rs:5:10: 5:15 + (_0.2: [f32; 3]) = move _11; // scope 1 at $DIR/combine_clone_of_primitives.rs:5:10: 5:15 + return; // scope 0 at $DIR/combine_clone_of_primitives.rs:5:15: 5:15 + } + + bb4 (cleanup): { + drop(_5) -> bb5; // scope 1 at $DIR/combine_clone_of_primitives.rs:5:14: 5:15 + } + + bb5 (cleanup): { + resume; // scope 0 at $DIR/combine_clone_of_primitives.rs:5:10: 5:15 + } + } +