[PATCH] D29102: [coroutines] Spill the result of the invoke instruction correctly

[David Majnemer via Phabricator via llvm-commits]

majnemer accepted this revision.
majnemer added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:427
+        if (isa<Argument>(CurrentValue)) {
+          // For, argument, we will place the store instruction right after
+          // the coroutine frame pointer instruction, i.e. bitcase of
----------------
Maybe "For arguments,"


================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:428
+          // For, argument, we will place the store instruction right after
+          // the coroutine frame pointer instruction, i.e. bitcase of
+          // coro.begin from i8* to %f.frame*.
----------------
bitcase -> bitcast


================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:431-432
+          InsertPt = FramePtr->getNextNode();
+        }
+        else if (auto *II = dyn_cast<InvokeInst>(CurrentValue)) {
+          // If we are spilling the result of the invoke instruction, split the
----------------
Make this one line.


================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:437-438
+          InsertPt = NewBB->getTerminator();
+        }
+        else {
+          // For all other values, the spill is placed immediately after
----------------
Ditto.


================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:439
+        else {
+          // For all other values, the spill is placed immediately after
+          // the definition.
----------------
We should probably assert that the instruction is not a terminator. Otherwise, this code would also be wrong. Today there are no such instructions but who knows what tomorrow will bring :)


https://reviews.llvm.org/D29102