diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index fab2f2c97e4ae..cd2bea86ea1a7 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -54,7 +54,7 @@ mod type_foldable; pub mod visit; /// Types for locals -type LocalDecls<'tcx> = IndexVec>; +pub type LocalDecls<'tcx> = IndexVec>; pub trait HasLocalDecls<'tcx> { fn local_decls(&self) -> &LocalDecls<'tcx>; diff --git a/compiler/rustc_mir/src/transform/instcombine.rs b/compiler/rustc_mir/src/transform/instcombine.rs index b0c70372b161a..74dadb2572565 100644 --- a/compiler/rustc_mir/src/transform/instcombine.rs +++ b/compiler/rustc_mir/src/transform/instcombine.rs @@ -1,329 +1,122 @@ //! Performs various peephole optimizations. use crate::transform::MirPass; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::Mutability; -use rustc_index::vec::Idx; use rustc_middle::mir::{ - visit::PlaceContext, - visit::{MutVisitor, Visitor}, - Statement, -}; -use rustc_middle::mir::{ - BinOp, Body, BorrowKind, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem, - Rvalue, + BinOp, Body, Constant, LocalDecls, Operand, Place, ProjectionElem, Rvalue, SourceInfo, + StatementKind, }; use rustc_middle::ty::{self, TyCtxt}; -use std::mem; pub struct InstCombine; impl<'tcx> MirPass<'tcx> for InstCombine { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - // First, find optimization opportunities. This is done in a pre-pass to keep the MIR - // read-only so that we can do global analyses on the MIR in the process (e.g. - // `Place::ty()`). - let optimizations = { - let mut optimization_finder = OptimizationFinder::new(body, tcx); - optimization_finder.visit_body(body); - optimization_finder.optimizations - }; - - if !optimizations.is_empty() { - // Then carry out those optimizations. - MutVisitor::visit_body(&mut InstCombineVisitor { optimizations, tcx }, body); + let (basic_blocks, local_decls) = body.basic_blocks_and_local_decls_mut(); + let ctx = InstCombineContext { tcx, local_decls }; + for block in basic_blocks.iter_mut() { + for statement in block.statements.iter_mut() { + match statement.kind { + StatementKind::Assign(box (_place, ref mut rvalue)) => { + ctx.combine_bool_cmp(&statement.source_info, rvalue); + ctx.combine_ref_deref(&statement.source_info, rvalue); + ctx.combine_len(&statement.source_info, rvalue); + } + _ => {} + } + } } } } -pub struct InstCombineVisitor<'tcx> { - optimizations: OptimizationList<'tcx>, +struct InstCombineContext<'tcx, 'a> { tcx: TyCtxt<'tcx>, + local_decls: &'a LocalDecls<'tcx>, } -impl<'tcx> InstCombineVisitor<'tcx> { - fn should_combine(&self, rvalue: &Rvalue<'tcx>, location: Location) -> bool { +impl<'tcx, 'a> InstCombineContext<'tcx, 'a> { + fn should_combine(&self, source_info: &SourceInfo, rvalue: &Rvalue<'tcx>) -> bool { self.tcx.consider_optimizing(|| { - format!("InstCombine - Rvalue: {:?} Location: {:?}", rvalue, location) + format!("InstCombine - Rvalue: {:?} SourceInfo: {:?}", rvalue, source_info) }) } -} - -impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> { - fn tcx(&self) -> TyCtxt<'tcx> { - self.tcx - } - fn visit_rvalue(&mut self, rvalue: &mut Rvalue<'tcx>, location: Location) { - if self.optimizations.and_stars.remove(&location) && self.should_combine(rvalue, location) { - debug!("replacing `&*`: {:?}", rvalue); - let new_place = match rvalue { - Rvalue::Ref(_, _, place) => { - if let &[ref proj_l @ .., proj_r] = place.projection.as_ref() { - place.projection = self.tcx().intern_place_elems(&[proj_r]); - - Place { - // Replace with dummy - local: mem::replace(&mut place.local, Local::new(0)), - projection: self.tcx().intern_place_elems(proj_l), - } - } else { - unreachable!(); + /// Transform boolean comparisons into logical operations. + fn combine_bool_cmp(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { + match rvalue { + Rvalue::BinaryOp(op @ (BinOp::Eq | BinOp::Ne), a, b) => { + let new = match (op, self.try_eval_bool(a), self.try_eval_bool(b)) { + // Transform "Eq(a, true)" ==> "a" + (BinOp::Eq, _, Some(true)) => Some(a.clone()), + + // Transform "Ne(a, false)" ==> "a" + (BinOp::Ne, _, Some(false)) => Some(a.clone()), + + // Transform "Eq(true, b)" ==> "b" + (BinOp::Eq, Some(true), _) => Some(b.clone()), + + // Transform "Ne(false, b)" ==> "b" + (BinOp::Ne, Some(false), _) => Some(b.clone()), + + // FIXME: Consider combining remaining comparisons into logical operations: + // Transform "Eq(false, b)" ==> "Not(b)" + // Transform "Ne(true, b)" ==> "Not(b)" + // Transform "Eq(a, false)" ==> "Not(a)" + // Transform "Ne(a, true)" ==> "Not(a)" + _ => None, + }; + + if let Some(new) = new { + if self.should_combine(source_info, rvalue) { + *rvalue = Rvalue::Use(new); } } - _ => bug!("Detected `&*` but didn't find `&*`!"), - }; - *rvalue = Rvalue::Use(Operand::Copy(new_place)) - } - - if let Some(constant) = self.optimizations.arrays_lengths.remove(&location) { - if self.should_combine(rvalue, location) { - debug!("replacing `Len([_; N])`: {:?}", rvalue); - *rvalue = Rvalue::Use(Operand::Constant(box constant)); - } - } - - if let Some(operand) = self.optimizations.unneeded_equality_comparison.remove(&location) { - if self.should_combine(rvalue, location) { - debug!("replacing {:?} with {:?}", rvalue, operand); - *rvalue = Rvalue::Use(operand); - } - } - - if let Some(place) = self.optimizations.unneeded_deref.remove(&location) { - if self.should_combine(rvalue, location) { - debug!("unneeded_deref: replacing {:?} with {:?}", rvalue, place); - *rvalue = Rvalue::Use(Operand::Copy(place)); } - } - - // We do not call super_rvalue as we are not interested in any other parts of the tree - } -} - -struct MutatingUseVisitor { - has_mutating_use: bool, - local_to_look_for: Local, -} - -impl MutatingUseVisitor { - fn has_mutating_use_in_stmt(local: Local, stmt: &Statement<'tcx>, location: Location) -> bool { - let mut _self = Self { has_mutating_use: false, local_to_look_for: local }; - _self.visit_statement(stmt, location); - _self.has_mutating_use - } -} -impl<'tcx> Visitor<'tcx> for MutatingUseVisitor { - fn visit_local(&mut self, local: &Local, context: PlaceContext, _: Location) { - if *local == self.local_to_look_for { - self.has_mutating_use |= context.is_mutating_use(); + _ => {} } } -} - -/// Finds optimization opportunities on the MIR. -struct OptimizationFinder<'b, 'tcx> { - body: &'b Body<'tcx>, - tcx: TyCtxt<'tcx>, - optimizations: OptimizationList<'tcx>, -} -impl OptimizationFinder<'b, 'tcx> { - fn new(body: &'b Body<'tcx>, tcx: TyCtxt<'tcx>) -> OptimizationFinder<'b, 'tcx> { - OptimizationFinder { body, tcx, optimizations: OptimizationList::default() } + fn try_eval_bool(&self, a: &Operand<'_>) -> Option { + let a = a.constant()?; + if a.literal.ty.is_bool() { a.literal.val.try_to_bool() } else { None } } - fn find_deref_of_address(&mut self, rvalue: &Rvalue<'tcx>, location: Location) -> Option<()> { - // FIXME(#78192): This optimization can result in unsoundness. - if !self.tcx.sess.opts.debugging_opts.unsound_mir_opts { - return None; - } - - // Look for the sequence - // - // _2 = &_1; - // ... - // _5 = (*_2); - // - // which we can replace the last statement with `_5 = _1;` to avoid the load of `_2`. - if let Rvalue::Use(op) = rvalue { - let local_being_derefed = match op.place()?.as_ref() { - PlaceRef { local, projection: [ProjectionElem::Deref] } => Some(local), - _ => None, - }?; - - let mut dead_locals_seen = vec![]; - - let stmt_index = location.statement_index; - // Look behind for statement that assigns the local from a address of operator. - // 6 is chosen as a heuristic determined by seeing the number of times - // the optimization kicked in compiling rust std. - let lower_index = stmt_index.saturating_sub(6); - let statements_to_look_in = self.body.basic_blocks()[location.block].statements - [lower_index..stmt_index] - .iter() - .rev(); - for stmt in statements_to_look_in { - match &stmt.kind { - // Exhaustive match on statements to detect conditions that warrant we bail out of the optimization. - rustc_middle::mir::StatementKind::Assign(box (l, r)) - if l.local == local_being_derefed => - { - match r { - // Looking for immutable reference e.g _local_being_deref = &_1; - Rvalue::Ref( - _, - // Only apply the optimization if it is an immutable borrow. - BorrowKind::Shared, - place_taken_address_of, - ) => { - // Make sure that the place has not been marked dead - if dead_locals_seen.contains(&place_taken_address_of.local) { - return None; - } - - self.optimizations - .unneeded_deref - .insert(location, *place_taken_address_of); - return Some(()); - } - - // We found an assignment of `local_being_deref` that is not an immutable ref, e.g the following sequence - // _2 = &_1; - // _3 = &5 - // _2 = _3; <-- this means it is no longer valid to replace the last statement with `_5 = _1;` - // _5 = (*_2); - _ => return None, - } - } - - // Inline asm can do anything, so bail out of the optimization. - rustc_middle::mir::StatementKind::LlvmInlineAsm(_) => return None, - - // Remember `StorageDead`s, as the local being marked dead could be the - // place RHS we are looking for, in which case we need to abort to avoid UB - // using an uninitialized place - rustc_middle::mir::StatementKind::StorageDead(dead) => { - dead_locals_seen.push(*dead) - } + /// Transform "&(*a)" ==> "a". + fn combine_ref_deref(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { + if let Rvalue::Ref(_, _, place) = rvalue { + if let Some((base, ProjectionElem::Deref)) = place.as_ref().last_projection() { + if let ty::Ref(_, _, Mutability::Not) = + base.ty(self.local_decls, self.tcx).ty.kind() + { + // The dereferenced place must have type `&_`, so that we don't copy `&mut _`. + } else { + return; + } - // Check that `local_being_deref` is not being used in a mutating way which can cause misoptimization. - rustc_middle::mir::StatementKind::Assign(box (_, _)) - | rustc_middle::mir::StatementKind::Coverage(_) - | rustc_middle::mir::StatementKind::Nop - | rustc_middle::mir::StatementKind::FakeRead(_, _) - | rustc_middle::mir::StatementKind::StorageLive(_) - | rustc_middle::mir::StatementKind::Retag(_, _) - | rustc_middle::mir::StatementKind::AscribeUserType(_, _) - | rustc_middle::mir::StatementKind::SetDiscriminant { .. } => { - if MutatingUseVisitor::has_mutating_use_in_stmt( - local_being_derefed, - stmt, - location, - ) { - return None; - } - } + if !self.should_combine(source_info, rvalue) { + return; } - } - } - Some(()) - } - fn find_unneeded_equality_comparison(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { - // find Ne(_place, false) or Ne(false, _place) - // or Eq(_place, true) or Eq(true, _place) - if let Rvalue::BinaryOp(op, l, r) = rvalue { - let const_to_find = if *op == BinOp::Ne { - false - } else if *op == BinOp::Eq { - true - } else { - return; - }; - // (const, _place) - if let Some(o) = self.find_operand_in_equality_comparison_pattern(l, r, const_to_find) { - self.optimizations.unneeded_equality_comparison.insert(location, o.clone()); - } - // (_place, const) - else if let Some(o) = - self.find_operand_in_equality_comparison_pattern(r, l, const_to_find) - { - self.optimizations.unneeded_equality_comparison.insert(location, o.clone()); + *rvalue = Rvalue::Use(Operand::Copy(Place { + local: base.local, + projection: self.tcx.intern_place_elems(base.projection), + })); } } } - fn find_operand_in_equality_comparison_pattern( - &self, - l: &Operand<'tcx>, - r: &'a Operand<'tcx>, - const_to_find: bool, - ) -> Option<&'a Operand<'tcx>> { - let const_ = l.constant()?; - if const_.literal.ty == self.tcx.types.bool - && const_.literal.val.try_to_bool() == Some(const_to_find) - { - if r.place().is_some() { - return Some(r); - } - } - - None - } -} - -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 Some((place_base, ProjectionElem::Deref)) = place.as_ref().last_projection() { - // The dereferenced place must have type `&_`. - let ty = place_base.ty(self.body, self.tcx).ty; - if let ty::Ref(_, _, Mutability::Not) = ty.kind() { - self.optimizations.and_stars.insert(location); - } - } - } - + /// Transform "Len([_; N])" ==> "N". + fn combine_len(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { if let Rvalue::Len(ref place) = *rvalue { - let place_ty = place.ty(&self.body.local_decls, self.tcx).ty; + let place_ty = place.ty(self.local_decls, self.tcx).ty; if let ty::Array(_, len) = place_ty.kind() { - let span = self.body.source_info(location).span; - let constant = Constant { span, literal: len, user_ty: None }; - self.optimizations.arrays_lengths.insert(location, constant); - } - } - - let _ = self.find_deref_of_address(rvalue, location); - - self.find_unneeded_equality_comparison(rvalue, location); - - // We do not call super_rvalue as we are not interested in any other parts of the tree - } -} - -#[derive(Default)] -struct OptimizationList<'tcx> { - and_stars: FxHashSet, - arrays_lengths: FxHashMap>, - unneeded_equality_comparison: FxHashMap>, - unneeded_deref: FxHashMap>, -} + if !self.should_combine(source_info, rvalue) { + return; + } -impl<'tcx> OptimizationList<'tcx> { - fn is_empty(&self) -> bool { - match self { - OptimizationList { - and_stars, - arrays_lengths, - unneeded_equality_comparison, - unneeded_deref, - } => { - and_stars.is_empty() - && arrays_lengths.is_empty() - && unneeded_equality_comparison.is_empty() - && unneeded_deref.is_empty() + let constant = Constant { span: source_info.span, literal: len, user_ty: None }; + *rvalue = Rvalue::Use(Operand::Constant(box constant)); } } } diff --git a/src/test/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir b/src/test/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir index db504b416fe1d..4bda9ae383c22 100644 --- a/src/test/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir +++ b/src/test/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir @@ -40,7 +40,7 @@ fn foo(_1: T, _2: &i32) -> i32 { _9 = move (_5.1: &i32); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 StorageLive(_10); // scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 _10 = _8; // scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 - _0 = (*_8); // scope 3 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 + _0 = (*_10); // scope 3 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 StorageDead(_10); // scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 StorageDead(_9); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 StorageDead(_8); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12 diff --git a/src/test/mir-opt/inst_combine_deref.rs b/src/test/mir-opt/inst_combine_deref.rs deleted file mode 100644 index 78361c336607c..0000000000000 --- a/src/test/mir-opt/inst_combine_deref.rs +++ /dev/null @@ -1,69 +0,0 @@ -// compile-flags: -O -Zunsound-mir-opts -// EMIT_MIR inst_combine_deref.simple_opt.InstCombine.diff -fn simple_opt() -> u64 { - let x = 5; - let y = &x; - let z = *y; - z -} - -// EMIT_MIR inst_combine_deref.deep_opt.InstCombine.diff -fn deep_opt() -> (u64, u64, u64) { - let x1 = 1; - let x2 = 2; - let x3 = 3; - let y1 = &x1; - let y2 = &x2; - let y3 = &x3; - let z1 = *y1; - let z2 = *y2; - let z3 = *y3; - (z1, z2, z3) -} - -struct S { - a: u64, - b: u64, -} - -// EMIT_MIR inst_combine_deref.opt_struct.InstCombine.diff -fn opt_struct(s: S) -> u64 { - let a = &s.a; - let b = &s.b; - let x = *a; - *b + x -} - -// EMIT_MIR inst_combine_deref.dont_opt.InstCombine.diff -// do not optimize a sequence looking like this: -// _1 = &_2; -// _1 = _3; -// _4 = *_1; -// as the _1 = _3 assignment makes it not legal to replace the last statement with _4 = _2 -fn dont_opt() -> u64 { - let y = 5; - let _ref = &y; - let x = 5; - let mut _1 = &x; - _1 = _ref; - let _4 = *_1; - 0 -} - -// EMIT_MIR inst_combine_deref.do_not_miscompile.InstCombine.diff -fn do_not_miscompile() { - let x = 42; - let a = 99; - let mut y = &x; - let z = &mut y; - *z = &a; - assert!(*y == 99); -} - -fn main() { - simple_opt(); - deep_opt(); - opt_struct(S { a: 0, b: 1 }); - dont_opt(); - do_not_miscompile(); -}