[PATCH] D82314: [RFC][Coroutines] Optimize the lifespan of temporary co_await object

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D82314#2109437 <https://reviews.llvm.org/D82314#2109437>, @lxfind wrote:

> In D82314#2107910 <https://reviews.llvm.org/D82314#2107910>, @junparser wrote:
>
> > Rather than doing it here, can we build await_resume call expression with MaterializedTemporaryExpr when expand the coawait expression. That's how gcc does.
>
>
> There doesn't appear to be a way to do that in Clang. It goes from the AST to IR directly, and there needs to be a MaterializedTemporaryExpr to wrap the result of co_await. Could you elaborate on how this might be done in Clang?


For a call such as:

  coro f() { awaitable a; (co_await a).g(); }

we produce an AST like:

  |   |   `-CXXMemberCallExpr <col:25, col:40> 'void'
  |   |     `-MemberExpr <col:25, col:38> '<bound member function type>' .g 0x55d38948ca98
  |   |       `-MaterializeTemporaryExpr <col:25, col:36> 'huge' xvalue
  |   |         `-ParenExpr <col:25, col:36> 'huge'
  |   |           `-CoawaitExpr <col:26, col:35> 'huge'
  |   |             |-DeclRefExpr <col:35> 'awaitable' lvalue Var 0x55d38948d4c8 'a' 'awaitable'
  |   |             |-CXXMemberCallExpr <col:35> 'bool'
  |   |             | `-MemberExpr <col:35> '<bound member function type>' .await_ready 0x55d38948cee8
  |   |             |   `-OpaqueValueExpr <col:35> 'awaitable' lvalue
  |   |             |     `-DeclRefExpr <col:35> 'awaitable' lvalue Var 0x55d38948d4c8 'a' 'awaitable'
  |   |             |-CXXMemberCallExpr <col:35> 'void'
  |   |             | |-MemberExpr <col:35> '<bound member function type>' .await_suspend 0x55d38948d298
  |   |             | | `-OpaqueValueExpr <col:35> 'awaitable' lvalue
  |   |             | |   `-DeclRefExpr <col:35> 'awaitable' lvalue Var 0x55d38948d4c8 'a' 'awaitable'
  |   |             | `-CXXConstructExpr <col:35> 'std::coroutine_handle<>':'std::experimental::coroutines_v1::coroutine_handle<void>' 'void (std::experimental::coroutines_v1::coroutine_handle<void> &&) noexcept'
  |   |             |   `-ImplicitCastExpr <col:35> 'std::experimental::coroutines_v1::coroutine_handle<void>' xvalue <DerivedToBase (coroutine_handle)>
  |   |             |     `-MaterializeTemporaryExpr <col:35> 'std::experimental::coroutines_v1::coroutine_handle<coro::promise_type>' xvalue
  |   |             |       `-CallExpr <col:35> 'std::experimental::coroutines_v1::coroutine_handle<coro::promise_type>'
  |   |             |         |-ImplicitCastExpr <col:35> 'std::experimental::coroutines_v1::coroutine_handle<coro::promise_type> (*)(void *) noexcept' <FunctionToPointerDecay>
  |   |             |         | `-DeclRefExpr <col:35> 'std::experimental::coroutines_v1::coroutine_handle<coro::promise_type> (void *) noexcept' lvalue CXXMethod 0x55d38948adb0 'from_address' 'std::experimental::coroutines_v1::coroutine_handle<coro::promise_type> (void *) noexcept'
  |   |             |         `-CallExpr <col:35> 'void *'
  |   |             |           `-ImplicitCastExpr <col:35> 'void *(*)() noexcept' <FunctionToPointerDecay>
  |   |             |             `-DeclRefExpr <col:35> 'void *() noexcept' lvalue Function 0x55d38948ef38 '__builtin_coro_frame' 'void *() noexcept'
  |   |             `-CXXBindTemporaryExpr <col:35> 'huge' (CXXTemporary 0x55d38948ffb8)
  |   |               `-CXXMemberCallExpr <col:35> 'huge'
  |   |                 `-MemberExpr <col:35> '<bound member function type>' .await_resume 0x55d38948cff8
  |   |                   `-OpaqueValueExpr <col:35> 'awaitable' lvalue
  |   |                     `-DeclRefExpr <col:35> 'awaitable' lvalue Var 0x55d38948d4c8 'a' 'awaitable'

See https://godbolt.org/z/hVn2u9.

I think the suggestion is to move the `MaterializeTemporaryExpr` from being wrapped around the  `CoawaitExpr` to being wrapped around the `.await_resume` call within it.

Unfortunately, that's not a correct change. The language rules require us to delay materializing the temporary until we see how it is used. For example, in a case such as

  g(co_await a);

... no temporary is materialized at all, and the `await_resume` call instead directly initializes the parameter slot for the function.

----

It seems to me that the same issue (of making large objects unnecessarily live across suspend points) can arise for other similar cases too. For example, consider:

  huge(co_await a).g(); // suppose `co_await a` returns something small

Here again we will start the lifetime of the `huge` temporary before the suspend point, and only initialize it after the resume. Your change won't help here, because it's not the lifetime markers of the value produced by `co_await` that are the problem.

As a result of the above, I think this is the wrong level at which to perform this optimization. Instead, I think you should consider whether we can move lifetime start markers later (and end markers earlier, for unescaped locals) as part of the coroutine splitting pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314