From 12a77b7c5f9408674125ca6e70b77b967a2f85df Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Wed, 16 Sep 2020 20:27:23 +0200 Subject: [PATCH 1/2] Add test demonstrating missing optimization _3 = &mut (*_1); can be _3 = _1 if _1 is of type &mut (which it is) --- .../and_star.opt_immutable.InstCombine.diff | 44 +++++++++++++++++++ .../and_star.opt_mutable.InstCombine.diff | 43 ++++++++++++++++++ src/test/mir-opt/and_star.rs | 14 ++++++ 3 files changed, 101 insertions(+) create mode 100644 src/test/mir-opt/and_star.opt_immutable.InstCombine.diff create mode 100644 src/test/mir-opt/and_star.opt_mutable.InstCombine.diff create mode 100644 src/test/mir-opt/and_star.rs diff --git a/src/test/mir-opt/and_star.opt_immutable.InstCombine.diff b/src/test/mir-opt/and_star.opt_immutable.InstCombine.diff new file mode 100644 index 0000000000000..355f29fe5ea09 --- /dev/null +++ b/src/test/mir-opt/and_star.opt_immutable.InstCombine.diff @@ -0,0 +1,44 @@ +- // MIR for `opt_immutable` before InstCombine ++ // MIR for `opt_immutable` after InstCombine + + fn opt_immutable(_1: &[u8]) -> () { + debug dst => _1; // in scope 0 at $DIR/and_star.rs:7:18: 7:21 + let mut _0: (); // return place in scope 0 at $DIR/and_star.rs:7:30: 7:30 + let _2: std::option::Option<&u8>; // in scope 0 at $DIR/and_star.rs:8:5: 8:15 + let mut _3: &[u8]; // in scope 0 at $DIR/and_star.rs:8:5: 8:8 + let mut _6: usize; // in scope 0 at $DIR/and_star.rs:8:5: 8:15 + scope 1 { + debug self => _3; // in scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + debug index => _6; // in scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + let mut _4: usize; // in scope 1 at $DIR/and_star.rs:8:5: 8:15 + let mut _5: &[u8]; // in scope 1 at $DIR/and_star.rs:8:5: 8:15 + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/and_star.rs:8:5: 8:15 + StorageLive(_3); // scope 0 at $DIR/and_star.rs:8:5: 8:8 +- _3 = &(*_1); // scope 0 at $DIR/and_star.rs:8:5: 8:8 ++ _3 = _1; // scope 0 at $DIR/and_star.rs:8:5: 8:8 + StorageLive(_6); // scope 0 at $DIR/and_star.rs:8:5: 8:15 + _6 = const 0_usize; // scope 0 at $DIR/and_star.rs:8:5: 8:15 + StorageLive(_4); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _4 = move _6; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + StorageLive(_5); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _5 = _3; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _2 = >::get(move _4, move _5) -> bb1; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + // mir::Constant + // + span: $SRC_DIR/core/src/slice/mod.rs:LL:COL + // + literal: Const { ty: for<'r> fn(usize, &'r [u8]) -> std::option::Option<&'r >::Output> {>::get}, val: Value(Scalar()) } + } + + bb1: { + StorageDead(_5); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + StorageDead(_4); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + StorageDead(_6); // scope 0 at $DIR/and_star.rs:8:5: 8:15 + StorageDead(_3); // scope 0 at $DIR/and_star.rs:8:14: 8:15 + StorageDead(_2); // scope 0 at $DIR/and_star.rs:8:15: 8:16 + _0 = const (); // scope 0 at $DIR/and_star.rs:7:30: 9:2 + return; // scope 0 at $DIR/and_star.rs:9:2: 9:2 + } + } + diff --git a/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff b/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff new file mode 100644 index 0000000000000..2f9c55613f3b4 --- /dev/null +++ b/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff @@ -0,0 +1,43 @@ +- // MIR for `opt_mutable` before InstCombine ++ // MIR for `opt_mutable` after InstCombine + + fn opt_mutable(_1: &mut [u8]) -> () { + debug dst => _1; // in scope 0 at $DIR/and_star.rs:2:16: 2:19 + let mut _0: (); // return place in scope 0 at $DIR/and_star.rs:2:32: 2:32 + let _2: std::option::Option<&mut u8>; // in scope 0 at $DIR/and_star.rs:3:5: 3:19 + let mut _3: &mut [u8]; // in scope 0 at $DIR/and_star.rs:3:5: 3:8 + let mut _6: usize; // in scope 0 at $DIR/and_star.rs:3:5: 3:19 + scope 1 { + debug self => _3; // in scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + debug index => _6; // in scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + let mut _4: usize; // in scope 1 at $DIR/and_star.rs:3:5: 3:19 + let mut _5: &mut [u8]; // in scope 1 at $DIR/and_star.rs:3:5: 3:19 + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/and_star.rs:3:5: 3:19 + StorageLive(_3); // scope 0 at $DIR/and_star.rs:3:5: 3:8 + _3 = &mut (*_1); // scope 0 at $DIR/and_star.rs:3:5: 3:8 + StorageLive(_6); // scope 0 at $DIR/and_star.rs:3:5: 3:19 + _6 = const 0_usize; // scope 0 at $DIR/and_star.rs:3:5: 3:19 + StorageLive(_4); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _4 = move _6; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + StorageLive(_5); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _5 = &mut (*_3); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _2 = >::get_mut(move _4, move _5) -> bb1; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + // mir::Constant + // + span: $SRC_DIR/core/src/slice/mod.rs:LL:COL + // + literal: Const { ty: for<'r> fn(usize, &'r mut [u8]) -> std::option::Option<&'r mut >::Output> {>::get_mut}, val: Value(Scalar()) } + } + + bb1: { + StorageDead(_5); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + StorageDead(_4); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + StorageDead(_6); // scope 0 at $DIR/and_star.rs:3:5: 3:19 + StorageDead(_3); // scope 0 at $DIR/and_star.rs:3:18: 3:19 + StorageDead(_2); // scope 0 at $DIR/and_star.rs:3:19: 3:20 + _0 = const (); // scope 0 at $DIR/and_star.rs:2:32: 4:2 + return; // scope 0 at $DIR/and_star.rs:4:2: 4:2 + } + } + diff --git a/src/test/mir-opt/and_star.rs b/src/test/mir-opt/and_star.rs new file mode 100644 index 0000000000000..1ef5878497ea0 --- /dev/null +++ b/src/test/mir-opt/and_star.rs @@ -0,0 +1,14 @@ +// EMIT_MIR and_star.opt_mutable.InstCombine.diff +fn opt_mutable(dst: &mut [u8]) { + dst.get_mut(0); +} + +// EMIT_MIR and_star.opt_immutable.InstCombine.diff +fn opt_immutable(dst: &[u8]) { + dst.get(0); +} + +fn main() { + opt_mutable(&mut [1]); + opt_immutable(&[1]) +} From 5b354edd5bb7a1fc5c28e1a4a233f0bbf9cd59d5 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Wed, 16 Sep 2020 21:05:06 +0200 Subject: [PATCH 2/2] Generalize &(*_1) optimization to mutable references --- compiler/rustc_middle/src/mir/mod.rs | 8 +++++++ .../rustc_mir/src/transform/instcombine.rs | 23 +++++++++++-------- .../and_star.opt_mutable.InstCombine.diff | 5 ++-- ...float_to_exponential_common.ConstProp.diff | 4 ++-- ...67_inline_as_ref_as_mut.a.Inline.after.mir | 6 +---- ...67_inline_as_ref_as_mut.b.Inline.after.mir | 8 ++----- .../nrvo_simple.nrvo.RenameReturnPlace.diff | 7 +----- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 29daf7e9309aa..e5dc106d4b646 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -649,6 +649,14 @@ impl BorrowKind { BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow, } } + + /// Returns the mutability of the borrow + pub fn mutability(&self) -> Mutability { + match self { + BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => Mutability::Not, + BorrowKind::Mut { .. } => Mutability::Mut, + } + } } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_mir/src/transform/instcombine.rs b/compiler/rustc_mir/src/transform/instcombine.rs index c4924cf16ab64..ee55a9142500b 100644 --- a/compiler/rustc_mir/src/transform/instcombine.rs +++ b/compiler/rustc_mir/src/transform/instcombine.rs @@ -1,8 +1,8 @@ //! Performs various peephole optimizations. use crate::transform::{MirPass, MirSource}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_hir::Mutability; +use rustc_ast::Mutability; +use rustc_data_structures::fx::FxHashMap; use rustc_index::vec::Idx; use rustc_middle::mir::visit::{MutVisitor, Visitor}; use rustc_middle::mir::{ @@ -40,8 +40,9 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> { } fn visit_rvalue(&mut self, rvalue: &mut Rvalue<'tcx>, location: Location) { - if self.optimizations.and_stars.remove(&location) { - debug!("replacing `&*`: {:?}", rvalue); + if let Some(mutability) = self.optimizations.and_stars.remove(&location) { + let type_to_print = if mutability == Mutability::Not { "&" } else { "&mut" }; + debug!("replacing `{}*`: {:?}", type_to_print, rvalue); let new_place = match rvalue { Rvalue::Ref(_, _, place) => { if let &[ref proj_l @ .., proj_r] = place.projection.as_ref() { @@ -56,7 +57,7 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> { unreachable!(); } } - _ => bug!("Detected `&*` but didn't find `&*`!"), + _ => bug!("Detected `{}*` but didn't find `{}*`!", type_to_print, type_to_print), }; *rvalue = Rvalue::Use(Operand::Copy(new_place)) } @@ -132,14 +133,16 @@ impl OptimizationFinder<'b, 'tcx> { impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> { fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { - if let Rvalue::Ref(_, _, place) = rvalue { + if let Rvalue::Ref(_, borrowkind, place) = rvalue { if let PlaceRef { local, projection: &[ref proj_base @ .., ProjectionElem::Deref] } = place.as_ref() { - // The dereferenced place must have type `&_`. + // The dereferenced place must have a reference type with the same mutability let ty = Place::ty_from(local, proj_base, self.body, self.tcx).ty; - if let ty::Ref(_, _, Mutability::Not) = ty.kind() { - self.optimizations.and_stars.insert(location); + let mutability = borrowkind.mutability(); + if matches!(ty.kind(), ty::Ref(_, _, derefed_place_mutability) if *derefed_place_mutability == mutability) + { + self.optimizations.and_stars.insert(location, mutability); } } } @@ -161,7 +164,7 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> { #[derive(Default)] struct OptimizationList<'tcx> { - and_stars: FxHashSet, + and_stars: FxHashMap, arrays_lengths: FxHashMap>, unneeded_equality_comparison: FxHashMap>, } diff --git a/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff b/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff index 2f9c55613f3b4..30c28fbe43648 100644 --- a/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff +++ b/src/test/mir-opt/and_star.opt_mutable.InstCombine.diff @@ -17,13 +17,14 @@ bb0: { StorageLive(_2); // scope 0 at $DIR/and_star.rs:3:5: 3:19 StorageLive(_3); // scope 0 at $DIR/and_star.rs:3:5: 3:8 - _3 = &mut (*_1); // scope 0 at $DIR/and_star.rs:3:5: 3:8 +- _3 = &mut (*_1); // scope 0 at $DIR/and_star.rs:3:5: 3:8 ++ _3 = _1; // scope 0 at $DIR/and_star.rs:3:5: 3:8 StorageLive(_6); // scope 0 at $DIR/and_star.rs:3:5: 3:19 _6 = const 0_usize; // scope 0 at $DIR/and_star.rs:3:5: 3:19 StorageLive(_4); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL _4 = move _6; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL StorageLive(_5); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL - _5 = &mut (*_3); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL + _5 = _3; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL _2 = >::get_mut(move _4, move _5) -> bb1; // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL // mir::Constant // + span: $SRC_DIR/core/src/slice/mod.rs:LL:COL diff --git a/src/test/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff b/src/test/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff index bb79cd80e51b6..9594c989eca12 100644 --- a/src/test/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff +++ b/src/test/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff @@ -78,7 +78,7 @@ bb6: { StorageLive(_18); // scope 2 at $DIR/funky_arms.rs:28:46: 28:49 - _18 = &mut (*_1); // scope 2 at $DIR/funky_arms.rs:28:46: 28:49 + _18 = _1; // scope 2 at $DIR/funky_arms.rs:28:46: 28:49 StorageLive(_19); // scope 2 at $DIR/funky_arms.rs:28:51: 28:54 _19 = _2; // scope 2 at $DIR/funky_arms.rs:28:51: 28:54 StorageLive(_20); // scope 2 at $DIR/funky_arms.rs:28:56: 28:60 @@ -95,7 +95,7 @@ StorageLive(_10); // scope 2 at $DIR/funky_arms.rs:24:17: 24:26 _10 = ((_7 as Some).0: usize); // scope 2 at $DIR/funky_arms.rs:24:17: 24:26 StorageLive(_11); // scope 3 at $DIR/funky_arms.rs:26:43: 26:46 - _11 = &mut (*_1); // scope 3 at $DIR/funky_arms.rs:26:43: 26:46 + _11 = _1; // scope 3 at $DIR/funky_arms.rs:26:43: 26:46 StorageLive(_12); // scope 3 at $DIR/funky_arms.rs:26:48: 26:51 _12 = _2; // scope 3 at $DIR/funky_arms.rs:26:48: 26:51 StorageLive(_13); // scope 3 at $DIR/funky_arms.rs:26:53: 26:57 diff --git a/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.a.Inline.after.mir b/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.a.Inline.after.mir index 501e3e9cf968b..1b09b8baa0ad8 100644 --- a/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.a.Inline.after.mir +++ b/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.a.Inline.after.mir @@ -8,7 +8,6 @@ fn a(_1: &mut [T]) -> &mut [T] { let mut _4: &mut [T]; // in scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:6 scope 1 { debug self => _4; // in scope 1 at $SRC_DIR/core/src/convert/mod.rs:LL:COL - let mut _5: &mut [T]; // in scope 1 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15 } bb0: { @@ -16,10 +15,7 @@ fn a(_1: &mut [T]) -> &mut [T] { StorageLive(_3); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15 StorageLive(_4); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:6 _4 = &mut (*_1); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:6 - StorageLive(_5); // scope 1 at $SRC_DIR/core/src/convert/mod.rs:LL:COL - _5 = &mut (*_4); // scope 1 at $SRC_DIR/core/src/convert/mod.rs:LL:COL - _3 = &mut (*_5); // scope 1 at $SRC_DIR/core/src/convert/mod.rs:LL:COL - StorageDead(_5); // scope 1 at $SRC_DIR/core/src/convert/mod.rs:LL:COL + _3 = _4; // scope 1 at $SRC_DIR/core/src/convert/mod.rs:LL:COL _2 = &mut (*_3); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15 StorageDead(_4); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:14: 3:15 _0 = &mut (*_2); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15 diff --git a/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.b.Inline.after.mir b/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.b.Inline.after.mir index c9a6aed3d4ab3..d2aa522cf1c32 100644 --- a/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.b.Inline.after.mir +++ b/src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.b.Inline.after.mir @@ -9,7 +9,6 @@ fn b(_1: &mut Box) -> &mut T { scope 1 { debug self => _4; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL let mut _5: &mut T; // in scope 1 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:15 - let mut _6: &mut T; // in scope 1 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:15 } bb0: { @@ -18,11 +17,8 @@ fn b(_1: &mut Box) -> &mut T { StorageLive(_4); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:6 _4 = &mut (*_1); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:6 StorageLive(_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL - StorageLive(_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL - _6 = &mut (*(*_4)); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL - _5 = &mut (*_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL - _3 = &mut (*_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL - StorageDead(_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL + _5 = &mut (*(*_4)); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL + _3 = _5; // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL StorageDead(_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL _2 = &mut (*_3); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:15 StorageDead(_4); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:14: 8:15 diff --git a/src/test/mir-opt/nrvo_simple.nrvo.RenameReturnPlace.diff b/src/test/mir-opt/nrvo_simple.nrvo.RenameReturnPlace.diff index 924e87ea8c0ad..18cc4e6e816c7 100644 --- a/src/test/mir-opt/nrvo_simple.nrvo.RenameReturnPlace.diff +++ b/src/test/mir-opt/nrvo_simple.nrvo.RenameReturnPlace.diff @@ -20,17 +20,12 @@ - _2 = [const 0_u8; 1024]; // scope 0 at $DIR/nrvo-simple.rs:3:19: 3:28 + _0 = [const 0_u8; 1024]; // scope 0 at $DIR/nrvo-simple.rs:3:19: 3:28 StorageLive(_3); // scope 1 at $DIR/nrvo-simple.rs:4:5: 4:19 - StorageLive(_5); // scope 1 at $DIR/nrvo-simple.rs:4:10: 4:18 - StorageLive(_6); // scope 1 at $DIR/nrvo-simple.rs:4:10: 4:18 - _6 = &mut _2; // scope 1 at $DIR/nrvo-simple.rs:4:10: 4:18 + _6 = &mut _0; // scope 1 at $DIR/nrvo-simple.rs:4:10: 4:18 - _5 = &mut (*_6); // scope 1 at $DIR/nrvo-simple.rs:4:10: 4:18 - _3 = move _1(move _5) -> bb1; // scope 1 at $DIR/nrvo-simple.rs:4:5: 4:19 + _3 = move _1(move _6) -> bb1; // scope 1 at $DIR/nrvo-simple.rs:4:5: 4:19 } bb1: { - StorageDead(_5); // scope 1 at $DIR/nrvo-simple.rs:4:18: 4:19 - StorageDead(_6); // scope 1 at $DIR/nrvo-simple.rs:4:19: 4:20 StorageDead(_3); // scope 1 at $DIR/nrvo-simple.rs:4:19: 4:20 - _0 = _2; // scope 1 at $DIR/nrvo-simple.rs:5:5: 5:8 - StorageDead(_2); // scope 0 at $DIR/nrvo-simple.rs:6:1: 6:2