Page MenuHomePhabricator

[Coroutines] Fix PR45130

Authored by junparser on Mar 17 2020, 10:54 PM.



For now, when final suspend can be simplified by simplifySuspendPoint, handleFinalSuspend is executed as well to remove last case in switch instruction. This causes issue in, this patch fixes it.

TestPlan: check-llvm

Diff Detail

Event Timeline

junparser created this revision.Mar 17 2020, 10:54 PM
modocache accepted this revision.Mar 19 2020, 7:34 PM

Excellent, thank you! I had one question, otherwise this looks great!


Beginner question of mine: what is this assert testing? I would guess that it's testing that SI != nullptr, but the cast<CoroSuspendInst>(...) above would also fail were that the case. As far as I know CoroSuspendInst and its class hierarchy don't implement a boolean conversion operator either... so if this is a null check, I think it's redundant and can be removed, but if it does something more, please let me know!

This revision is now accepted and ready to land.Mar 19 2020, 7:34 PM
junparser marked an inline comment as done.Mar 19 2020, 8:20 PM

Thanks for review! :)


Thanks! you are completely right. This redundant assert testing added here is just my unintentional behavior.

junparser updated this revision to Diff 251546.Mar 19 2020, 8:25 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2020, 8:51 PM