[PATCH] D87817: [LICM][Coroutine] Don't sink stores to coroutine suspend point.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/docs/Coroutines.rst:1561
+#. Make required changes to make sure not to insert or move any instructions
+   into edges from suspended(-1) case of coro.suspend intrinsics to their label.
+
----------------
This isn't what I was looking for.

Say I'm someone who doesn't know anything about coroutines, and I'm writing a new optimization.  What do I need to know about coroutines so I don't break them?


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:400
+
+        if (auto *SWI = dyn_cast<SwitchInst>(PredBB->getTerminator())) {
+          IntrinsicInst *II = dyn_cast<IntrinsicInst>(SWI->getCondition());
----------------
junparser wrote:
> efriedma wrote:
> > Is there any particular reason to expect the edge from the coro_suspend to the coro_end is a switch?
> The result of this coro.suspend lead to basic blocks where coroutine should proceed when suspended (-1), resumed (0) or destroyed (1). we also generate switch from front end. 
> 
> Although sometimes  it would be optimized to conditional branch because of that both of resumed and destroyed case lead to the same basic block. since we are inside loop, the resumed basic block and destroyed basic block should not be same.
> the resumed basic block and destroyed basic block should not be same

"should not be the same" here seems to mean "usually won't be the same"; is there any rule that actually precludes it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87817/new/

https://reviews.llvm.org/D87817