#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 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). 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". 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 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:
#68943 and #68528 changed drop elaboration to reduce the number of
Dropterminators emitted for enums. Originally, these PRs were intended only to solve #66753, but they also resulted in significant perf improvements 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_effecttoMaybeInitializedPlacesbut did not make the equivalent change toMaybeUninitializedPlaces. As a result, some frivolous drop terminators remain after drop elaboration (e.g.,bb5in this recreation ofOption::unwrap). This occurs because the move path for the local containing theOptionis calculated to be "maybe live" by drop elaboration, but is determined not to be "maybe dead" sinceMaybeUninitializedLocalsdoes not mark the move path for theOption::Some(_)variant as uninitialized. As a result, elaboration considers the drop ofOption<T>along the unwind edge as "static" instead of "open". Only "open" drops trigger the new code introduced in #68943 for fieldless enum variants such asOption::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.
MaybeUninitializedLocalsis 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
NeedsDropdoes 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:
discriminant_switch_effecttoMaybeUninitializedPlacesin the style of Mark other variants as uninitialized after switch on discriminant #68528.