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

[simonvandel]

Add peephole optimization that simplifies Ne(_1, false) and Ne(false, _1) into _1. Similarly handles Eq(_1, true) and Eq(true, _1).

This was observed emitted from the MatchBranchSimplification pass.

[rust-highfive]

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

[pickfire]

I don't know much about peephole optimization but looks good to me.

[pickfire]

Would this affect compile time? Should we run a timer?

[simonvandel]

My intuition says that this shouldn't be that expensive but let's try perf bot to be sure?

[pickfire]

@jyn514 Can you please help to run perf bot?

[davidtwco]

LGTM, r=me after perf

[davidtwco]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

@davidtwco: 🔑 Insufficient privileges: not in try users

[davidtwco]

@bors try

[bors]

⌛ Trying commit 320929a2d83ac8a960254eb89380e08dd5f58c55 with merge 13cacc3bbd9269bca180ad9e3a9d2bed057b60d1...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 13cacc3bbd9269bca180ad9e3a9d2bed057b60d1 (13cacc3bbd9269bca180ad9e3a9d2bed057b60d1)

[rust-timer]

Queued 13cacc3bbd9269bca180ad9e3a9d2bed057b60d1 with parent b1d092c, future comparison URL.

[rust-timer]

Finished benchmarking try commit (13cacc3bbd9269bca180ad9e3a9d2bed057b60d1): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

[simonvandel]

@davidtwco rebased on master to fix merge conflicts. It should be ready for a new bors try

[davidtwco]

I'm not sure what conclusions can be drawn from the perf results - going to let someone else take a look before merging.

r? @wesleywiser

@davidtwco rebased on master to fix merge conflicts. It should be ready for a new bors try

I don't think we'll need another perf run, so another try build isn't necessary.

[wesleywiser]

This looks good to me but I think for consistency we should do the same thing for Eq(_, true) | Eq(true, _). @simonvandel if you don't want to do that in the PR, let me know so we can open an issue instead and someone else can work on it.

[simonvandel]

I can extend the PR to also handle the Eq case today, sure.

[simonvandel]

@wesleywiser added a new commit so that Eq is handled similarly

[wesleywiser]

We usually do not rollup MIR optimizations even if they don't seem to have a performance impact because it would be best for bisection purposes if this does not end up in a rollup. Therefore

@bors r+ rollup=never

[bors]

📌 Commit 14cd6da2fdcf38d7c226ea8bfaf530357153e0e1 has been approved by wesleywiser

[pickfire]

@wesleywiser Do we usually not run timer for this?

[wesleywiser]

@pickfire I think since just the Ne optimization had little effect, both the Ne and Eq ones will probably not have a huge effect either. perf.rlo will run on the merge commit so we will be able to see exactly what the impact is.

[bors]

⌛ Testing commit 14cd6da2fdcf38d7c226ea8bfaf530357153e0e1 with merge e87c8a880736e304322e8d5be6b51ee33a2dd162...

[bors]

💔 Test failed - checks-actions

[simonvandel]

So I don't know why exactly the test failed:

-	  fn opt(_1: std::option::Option<()>) -> bool {
+	  fn opt(_1: Option<()>) -> bool {

I rebased on master locally, rebless and that did change my test. So i have now force pushed that. @wesleywiser can you start the bors dance again?

[nagisa]

@bors r=wesleywiser

[bors]

📌 Commit 9b0fc62 has been approved by wesleywiser

[bors]

⌛ Testing commit 9b0fc62 with merge 4b65872...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: wesleywiser
Pushing 4b65872 to master...