[coroutine] Fixes "cannot move instruction since its users are not dominated by CoroB...
authorGor Nishanov <GorNishanov@gmail.com>
Thu, 15 Aug 2019 00:48:51 +0000 (00:48 +0000)
committerGor Nishanov <GorNishanov@gmail.com>
Thu, 15 Aug 2019 00:48:51 +0000 (00:48 +0000)
commitefe00934044810fcb8d027b675845ec7123884c3
tree2e1811f81fc25e1fa81d3b0add8c69fc59978fef
parent399408a92f1dbbefeb708f718b0d8eb62dfa9f09
[coroutine] Fixes "cannot move instruction since its users are not dominated by CoroBegin" problem.

Summary:
Fixes https://bugs.llvm.org/show_bug.cgi?id=36578 and https://bugs.llvm.org/show_bug.cgi?id=36296.
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

Reviewers: llvm-commits, modocache, ben-clayton, tks2103, rjmccall

Reviewed By: modocache

Subscribers: bartdesmet

Differential Revision: https://reviews.llvm.org/D66230

llvm-svn: 368949
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/test/Transforms/Coroutines/coro-alloc-with-param.ll [new file with mode: 0644]
llvm/test/Transforms/Coroutines/coro-param-copy.ll [new file with mode: 0644]