[PATCH] D86859: [Coroutine] Make dealing with alloca spills more robust

[Xun Li via Phabricator via llvm-commits]

lxfind added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:634-638
+// TODO: If the pointer is really escaped, we are in big trouble because we
+// will be escaping a pointer to a stack address that would no longer exist
+// soon. However most escape analysis isn't good enough to precisely tell,
+// so we are assuming that if a pointer is escaped that it's written into.
+// TODO: Another potential issue is if we are creating an alias through
----------------
hoy wrote:
> This does sound fragile. I'm wondering if this is a defined behavior in the language standard. For a local variable first allocated on the local frame and later on copied into the coroutine frame, all subsequent accesses to it should be redirected to the coroutine frame? If so, can the coroutine frame be allocated ealier? 
This is defined behavior in the language. How to manage the data in the heap is implementation details and should not affect program correctness. So it's certainly true that this implementation is not quite right yet. The hard part is that some optimization passes will insert allocas before coro.begin, which makes it difficult to track. A proper implementation should provide strong guarantees.
This patch only makes existing algorithm slightly more robust, but does not solve the more fundamental problem, which will take some time to design and implement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86859