[PATCH] D37112: [coroutines] Always place escaped allocas into the coroutine frame

[Lewis Baker via Phabricator via llvm-commits]

lewissbaker added inline comments.


================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:889
     for (User *U : I.users())
-      if (Checker.isDefinitionAcrossSuspend(I, U)) {
+      if (Checker.isDefinitionAcrossSuspend(I, U) || allocaEscapes(&I)) {
         // We cannot spill a token.
----------------
lewissbaker wrote:
> Does this mean that passing the address of any `alloca`d memory to a function call will cause it to be spilled even if its lifetime does not extend past a suspension point?
> 
> I'm thinking of cases where we have:
> ```
> {
>   co_await something;
>   {
>     int x = 123;
>     escapes(&x);
>   }
>   co_await somethingElse;
> }
> ```
> 
> Am I correct in understanding that with the proposed logic that `x` would be spilled to the frame even thought its lifetime does not cross a suspension-point? Can we avoid that?
> 
> Should we be checking for `llvm.lifetime.start` / `llvm.lifetime.end` intrinsics and if found only considering the result of `allocaEscapes(&I)` if the lifetime start/end span contains a suspension point?
> 
You're calling `allocaEscapes(&I)` for each `User` in `I.users()` here but the call doesn't seem to depend on the user.

Can the call to `allocaEscapes(&I)` be moved out of the loop and just called once?


https://reviews.llvm.org/D37112