Skip to content

Commit

Permalink
[Coroutines] Verify normalization was not missed (llvm#108096)
Browse files Browse the repository at this point in the history
* Add asserts to verify normalization of the coroutine happened.
* This will be important when normalization becomes part of an ABI
object.

--- From a previous discussion here
llvm#108076

Normalization performs these important steps:

split around each suspend, this adds BBs before/after each suspend so
each suspend now exists in its own block.
split around coro.end (similar to above)
break critical edges and add single-edge phis in the new blocks (also
removing other single-edge phis).
Each of these things can individually be tested
A) Check that each suspend is the only inst in its BB
B) Check that coro.end is the only inst in its BB
C) Check that each edge of a multi-edge phis is preceded by single-edge
phi in an immediate pred

For 1) and 2) I believe the purpose of the transform is in part for
suspend crossing info's analysis so it can specifically 'mark' the
suspend blocks and identify the end of the coroutine. There are some
existing places within suspend crossing info that visit the CoroSuspends
and CoroEnds so we could check A) and B) there.

For 3) I believe the purpose of this transform is for insertSpills to
work properly. Infact there is already a check for the result of this
transform!

assert(PN->getNumIncomingValues() == 1 &&
           "unexpected number of incoming "
           "values in the PHINode");

I think to verify the result of normalization we just need to add checks
A) and B) to suspend crossing info.
  • Loading branch information
TylerNowicki authored Sep 12, 2024
1 parent adc1ab3 commit 0989a77
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Coroutines/CoroFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1284,7 +1284,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {

// If we have a single edge PHINode, remove it and replace it with a
// reload from the coroutine frame. (We already took care of multi edge
// PHINodes by rewriting them in the rewritePHIs function).
// PHINodes by normalizing them in the rewritePHIs function).
if (auto *PN = dyn_cast<PHINode>(U)) {
assert(PN->getNumIncomingValues() == 1 &&
"unexpected number of incoming "
Expand Down
12 changes: 11 additions & 1 deletion llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,13 @@ SuspendCrossingInfo::SuspendCrossingInfo(
// Mark all CoroEnd Blocks. We do not propagate Kills beyond coro.ends as
// the code beyond coro.end is reachable during initial invocation of the
// coroutine.
for (auto *CE : CoroEnds)
for (auto *CE : CoroEnds) {
// Verify CoroEnd was normalized
assert(CE->getParent()->getFirstInsertionPt() == CE->getIterator() &&
CE->getParent()->size() <= 2 && "CoroEnd must be in its own BB");

getBlockData(CE->getParent()).End = true;
}

// Mark all suspend blocks and indicate that they kill everything they
// consume. Note, that crossing coro.save also requires a spill, as any code
Expand All @@ -179,6 +184,11 @@ SuspendCrossingInfo::SuspendCrossingInfo(
B.Kills |= B.Consumes;
};
for (auto *CSI : CoroSuspends) {
// Verify CoroSuspend was normalized
assert(CSI->getParent()->getFirstInsertionPt() == CSI->getIterator() &&
CSI->getParent()->size() <= 2 &&
"CoroSuspend must be in its own BB");

markSuspendBlock(CSI);
if (auto *Save = CSI->getCoroSave())
markSuspendBlock(Save);
Expand Down

0 comments on commit 0989a77

Please sign in to comment.