Extract rustc_mir_pretty crate from rustc_middle

[camsteffen]

This shrinks and removes dependencies from rustc_middle so I'm hoping this is beneficial for bootstrap times. It also just seems sensible and consistent.

[rust-highfive]

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

[rust-highfive]

r? @matthewjasper

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

[camsteffen]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 3dbbb3ff8f9c8a920421a6c5e33c1311a68e2653 with merge 69a39b977c724cb08a032b6ff8bdb84342af6190...

[bors]

☀️ Try build successful - checks-actions
Build commit: 69a39b977c724cb08a032b6ff8bdb84342af6190 (69a39b977c724cb08a032b6ff8bdb84342af6190)

[rust-timer]

Queued 69a39b977c724cb08a032b6ff8bdb84342af6190 with parent 0157cc9, future comparison URL.

[rust-timer]

Finished benchmarking commit (69a39b977c724cb08a032b6ff8bdb84342af6190): comparison url.

Summary: This change led to large relevant regressions 😿 in compiler performance.

• Large regression in instruction counts (up to 2.3% on full builds of cranelift-codegen)

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 led 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

[camsteffen]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 24b2981bf03ad68dc167124a16e9d90ec9619b6f with merge da7f27691b881b90b3f2ca16092dfb032114a0ab...

[bors]

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

[rust-timer]

Queued da7f27691b881b90b3f2ca16092dfb032114a0ab with parent e0aaffd, future comparison URL.

[rust-timer]

Finished benchmarking commit (da7f27691b881b90b3f2ca16092dfb032114a0ab): comparison url.

Summary: This change led to large relevant regressions 😿 in compiler performance.

• Large regression in instruction counts (up to 2.3% on full builds of cranelift-codegen)

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 led 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

[camsteffen]

Shucks. I'd love to be mentored in diagnosing perf regressions. Otherwise I may have to close this.

[oli-obk]

One way to find out where to add inline attributes is to run rustc via cachegrind before and after your change on one of the regressed benchmarks. Then let kcachegrind give you a diff view between those runs. From there it should be obvious what functions were inlined before and aren'tnow, as you'll suddenly see a huge amount of new executions of some functions. Those will then need inlining...

If you have the extra time, could you write down more detailed instructions as you go (with exakt commands that you used for your situation)? I should have done so last time and added it to the guide, but I didn't think of it

[bors]

☔ The latest upstream changes (presumably #90577) made this pull request unmergeable. Please resolve the merge conflicts.

[camsteffen]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 7c20412ad5f030624cbe6d5636f9aa02ce0e8e79 with merge 9c5b805c7bd92ff9c7fb1074cfc55ebfd1b71a8b...

[bors]

☀️ Try build successful - checks-actions
Build commit: 9c5b805c7bd92ff9c7fb1074cfc55ebfd1b71a8b (9c5b805c7bd92ff9c7fb1074cfc55ebfd1b71a8b)

[rust-timer]

Queued 9c5b805c7bd92ff9c7fb1074cfc55ebfd1b71a8b with parent d608229, future comparison URL.

[rust-timer]

Finished benchmarking commit (9c5b805c7bd92ff9c7fb1074cfc55ebfd1b71a8b): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

• Small improvement in instruction counts (up to -0.2% on full builds of many-assoc-items)
• Moderate regression in instruction counts (up to 0.4% on incr-unchanged builds of style-servo)

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 led 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

[camsteffen]

@rustbot label: -S-waiting-on-author

[camsteffen]

Well it's still a small regression (noise?) so I don't know if this is desirable.

[bors]

☔ The latest upstream changes (presumably #91388) made this pull request unmergeable. Please resolve the merge conflicts.

[apiraino]

Mentioned during today's compiler meeting on Zulip.

In order to move this further on, this perf. regression should be investigated.
Another question was if adding any inline attributes suggested in this comment were tried and, if yes, which results were observed, if there is any feedback on this.

Finally a question about any possible build time improvement with -j2 or higher (perf.r-l.o builds with-j1 and 2 threads).

[camsteffen]

In order to move this further on, this perf. regression should be investigated.
Another question was if adding any inline attributes suggested in this comment were tried and, if yes, which results were observed, if there is any feedback on this.

Don't we only care about the most recent perf run?
I did try this and I didn't see anything in the cachegrind output diff that looked like a candidate for inlining. I did however submit #89893 and the subsequent perf run looks neutral to me.

Finally a question about any possible build time improvement with -j2 or higher (perf.r-l.o builds with-j1 and 2 threads).

Hmm I'm not sure. Should I just time it locally?

[Mark-Simulacrum]

The stated motivation in the PR description is that this improves bootstrap times, so before we proceed we should actually have some numbers showing that's the case - current perf.rlo data rather shows the opposite (or at least net neutral/within margin of error).

If we don't observe bootstrap time improvement, it may still make sense to merge as a cleanup/refactoring improving code quality, but we should be clear about that being the motivation rather than bootstrap perf.

Running numbers locally should be good, and looking at x.py check performance would also be good (perf.rlo reflects x.py build performance only).

[bjorn3]

perf.r-l.o shows the performance of building each crate individually. It doesn't show the effect of building multiple crates in parallel.

[apiraino]

@camsteffen checking if the s-waiting-on-review label is correct here. Do you need clarifications or further review after the latest comments? I'll tentatively set this to be waiting on author, but please ask for another check if it's the case, thanks!

@rustbot label -S-waiting-on-review +S-waiting-on-author

[Dylan-DPC]

Closing this due to inactivity