[PATCH] D101980: [RFC] [Coroutines] Put the contents of byval argument to the frame

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: lxfind, rjmccall, junparser.
Herald added subscribers: rriddle, hiraditya.
ChuanqiXu requested review of this revision.
Herald added subscribers: llvm-commits, stephenneuendorffer.
Herald added a project: LLVM.

This patch solves bug 48857 <https://bugs.llvm.org/show_bug.cgi?id=48857>.

>From my perspective, the root cause for problem `bug 48857` is that the arguments marked with `byval` wouldn't be put to the frame actually. Consider the following example:

  void foo() {
       A a;
       // Some works with a
       coro_func(a);
       // Other works with a
  } 

Every C++ programmer would believe there would be an copy for `a` in the frame of `coro_func`. From 9.5.4 of N4878, we can found:

  When a coroutine is invoked, after initializing its parameters (7.6.1.3), a copy is created for each coroutine
  parameter. For a parameter of type cv T, the copy is a variable of type cv T with automatic storage duration
  that is direct-initialized from an xvalue of type T referring to the parameter.

However, there would be only a pointer to type `A` in the frame. The problems comes from that LLVM would copy the `byval` value in the caller site instead of callee site. In other words, the IR for the example above would be:

  void foo() {
       %a = Alloca A
       // Some works with %a
       %a.byval-temp = Alloca A
       memcpy(%a.byval-temp, %a)
       coro_func(%a.byval-temp);
       // Other works with %a
  }

The actual type for the argument of `coro_func` would be `A*` marked with `byval`.

This patch tries to solve this by copying the byval arguments in the callee site if the callee is a coroutine. Now the semantic looks more consistent with the standard of C++. And I believe it should be good for Swift and MLIR. However, the down side for this approach is the extra copy. It is normal that this would make us worry about the performance. It shows that we lack a benchmark for coroutine again. But now, I could only argue for that it wouldn't hurt the performance much since the extra copy happens after the previous one and it should hit the cache.

BTW, I tried to modify the front end to make the coroutine special again - if the callee is a coroutine, we should copy the byval argument in the callee side instead of caller side.
However, the frontend looks complex and we would make coroutine violates the principle of LLVM again.

Test-Plan: check-llvm, check-clang


https://reviews.llvm.org/D101980

Files:
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/test/Transforms/Coroutines/coro-alloca-byval-param.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101980.343317.patch
Type: text/x-patch
Size: 4731 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210506/8d6ae7f2/attachment.bin>