Which MIR passes should run on generator drop shims?

[ecstatic-morse]

Under normal circumstances, MIR shims have the following 5 passes applied:

• AddMovesForPackedDrops
• RemoveNoopLandingPads
• SimplifyCfg (a trivial cleanup pass)
• CriticalCallEdges
• AbortUnwindingCalls

However, generator drop shims are created by cloning the MIR for the original generator (?), which means they start out in MirPhase::GeneratorLowering. Prior to #91475, run_passes silently returned without doing anything if the MirPhase was greater than or equal to the expected one, so the aforementioned MIR passes were never executed. That behavior was only relevant when building drop shims, so #91475 added an explicit check to preserve it.

Was this intentional? Notably AddMovesForPackedDrops and AbortUnwindingCalls have soundness implications, and CriticalCallEdges is needed to work around some codegen issues, so it seems like they should always run. However, they might not be applicable to generators specifically.

Perhaps @tmandry knows the answer, or knows someone who does?

[tmandry]

I'm not sure why those passes aren't run, and I haven't done much on the code with drop shims. @Zoxc might know. @jonas-schievink also made some recent-ish changes in that code.

[tmiasko]

The AddMovesForPackedDrops and AbortUnwindingCalls should at some point run on drop shim MIR. As far as I can see, precisely because the drop shim starts as a clone of original MIR those passes are already in effect. (There are new drop terminators introduced afterwards, to drop a generator in an unresumed state, but since generator itself is not packed and drops are shallow that seems fine, since those drops will be responsible for introducing any extra moves as necessary).

On the other hand, the omission of CriticalCallEdges immediately before the code generation might be problematic.

[RalfJung]

This issue also means that MIR validation doesn't run on generator drop shims, which makes me uneasy. Or has the validator been run somewhere earlier?

the drop shim starts as a clone of original MIR

Which original MIR and where/how is that generated?

Prior to #91475, run_passes silently returned without doing anything if the MirPhase was greater than or equal to the expected one, so the aforementioned MIR passes were never executed. That behavior was only relevant when building drop shims, so #91475 added an explicit check to preserve it.

What would go wrong if this made the same run_passes call as the other shims?