Optimize away BitAnd and BitOr when possible

[xldenis]

This PR lets const_prop optimize away a | true == true , a & false == false and a * 0 = 0. While I was writing this I've realized that constant propagation misses a lot of opportunities. For example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2a4b45e772f214210a36749b27223bb0

Constant propagation doesn't seem to... propagate constants, additionally the way constant propagation is currently setup makes it tricky to add cases like a | false == a.

I tried to organize eval_rvalue_with_identities to make the pattern of the optimizations easier to see but it still obscurs what should be a simple peephole optmization.

cc @oli-obk

[rust-highfive]

r? @estebank

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

[jyn514]

I think you a word.

[estebank]

The code looks fine but I can't take a good look right now. r? @oli-obk for final review.

[xldenis]

I don't understand why CI is failing:

error: failed to run custom build command for `crossbeam-utils v0.7.2`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/release/build/crossbeam-utils-a40e83968eeae062/build-script-build` (exit code: 101)
  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Other("missing patch version") }', /cargo/registry/src/git.luolix.top-1ecc6299db9ec823/autocfg-1.0.0/src/lib.rs:128:20

I have no idea what to do about this? It doesn't seem related to my changes but it's also not flakiness from what I can tell

[estebank]

@xldenis can you try rebasing on top of a recent master? This doesn't seem like something caused by your PR and doing that might be enough to get CI to pass. Otherwise you might be introducing a bug in this PR and it's triggered in crossbeam-utils (this is happening in stage1, not stage0, so it's using the new code).

[xldenis]

I just rebased it from master, so how would I reproduce this locally? do I just run a full stage1 build? reproduced the error just need to figure out why now

[oli-obk]

it seems to work now on CI, you just need to ./x.py fmt

[xldenis]

this optimization causes a failure for a testcase, that I am unsure how to handle:

error: this operation will panic at runtime
  --> /Users/xavier/Code/rust/rust/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs:10:61
   |
LL |     const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
   |                                                             ^^^^^ index out of bounds: the len is 0 but the index is 0
   |
   = note: `#[deny(unconditional_panic)]` on by default

previously it seems like this code would compile. What is the correct behavior here? Remove the optimization for multiplication? Or should this code not compile because it was a ! type anyways?

EDIT: that test was supposed to fail, my opt just changes where the error is raised. I fixed the test output.

[oli-obk]

please add #![warn(unconditional_panic)] to this file so we test the original thing again. This line can the be changed to

Suggested change

	//~| ERROR this operation will panic at runtime [unconditional_panic]
	//~| WARN this operation will panic at runtime [unconditional_panic]

[oli-obk]

cc @lcnr this will make things worse for "some functions that depend on CTFE will evaluate successfully if that generic thing is not needed/used". We already have this situation elsewhere, but we may need to be more aggressive in putting things into the "unevaluated const" list. Right now it's just about generic constants, but after this PR we may need to put function calls of generic functions into it, too.

[oli-obk]

@xldenis this is not a blocker for this PR in general, We may should probably just move your optimization under mir-opt-level 3 until we have the scheme for function calls. What do you think?

[lcnr]

@oli-obk because now 0 * std::mem::size_of::<T>() evaluates without requiring std::mem::size_of::<T>()?

So the main problem here is probably something like

fn test<T>() {
    let _ = [0; SOME_MONOMORPHIC_CONST * std::mem::size_of::<T>()];
}

That means we can't really fix this by just not looking into std::mem::size_of::<T>() as std::mem::size_of::<T>() won't exist in the optimized mir afaict.

It's probably cleaner to add a flag to mir which indicates if it depends on type parameters (during mir_built), so we don't have to care about this while optimizing 🤔

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

The job dist-i686-msvc of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

[ehuss]

@bors retry
rust-lang/cargo#8517

[Manishearth]

@bors rollup=iffy

may have perf implications

[xldenis]

it doesn't do anything unless mir-opt-level=3 so it shouldn't affect normal code.

[Manishearth]

@bors rollup-

Okay, thanks.

@nnethercote we don't perf-check mir-opt=3, yes?

[nnethercote]

@nnethercote we don't perf-check mir-opt=3, yes?

rustc-perf runs cargo rustc --release (opt builds) and cargo rustc (debug builds) and cargo rustc --profile=check (check builds) for each benchmark. No options affecting optimization or codegen are given, i.e. it uses the default mir-opt value, which appears to be 1.

[xldenis]

@nnethercote is there anyway to specify flags for a perf run? I'm curious how much mir-opt-level=2/3 impacts it.

[Manishearth]

Perf runs just do regular try builds, so I don't think that's possible.

You could, however, edit the CI files to use a different set of flags, you just have to do it twice to get the "before" test results.