Remove redundant drop terminators ahead of mir_coroutine_witnesses#155022
Remove redundant drop terminators ahead of mir_coroutine_witnesses#155022tmandry wants to merge 5 commits intorust-lang:mainfrom
Conversation
|
@bors try |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Remove redundant drop terminators ahead of mir_coroutine_witnesses
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (5cfde26): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 6.7%, secondary 5.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.9%, secondary 2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary -0.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 489.506s -> 497.499s (1.63%) |
This comment was marked as off-topic.
This comment was marked as off-topic.
4d04916 to
11e9fef
Compare
|
@bors try |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Remove redundant drop terminators ahead of mir_coroutine_witnesses
|
The job Click to see the possible cause of the failure (guessed by this bot) |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (dc4f33d): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 6.7%, secondary 4.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 1.8%, secondary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary -0.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 489.468s -> 489.718s (0.05%) |
This change is meant to fix #151942. Putting this up to see the perf effects of running it on every MIR body; then I'll work out a strategy to merge it.
Related issues to triage:
awaitinasync fn#87309Coroutine traits like Send and Sync are computed based on the
mir_coroutine_witnessesquery to get the types saved in the coroutine. This query runs the regular coroutine analysis pass on mir_promoted to compute which locals are saved across suspension points.At this stage of MIR, drop terminators exist along every exit path of the function, even when a local has already been moved from. Those extra drop terminators causes locals (like
guardin the case below) to appear live over suspension points, even when they have just been dropped.See this gist for MIR of a simplified version of this function.
Later MIR passes like drop_elaboration and remove_unneeded_drops remove the redundant drop terminators, but they depend on type info to check needs_drop on a place's type. Since
mir_coroutine_witnessesquery is run inside typeck, we cannot depend on this without introducing cycles.Instead, we implement a simple pass that removes drop terminators for definitely-uninitialized places. It avoids the need for type info by skipping the check for whether types have destructors.
r? @ghost