Page MenuHomePhabricator

[coro] Fix rematerializable instruction sinking to coro.suspend blocks
ClosedPublic

Authored by aschwaighofer on Jun 10 2021, 11:05 AM.

Details

Summary

There is a constraint that coro.suspend instructions need to be in their
own blocks. The coro split pass initially creates IR that obeys this constraint
(which is later checked). Sinking rematerializable instructions into these
blocks breaks that constraint.

Instead rematerialize in the predecessor block to the suspend's single
predecessor block.

Diff Detail

Event Timeline

aschwaighofer created this revision.Jun 10 2021, 11:05 AM
aschwaighofer requested review of this revision.Jun 10 2021, 11:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2021, 11:05 AM
nate_chandler added inline comments.
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
1952

This should this be getSinglePredecessor(), right?

aschwaighofer added inline comments.Jun 23 2021, 11:22 AM
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
1952

Thanks, for catching that. Yes we need to rematerialized the value in the predecessor block to the the suspend if the suspend uses it.

getSingleSuccessor call should be getSinglePredecessor

rjmccall accepted this revision.Jun 23 2021, 12:51 PM

Ah, and this is one of those things that probably doesn't affect the C++ lowering because it doesn't pass information directly to and from coro.suspend. LGTM.

This revision is now accepted and ready to land.Jun 23 2021, 12:51 PM
This revision was landed with ongoing or failed builds.Mon, Jun 28, 9:38 AM
This revision was automatically updated to reflect the committed changes.