[PATCH] D157070: [C++20] [Coroutines] Introduce `@llvm.coro.opt.blocker` to block the may-be-incorrect optimization for awaiter

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D157070#4573825 <https://reviews.llvm.org/D157070#4573825>, @rjmccall wrote:

> Let me try to rephrase to see if I understand the problem here.  In the coroutine function, the frontend allocates the awaiter object as a temporary like any other, i.e. with just an `alloca`.  The optimizer doesn't see any escapes of that `alloca`, so it thinks it can freely optimize accesses to it.  But in fact the awaiter object does escape and can be accessed separately somehow, so the proposed fix is just to emit a call which looks like an escape of the pointer.

Yes. Exactly.

> I don't really understand how the awaiter object escapes here — how is it possible to access it given just a `std::coroutine_handle`?

The `std::coroutine_handle` refers to the coroutine frame, which is a **semi** opaque object. The language doesn't allow programmers to access the local variables from the coroutine frame directly. But the language allows the programmer to resume/destroy the corresponding coroutine. So that the programmer can access the local variables by the coroutine handle **indirectly**.

For this example,

  C++
  std::coroutine_handle<> await_suspend(const std::coroutine_handle<> h) {
      // Assume we will suspend unless proven otherwise below. We must do
      // this *before* calling Register, since we may be destroyed by another
      // thread asynchronously as soon as we have registered.
      suspended = true;
  
      // Attempt to hand off responsibility for resuming/destroying the coroutine.
      const auto to_resume = awaitable.Register(h);
  
      if (!to_resume) {
        // The awaitable is already ready. In this case we know that Register didn't
        // hand off responsibility for the coroutine. So record the fact that we didn't
        // actually suspend, and tell the compiler to resume us inline.
        suspended = false;
        return h;
      }
  
      // Resume whatever Register wants us to resume.
      return to_resume;
    }

The problem is that the call `awaitable.Register(h);` may destroy the coroutine referred by `h` then it is problematic to access the local variable `suspended` after that. The source code is good since it only access the local variable conditionally. But the optimizer ignores the fact that the local variable may be alias with the coroutine handle. So the compiler optimize the conditional access to unconditional access. So problem happens.

---

Then above explanation tells the reason why this is only matters with awaiter. Since the language only allows the programmer to access the coroutine handle from `await_suspend` which is member function of an awaiter. So it is sufficient to take care of awaiter specially to me.


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

https://reviews.llvm.org/D157070