Split rustc_mir

[cjgillot]

The rustc_mir crate is the second largest in the compiler.
This PR splits it up into 5 crates:

• rustc_borrowck;
• rustc_const_eval;
• rustc_mir_dataflow;
• rustc_mir_transform;
• rustc_monomorphize.

[rust-highfive]

r? @petrochenkov

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

[petrochenkov]

The old dependency graph is

// nobody else depends on rustc_mir
rustc_mir -> rustc_interface

, the new dependency graph is

// nobody else depends on rustc_mir
rustc_mir -> rustc_borrowck -> rustc_interface

Does this actually improve build performance?

[petrochenkov]

r? @ecstatic-morse on the specifics (as gitHub suggests) in case this improves the build in general (in which I'm not sure).

[andjo403]

some strange results from cargo timing for this PR on my 32 thread cpu.
It takes more time to compile rustc_mir with this PR.
do not know if it is due to all cpus is already busy at the time when rustc_mir is compiled.
this is the results from the step "Building stage0 compiler artifacts" when using the command CARGOFLAGS_BOOTSTRAP=-Ztimings ./x.py build --stage 1
cargo timing for this pr
cargo timing before this pr

[cjgillot]

This new version now splits the rustc_mir crate into four:

• rustc_monomorphize ;
• rustc_mir_transform : holds MIR opts ;
• rustc_borrowck : holds the borrow checker ;
• rustc_mir : holds the dataflow framework, and const-eval.

[andjo403]

cargo timing for this pr new version

[bors]

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

[ecstatic-morse]

@cjgillot Please update the PR summary and title to reflect the status quo.

I think this is the right direction. Even though rustc_mir is not on the critical path for a fresh compile, it will improve compile times for people working on MIR transforms. Splitting up crates makes for better APIs in the long run as well, since people are forced to think about encapsulation.

The dataflow framework should also be separable with little effort. It depends on a few util methods for graphviz rendering, which could be moved into the rustc_graphviz crate or into the dataflow crate itself, and the Borrow indexes for defining the Borrows dataflow analysis, which can be moved into rustc_borrowck. After this, rustc_mir could become rustc_const_eval and util could be split up between its consumers and rustc_middle.

I think it's beneficial to do these big refactorings all in one go, so that people don't have to adjust their workflow for the intermediate state, only for it to change again a few months later. I also think you should try to publicize these changes somehow.

r? @wesleywiser (compiler-team lead and MIR opt person) for a final sign-off or referral to MCP.

[RalfJung]

Having to expose these methods publicly is not great.... that's the issue with this crate splitting business: it makes it a lot harder to uphold invariants inside modules like the interpreter engine.

[rust-timer]

Finished benchmarking commit (97032a6): comparison url.

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

• Large improvement in instruction counts (up to -2.0% on full builds of deeply-nested-async)
• Moderate regression in instruction counts (up to 0.8% on full builds of await-call-tree)

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

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

[Mark-Simulacrum]

@cjgillot @oli-obk This seems to have hurt the bootstrap time by perf.r-l.o's measurement and the results for individual benchmarks are also mixed. Is there reason to believe that measurement is flawed? Did we measure before this splitting? (I suppose recompilations with smaller changes may be helped by it...).

[bjorn3]

perf.r-l.o only checks the bootstra time for individual crates. It doesn't check the bootstrap time when multiple crates can compile at the same time, which is what this was meant to improve.

[Mark-Simulacrum]

I think the regressions here are unfortunate, but I don't think we're going to do anything about them. I think it's potentially true there's more parallelism here but it's also true that these crates are somewhat interconnected regardless (the dependency tree goes through most of the new crates linearly, rather than as siblings), but regardless, it seems like a better factoring is in theory going to make other aspects of this easier in the future.

Marking as triaged.

@rustbot label +perf-regression-triaged