[PATCH] D66230: [coroutine] Fixes "cannot move instruction since its users are not dominated by CoroBegin" problem.

[Gor Nishanov via Phabricator via llvm-commits]

GorNishanov created this revision.
GorNishanov added reviewers: llvm-commits, modocache, ben-clayton, tks2103.

Fixes https://bugs.llvm.org/show_bug.cgi?id=36578.
Supersedes: https://reviews.llvm.org/D55966

One of the fundamental transformation that CoroSplit pass performs before splitting the coroutine is to find which values need to survive between suspend and resume and provide a slot for them in the coroutine frame to spill and restore the value as needed.

Coroutine frame becomes available once the storage for it was allocated and that point is marked in the pre-split coroutine with a llvm.coro.begin intrinsic.

FE normally puts all of the user-authored code that would be accessing those values after llvm.coro.begin, however, sometimes instructions accessing those values would end up prior to coro.begin. For example, writing out a value of the parameter into the alloca done by the FE or instructions that are added by the optimization passes such as SROA when it rewrites allocas.

Prior to this change, CoroSplit pass would try to move instructions that may end up accessing the values in the coroutine frame after CoroBegin. However it would run into problems (report_fatal_error) if some of the values would be used both in the allocation function (for example allocator is passed as a parameter to a coroutine) and in the use-authored body of the coroutine.

To handle this case and to simplify the instruction moving logic, this change removes all of the instruction moving. Instead, we only change the uses of the spilled values that are dominated by coro.begin and leave other instructions intact.

Before:

  %var = alloca i32
  %1 = getelementptr .. %var; ; will move this one after coro.begin
  %f = call i8* @llvm.coro.begin(

After:

  %var = alloca i32
  %1 = getelementptr .. %var; stays put
  %f = call i8* @llvm.coro.begin(

If we discover that there is a potential write into an alloca, prior to coro.begin we would copy its value from the alloca into the spill slot in the coroutine frame.

Before:

  %var = alloca i32
  store .. %var ; will move this one after coro.begin
  %f = call i8* @llvm.coro.begin(

After:

  %var = alloca i32
  store .. %var ;stays put
  %f = call i8* @llvm.coro.begin(
  %tmp = load %var
  store %tmp, %spill.slot.for.var

Note: This change does not handle array allocas as that is something that C++ FE does not produce, but, it can be added in the future if need arises


https://reviews.llvm.org/D66230

Files:
  lib/Transforms/Coroutines/CoroFrame.cpp
  lib/Transforms/Coroutines/CoroSplit.cpp
  test/Transforms/Coroutines/coro-alloc-with-param.ll
  test/Transforms/Coroutines/coro-param-copy.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66230.215156.patch
Type: text/x-patch
Size: 18449 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190814/666b860f/attachment.bin>