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

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 3 22:18:33 PDT 2020


hoy 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
----------------
lxfind wrote:
> hoy wrote:
> > lxfind wrote:
> > > hoy wrote:
> > > > lxfind wrote:
> > > > > 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.
> > > > I see. Is it possible to fix those passes allocating locals before `coro.begin`? If that's too involved, can we move the `coro.begin` to the beginning of a function and redirect all local references to the coro frame? It's basically similar to outline all the function code except for the `coro.begin` call.
> > > The problem is that coro.begin depends on the creation of the frame on the heap, and the creation of the heap sometimes could depend on parameters of the function, which will lead to allocas before coro.begin.
> > > I will need to look into how this is done in the Swift ABI and potentially use that model.
> > How does the frame creation depend on the function local variable values? Does the frame size depend on the values ?
> @hoy My understanding is that you could provide a custom Allocator for the frame creation, which comes from a parameter, which will be moved to a local variable the first thing in the function.
I see. Thanks for the explanation. That sounds like a special use of locals which should have a very short lifetime. For regular local variables, their computation may be able to be deferred until they are allocated on the coroutine frame.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86859



More information about the llvm-commits mailing list