Skip to content

MIR peephole optimize {Ne, Eq}(_1, false) into _1 #76067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,15 @@ impl<'tcx> Operand<'tcx> {
Operand::Constant(_) => None,
}
}

/// Returns the `Constant` that is the target of this `Operand`, or `None` if this `Operand` is a
/// place.
pub fn constant(&self) -> Option<&Constant<'tcx>> {
match self {
Operand::Constant(x) => Some(&**x),
Operand::Copy(_) | Operand::Move(_) => None,
}
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down
52 changes: 51 additions & 1 deletion compiler/rustc_mir/src/transform/instcombine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hir::Mutability;
use rustc_index::vec::Idx;
use rustc_middle::mir::visit::{MutVisitor, Visitor};
use rustc_middle::mir::{
Body, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue,
BinOp, Body, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue,
};
use rustc_middle::ty::{self, TyCtxt};
use std::mem;
Expand Down Expand Up @@ -66,6 +66,11 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> {
*rvalue = Rvalue::Use(Operand::Constant(box constant));
}

if let Some(operand) = self.optimizations.unneeded_equality_comparison.remove(&location) {
debug!("replacing {:?} with {:?}", rvalue, operand);
*rvalue = Rvalue::Use(operand);
}

self.super_rvalue(rvalue, location)
}
}
Expand All @@ -81,6 +86,48 @@ impl OptimizationFinder<'b, 'tcx> {
fn new(body: &'b Body<'tcx>, tcx: TyCtxt<'tcx>) -> OptimizationFinder<'b, 'tcx> {
OptimizationFinder { body, tcx, optimizations: OptimizationList::default() }
}

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());
}
}
}

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);
}
}

return None;
}
}

impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
Expand All @@ -106,6 +153,8 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
}
}

self.find_unneeded_equality_comparison(rvalue, location);

self.super_rvalue(rvalue, location)
}
}
Expand All @@ -114,4 +163,5 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
struct OptimizationList<'tcx> {
and_stars: FxHashSet<Location>,
arrays_lengths: FxHashMap<Location, Constant<'tcx>>,
unneeded_equality_comparison: FxHashMap<Location, Operand<'tcx>>,
}
3 changes: 2 additions & 1 deletion compiler/rustc_mir/src/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,9 @@ fn run_optimization_passes<'tcx>(

// The main optimizations that we do on MIR.
let optimizations: &[&dyn MirPass<'tcx>] = &[
&instcombine::InstCombine,
&match_branches::MatchBranchSimplification,
// inst combine is after MatchBranchSimplification to clean up Ne(_1, false)
&instcombine::InstCombine,
&const_prop::ConstProp,
&simplify_branches::SimplifyBranches::new("after-const-prop"),
&simplify_comparison_integral::SimplifyComparisonIntegral,
Expand Down
35 changes: 35 additions & 0 deletions src/test/mir-opt/equal_true.opt.InstCombine.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
- // MIR for `opt` before InstCombine
+ // MIR for `opt` after InstCombine

fn opt(_1: bool) -> i32 {
debug x => _1; // in scope 0 at $DIR/equal_true.rs:3:8: 3:9
let mut _0: i32; // return place in scope 0 at $DIR/equal_true.rs:3:20: 3:23
let mut _2: bool; // in scope 0 at $DIR/equal_true.rs:4:8: 4:17
let mut _3: bool; // in scope 0 at $DIR/equal_true.rs:4:8: 4:9

bb0: {
StorageLive(_2); // scope 0 at $DIR/equal_true.rs:4:8: 4:17
StorageLive(_3); // scope 0 at $DIR/equal_true.rs:4:8: 4:9
_3 = _1; // scope 0 at $DIR/equal_true.rs:4:8: 4:9
- _2 = Eq(move _3, const true); // scope 0 at $DIR/equal_true.rs:4:8: 4:17
+ _2 = move _3; // scope 0 at $DIR/equal_true.rs:4:8: 4:17
StorageDead(_3); // scope 0 at $DIR/equal_true.rs:4:16: 4:17
switchInt(_2) -> [false: bb1, otherwise: bb2]; // scope 0 at $DIR/equal_true.rs:4:5: 4:34
}

bb1: {
_0 = const 1_i32; // scope 0 at $DIR/equal_true.rs:4:31: 4:32
goto -> bb3; // scope 0 at $DIR/equal_true.rs:4:5: 4:34
}

bb2: {
_0 = const 0_i32; // scope 0 at $DIR/equal_true.rs:4:20: 4:21
goto -> bb3; // scope 0 at $DIR/equal_true.rs:4:5: 4:34
}

bb3: {
StorageDead(_2); // scope 0 at $DIR/equal_true.rs:5:1: 5:2
return; // scope 0 at $DIR/equal_true.rs:5:2: 5:2
}
}

9 changes: 9 additions & 0 deletions src/test/mir-opt/equal_true.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// EMIT_MIR equal_true.opt.InstCombine.diff

fn opt(x: bool) -> i32 {
if x == true { 0 } else { 1 }
}

fn main() {
opt(true);
}
72 changes: 72 additions & 0 deletions src/test/mir-opt/not_equal_false.opt.InstCombine.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
- // MIR for `opt` before InstCombine
+ // MIR for `opt` after InstCombine

fn opt(_1: Option<()>) -> bool {
debug x => _1; // in scope 0 at $DIR/not_equal_false.rs:3:8: 3:9
let mut _0: bool; // return place in scope 0 at $DIR/not_equal_false.rs:3:26: 3:30
let mut _2: bool; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _3: isize; // in scope 0 at $DIR/not_equal_false.rs:4:17: 4:21
let mut _4: bool; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _5: isize; // in scope 0 at $DIR/not_equal_false.rs:4:38: 4:45

bb0: {
StorageLive(_2); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_3 = discriminant(_1); // scope 0 at $DIR/not_equal_false.rs:4:17: 4:21
_2 = Eq(_3, const 0_isize); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
goto -> bb7; // scope 0 at $DIR/not_equal_false.rs:4:17: 4:21
}

bb1: {
_0 = const true; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
goto -> bb4; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
}

bb2: {
_0 = const false; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
goto -> bb4; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
}

bb3: {
StorageLive(_4); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_5 = discriminant(_1); // scope 0 at $DIR/not_equal_false.rs:4:38: 4:45
_4 = Eq(_5, const 1_isize); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
goto -> bb10; // scope 0 at $DIR/not_equal_false.rs:4:38: 4:45
}

bb4: {
StorageDead(_4); // scope 0 at $DIR/not_equal_false.rs:4:45: 4:46
StorageDead(_2); // scope 0 at $DIR/not_equal_false.rs:4:45: 4:46
return; // scope 0 at $DIR/not_equal_false.rs:5:2: 5:2
}

bb5: {
_2 = const false; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
goto -> bb7; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}

bb6: {
_2 = const true; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
goto -> bb7; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}

bb7: {
switchInt(move _2) -> [false: bb3, otherwise: bb1]; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
}

bb8: {
_4 = const false; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
goto -> bb10; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}

bb9: {
_4 = const true; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
goto -> bb10; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}

bb10: {
- _0 = Ne(_4, const false); // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
+ _0 = _4; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
goto -> bb4; // scope 0 at $DIR/not_equal_false.rs:4:5: 4:46
}
}

9 changes: 9 additions & 0 deletions src/test/mir-opt/not_equal_false.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// EMIT_MIR not_equal_false.opt.InstCombine.diff

fn opt(x: Option<()>) -> bool {
matches!(x, None) || matches!(x, Some(_))
}

fn main() {
opt(None);
}