Enable MIR inlining in incremental mode too.

[cjgillot]

Gating MIR inlining on non-incremental mode causes spurious codegen test failures for contributors that do not compile rustc in incremental mode.

This PR stops this behaviour, to rely only on optimization options. In general, release mode disables incremental, so this should not change much for end users.

Fixes #99619

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[jackh726]

r? @oli-obk who reviewed the enable MIR inlining PR

[eddyb]

causes spurious codegen test failures for contributors that do not compile rustc in incremental mode

Note that it causes failures for people who do, because CI tests with incremental disabled.

[eddyb]

Could this match on optimize? The names are kind of confusing, and seeing them ordered by strength could make me more comfortable.

Alternatively, >= Default (assuming Default is -O aka -C opt-level=2?) would be another way to indicate that it "starts" somewhere and increases with opt-level.

[eddyb]

Could you add "Fixes #99619" to the issue description? For reference, it's this issue:

• ./x.py test src/test/codegen fails #99619

[bjorn3]

Won't this slow down compilation for people who compile rustc with incremental enabled? Rustc is always compiled in release mode.

[cjgillot]

There is a subtle ICE in incremental-ignore-spans mode used by the incremental test suite. This PR is blocked on me fixing that ICE.

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit ffec672 with merge 5a39b2d33adc481de0478cd0c3a78889b2733498...

[bors]

☀️ Try build successful - checks-actions
Build commit: 5a39b2d33adc481de0478cd0c3a78889b2733498 (5a39b2d33adc481de0478cd0c3a78889b2733498)

[rust-timer]

Queued 5a39b2d33adc481de0478cd0c3a78889b2733498 with parent 2f847b8, future comparison URL.

[rust-timer]

Finished benchmarking commit (5a39b2d33adc481de0478cd0c3a78889b2733498): comparison url.

Instruction count

• Primary benchmarks: mixed results
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	6.8%	52.9%	41
Regressions 😿 (secondary)	2.6%	5.4%	9
Improvements 🎉 (primary)	-1.7%	-3.1%	5
Improvements 🎉 (secondary)	-1.6%	-2.6%	3
All 😿🎉 (primary)	5.9%	52.9%	46

Max RSS (memory usage)

Results

• Primary benchmarks: 😿 relevant regressions found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	6.2%	18.1%	24
Regressions 😿 (secondary)	2.7%	3.6%	4
Improvements 🎉 (primary)	-2.6%	-2.6%	1
Improvements 🎉 (secondary)	-2.9%	-2.9%	1
All 😿🎉 (primary)	5.8%	18.1%	25

Cycles

Results

• Primary benchmarks: mixed results
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	8.0%	34.9%	24
Regressions 😿 (secondary)	3.8%	6.0%	5
Improvements 🎉 (primary)	-3.2%	-5.5%	5
Improvements 🎉 (secondary)	-2.4%	-2.4%	1
All 😿🎉 (primary)	6.0%	34.9%	29

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[cjgillot]

The perf impact has to be expected. Perf runs the compiler in incremental+optimizations mode, which is not a typical work case. The faster LLVM optimizations do not happen as often as in the non-incremental case, probably because we don't use the same amount of CGUs in the two cases. The cost centers are:

• loading optimized MIR from incremental cache ;
• writing optimized MIR to incremental cache ;
• writing metadata ;
• collecting monomorphizations.

I conclude that the regression is entirely due to larger MIR. The only workaround would be to tweak MIR inlining threshold for size.

[JakobDegen]

The one thing I'd note about the perf report is that (if I'm reading things correctly) the size of compiled binaries seems to have increased. That seems concerning, because it means we're inlining a lot of things that LLVM is not, which we generally shouldn't be. Lowering the threshold is one path towards fixing this, but we should potentially be revisiting the algorithm for calculating the decision more generally

[jyn514]

The original issue has been fixed since the PR was opened, I think because we had a bootstrap bump so both stage 1 and stage 2 are now using MIR inlining. Does it still make sense to keep this PR open?