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

[JunMa via Phabricator via llvm-commits]

junparser 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.
+
----------------
efriedma wrote:
> 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?
The coroutine framework is designed to not interfere other transformation, Ideally, we do not need know anything about coroutine when write optimization. so this is still temporary fix. 


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:400
+
+        if (auto *SWI = dyn_cast<SwitchInst>(PredBB->getTerminator())) {
+          IntrinsicInst *II = dyn_cast<IntrinsicInst>(SWI->getCondition());
----------------
efriedma wrote:
> 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?
> > 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?

Except we write llvm ir directly with two branches, it is always  a switch inside loop.


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

https://reviews.llvm.org/D87817