Skip to content

Commit 0989a77

Browse files
authored
[Coroutines] Verify normalization was not missed (llvm#108096)
* 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.
1 parent adc1ab3 commit 0989a77

File tree

2 files changed

+12
-2
lines changed

2 files changed

+12
-2
lines changed

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
12841284

12851285
// If we have a single edge PHINode, remove it and replace it with a
12861286
// reload from the coroutine frame. (We already took care of multi edge
1287-
// PHINodes by rewriting them in the rewritePHIs function).
1287+
// PHINodes by normalizing them in the rewritePHIs function).
12881288
if (auto *PN = dyn_cast<PHINode>(U)) {
12891289
assert(PN->getNumIncomingValues() == 1 &&
12901290
"unexpected number of incoming "

llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,13 @@ SuspendCrossingInfo::SuspendCrossingInfo(
165165
// Mark all CoroEnd Blocks. We do not propagate Kills beyond coro.ends as
166166
// the code beyond coro.end is reachable during initial invocation of the
167167
// coroutine.
168-
for (auto *CE : CoroEnds)
168+
for (auto *CE : CoroEnds) {
169+
// Verify CoroEnd was normalized
170+
assert(CE->getParent()->getFirstInsertionPt() == CE->getIterator() &&
171+
CE->getParent()->size() <= 2 && "CoroEnd must be in its own BB");
172+
169173
getBlockData(CE->getParent()).End = true;
174+
}
170175

171176
// Mark all suspend blocks and indicate that they kill everything they
172177
// consume. Note, that crossing coro.save also requires a spill, as any code
@@ -179,6 +184,11 @@ SuspendCrossingInfo::SuspendCrossingInfo(
179184
B.Kills |= B.Consumes;
180185
};
181186
for (auto *CSI : CoroSuspends) {
187+
// Verify CoroSuspend was normalized
188+
assert(CSI->getParent()->getFirstInsertionPt() == CSI->getIterator() &&
189+
CSI->getParent()->size() <= 2 &&
190+
"CoroSuspend must be in its own BB");
191+
182192
markSuspendBlock(CSI);
183193
if (auto *Save = CSI->getCoroSave())
184194
markSuspendBlock(Save);

0 commit comments

Comments
 (0)