MIR move elimination [1/6]: MIR preparation

[Amanieu]

View all comments

This PR contains preparatory work for supporting the new MIR semantics from rust-lang/rfcs#3943.

• Adds -Zmir-move-elimination which both opts-in to the new semantics and enables the new optimization pass.
• Updates doc comments with the new semantics of MIR under the flag.
• Adds the StorageAlloc MIR statement. This is used by the MIR inliner when it needs to borrow a destination place that hasn't been initialized.
• Fixes several cases where ZST unit values are not properly initialized.
• Fixes async drop elaboration which incorrectly used a move instead of a copy, leading to a use-after-move.

r? tmiasko

[rustbot]

Some changes occurred to constck

cc @fee1-dead

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

[Amanieu]

LLM disclosure: this PR is in large part the result of an LLM-assisted audit of all existing MIR transformations against the new MIR semantics as implemented in Miri in #163340. LLMs were also used to generate the boilerplate for the MIR tests, but each test was manually reviewed and adjusted before committing.

[tmiasko]

We started with a single pull request with 65 files changed, and now ended up with multiple "smaller" pull requests, where the first one has 115 files changed. This is not exactly what I expected. Could you move some parts to smaller pull requests still. For example parts of "Fix missing ZST initialization in MIR passes" which don't depend on -Zmir-move-elimination at all.

[Amanieu]

I'm currently working to make none of the changes in this PR depend on -Zmir-move-elimination. That way we always have MIR that works with both the old and new semantics, and we don't need to make this a target modifier.

The main concern is that this means restricting RemoveZsts to never remove assignments, which is causing a lot of churn in the mir-opt test snapshots. Most of these assignments are later removed by DSE, but most mir-opt tests only run a single MIR pass.

I will then move all of the ZST-related changes to a separate PR since they no longer depend on the flag.

[Amanieu]

I've moved the ZST-related changes to #163359.

We started with a single pull request with 65 files changed, and now ended up with multiple "smaller" pull requests, where the first one has 115 files changed. This is not exactly what I expected.

Most of the difference come from this preparation work which adjusts the rest of the compiler to work with the new semantics, which ends up touching a lot of tests. The actual compiler code changes are not that big.

[Amanieu]

I moved the MIR validity changes to the Miri PR.

[Amanieu]

I addressed all the review comments, as well as #163388. I also noticed that the handling of the destination place in inlining was incorrect: the destination place must become allocated-but-uninitialized if the call unwinds. This means we always need to use StorageAlloc on the destination place (unless it is indirect) since it needs to become initialized even if the inlined callee unwinds.

[RalfJung]

Given that the semantics have been overhauled, we should probably ensure t-opsem had a chance to look at the new semantics. In particular this PR seems to already alter MIR semantics by adding a new statement (StorageAlloc)?

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

I've rebased the PRs to swap their order. Now the review order should be:

• MIR move elimination [2/6]: TailCopyToMove #163336
• MIR move elimination [3/6]: PreciseLiveness #163337
• MIR move elimination [4/6]: MoveElimination #163338
• MIR move elimination [5/6]: Storage reconstruction #163339
• MIR move elimination [1/6]: MIR preparation #163335
• MIR move elimination [1.5/6]: Ensure ZSTs are always initialized #163359
• MIR move elimination [6/6]: Miri support #163340

[rust-bors]

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