Repository: rust
Issue: More precise drop elaboration
Labels: C-enhancement,I-compiletime,T-compiler,A-MIR

#68943 and #68528 changed drop elaboration to reduce the number of `Drop` terminators emitted for enums. Originally, these PRs were intended only to solve #66753, but they also resulted in [significant perf improvements](https://perf.rust-lang.org/compare.html?start=a8437cf213ac1e950b6f5c691c4d2a29bf949bcd&end=d9051341a1c142542a3f7dab509266606c775382&stat=instructions%3Au) for codegen-ed builds (look only at the "clean" runs).

It may be possible to find more perf wins by making drop elaboration more precise. Notably, #68528 added a `discriminant_switch_effect` to `MaybeInitializedPlaces` but did not make the equivalent change to `MaybeUninitializedPlaces`. As a result, some frivolous drop terminators remain after drop elaboration (e.g., `bb5` in [this recreation of `Option::unwrap`](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4e59ddba525666d19fba8d706f2aa822)). This occurs because the move path for the local containing the `Option` is calculated to be "maybe live" by drop elaboration, but is determined *not* to be "maybe dead" since `MaybeUninitializedLocals` does not mark the move path for the `Option::Some(_)` variant as uninitialized. As a result, elaboration considers the drop of `Option<T>` along the unwind edge as ["static" instead of "open"](https://github.com/rust-lang/rust/blob/e0e5d82e1677c82d209b214bbfc2cc5705c2336a/src/librustc_mir/transform/elaborate_drops.rs#L191). Only "open" drops trigger the new code introduced in #68943 for fieldless enum variants such as `Option::None`. 

On the other hand, marking more drops of enums as "open" ones could *increase* the amount of MIR we need to codegen. If we can't prove that they are frivolous, open drops cause drop flags to be created within a function's MIR, while static drops simply invoke the drop shim. I'm not confident enough in my understanding of drop elaboration to be sure of this, however.

`MaybeUninitializedLocals` is also used in the borrow-checker, so care should be taken when modifying it. In the worst case, we could use a different version of that analysis for drop elaboration than for borrow-checking. However, using a more precise variant of the analysis during borrow-checking may have positive effects.

@eddyb also suggested incorporating information about the active enum variant during drop elaboration, similar to what [`NeedsDrop`](https://github.com/rust-lang/rust/blob/e0e5d82e1677c82d209b214bbfc2cc5705c2336a/src/librustc_mir/transform/check_consts/qualifs.rs#L227) does for const-checking. If this were implemented, we could eliminate all drops of an enum that occur after it is assigned a variant that needs no drop glue (e.g. `Option::None`) with no intervening assignment. I'm more skeptical of this approach than the first, since it becomes pretty useless as soon as a pointer exists that could mutate the place (unless we are willing to do a much more complex alias analysis). Perhaps they disagree?

In summary, I think we (myself and possibly @rust-lang/wg-mir-opt) should investigate the following ideas, with the first one given higher priority:
- [x] Add a `discriminant_switch_effect` to `MaybeUninitializedPlaces` in the style of #68528.
- [ ] Track assignments of enum variants during drop elaboration.