[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

> This is an alternative to D97915 <https://reviews.llvm.org/D97915> which missed proper deallocation of the over-allocated frame. This patch handles both allocations and deallocations.

If D97915 <https://reviews.llvm.org/D97915> is not needed, we should abandon it.

For the example shows in D97915 <https://reviews.llvm.org/D97915>, it says:

  #include <experimental/coroutine>
  #include <iostream>
  #include <stdexcept>
  #include <thread>
  #include <cassert>
  
  struct task{
    struct alignas(64) promise_type {
      task get_return_object() { return {}; }
      std::experimental::suspend_never initial_suspend() { return {}; }
      std::experimental::suspend_never final_suspend() noexcept { return {}; }
      void return_void() {}
      void unhandled_exception() {}
    };
    using handle = std::experimental::coroutine_handle<promise_type>;
  };
  
  auto switch_to_new_thread() {
    struct awaitable {
      bool await_ready() { return false; }
      void await_suspend(task::handle h) {
        auto i = reinterpret_cast<std::uintptr_t>(&h.promise());
        std::cout << i << std::endl;
        assert(i % 64 == 0);
      }
      void await_resume() {}
    };
    return awaitable{};
  }
  
  task resuming_on_new_thread() {
    co_await switch_to_new_thread();
  }
  
  int main() {
    resuming_on_new_thread();
  }

The assertion would fail. If this is the root problem, I think we could adjust the align for the promise alloca like:

  %promise = alloca %promise_type, align 8

into

  %promise = alloca %promise_type, align 128

In other words, if this the problem we need to solve, I think we could make it in a simpler way.

The I looked into the document you give in the summary. The issue#3 says the frontend can't do some work in the process of template instantiation due to the frontend doesn't know about the align and size of the coroutine. But from the implementation, it looks like not the problem this patch wants to solve.

I am really confused about the problem. Could you please restate your problem more in more detail? For example, would it make the alignment incorrect like the example above? Or does we want the frontend to get alignment information? Then what would be affected? From the title, I can guess the size of frame would get bigger. But how big would it be? Who would control and determine the final size?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739