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

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

In D101980#2750311 <https://reviews.llvm.org/D101980#2750311>, @ChuanqiXu wrote:

> Thanks for the detailed explanation! I still had some questions.
>
>> An indirect argument is one that's implicitly passed by reference
>
> Do you mean `by value` here? If an argument is passed by reference, it makes no sense the coroutine need to copy the argument by value. Since the reference is pointer really. And I think it is responsibility for the programmers to maintain the lifetime for the pointer. And the programmers should agree on there is only pointer in the coroutine frame.

Implicitly by reference, i.e. the argument type is not a reference type but the ABI just passes a pointer behind the scenes.

>> For example, most x86-64 ABIs will pass a struct like struct { char x[1024]; }; as a pointer to a temporary instead of on the stack. Normally, functions that take such arguments just bind the parameter variable directly to the pointer that's passed in; if that's a problem for coroutines, you're going to need to force a copy there as well.
>
> From the statement, it looks like it is talking about passing argument by value. So I think it means there are some arguments who are passed by value in the language side but didn't be marked with `byval`. Did I get this? If it is true, I agree with that I need to find another method to handle it.

`byval` in LLVM IR has a very specific meaning: it means the argument is passed in the stack argument area.  On the caller side there's an implicit copy from the argument pointer into the argument area, and on the callee side the parameter pointer value is directly bound to the position in the argument area.  A lot of targets never use it for anything, because it's not considered beneficial for small arguments (like integers and small structs), and those targets pass large aggregates indirectly (as discussed above).

>> Indirect arguments are also used (on pretty much every target except Windows) for non-trivially-copyable class types.  That gets tricky because unfortunately you can't just emit a correct move-construction during coroutine lowering; we'll have to do it in the frontend, if we aren't already.
>
> For the case of move-construction, I think the current patch could handle it if the argument is marked with byval. For example:
>
>   coro_type foo(A); // A is move-constructible.
>   void bar() {
>       A  a;
>      // ... some operation
>      foo(std::move(A));
>   }
>
> And the corresponding IR should look like:
>
>   void bar() {
>       %a = alloca A
>      ; ... some operation
>      %tmp = alloca A
>      ; move-construct %tmp
>      foo(%tmp)
>   }
>   coro_type foo(A* [byval] %a) {
>      ; if a is marked with byval
>      %tmp.a.byval = alloca A
>      memcpy(%tmp.a.byval, %a)
>   }
>
> It looks OK to me except the extra copy. The move-construction is already handled by the frontend.

You can't assume that move construction is equivalent to a memcpy.  That's just not semantically correct.  Only the frontend has this information.

>> That brings up a related problem: there are LLVM optimizations that assume things about parameters during a function.  For example, the pointers passed as indirect arguments are generally assumed not to alias anything, which means that we can generally eliminate redundant things like, say, copies out of them that are perfectly happy to just use the original memory.  That's not a problem for copies emitted during coroutine lowering, since we'll immediately split the function and make the coroutine non-redundant from LLVM's perspective, but I am worried that we might trigger these optimizations on copies we emit earlier, and that could mess things up.
>
> I am not sure if I get your point right here. From my understanding, this paragraph and the following example tells about the problem we may met if we try to emit the copy for the argument passed by value in the callee site if the callee is a coroutine. Do I understand right? If I got it, I think the problems told here is a little bit hard to handle. I can't find solutions right now.

I am saying that there is a risk of miscompilation if LLVM applies assumptions that are valid in normal functions but aren't valid in unlowered coroutines.  Probably the right solution is just to teach LLVM that those assumptions aren't valid in unlowered coroutines.  You should probably bring it up on llvm-dev.


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

https://reviews.llvm.org/D101980