[PATCH] D89768: [Coroutine] Properly determine whether an alloca should live on the frame

[JunMa via Phabricator via llvm-commits]

junparser added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:921
+      for (auto *BB2 : UserBBs)
+        if (Checker.hasPathCrossingSuspendPoint(BB1, BB2))
+          return true;
----------------
lxfind wrote:
> junparser wrote:
> > Still need call Checker.hasPathCrossingSuspendPoint(AllocaDef,  Pointeruser)
> We cannot do that because the whole purpose of this rewrite is to check if uses of the alloca belong to different suspend regions, which causes the spill. 
> I thought about this a bit more, and my conclusion is that although the API is not intended to be used this way, it actually works here.
> The goal for this loop is to check whether an alloca needs to live across a suspension. If an alloca needs to live across a suspension, it means there exists a coro.suspend such that a user BB happens before it and a user BB happens after it, and these two BBs must dominate each other through coro.susend. Hence one of the user BB pairs must return true on the hasPathCrossingSuspendPoint check. On the other hand, for BBs that don't even dominate each other, hasPathCrossingSuspendPoint will return false anyway and won't affect the algorithm.
let's say :


```
void foo () {
  alloca a;
  if (cond)
    {
      co_await
      use1 a;
    }else
      use2 a;
}
```
in such case, hasPathCrossingSuspendPoint(use1, use2) return false which means a is not keep on frame, this is wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89768