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

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D100739#2700273 <https://reviews.llvm.org/D100739#2700273>, @ChuanqiXu wrote:

> I hadn't looked into the details. I would try to make it.
> But from my understanding to this problem, the correct solution should remain the previous behavior if the end user doesn't specify `alignas` for `promise_type`. I mean, it shouldn't make so many test cases fail.

The failures mostly are due to the `llvm.coro.size()` -> `llvm.coro.size(i1 alloc)` intrinsic signature change. The rest are due to the `align` arg of `llvm.coro.id` is illegal 0 in most of the tests. This patch would read that value causing asssertions.

>> The alloca of the raw frame pointer (suppose we insert it in the frontend) would be included in the non-overaligned frame if we don't teach CoroFrame how to elide it.
>
> It is confusing to me, maybe I need to look into the codes. First, the raw frame pointer isn't inserted in the frontend. Do you mean coro.begin? Then, what's the relationship between eliding and over-aligned? It also makes me confused.

Since we don't know if the frame is overaligned or not until after the frame type is decided in CoroFrame, *suppose* we do this (adding raw frame ptr alloca in frontend regardless of the presence of overalignment or not) in the frontend, the code generated from CGCoroutine would be like this:

  %1 = alloc i8*
  %2 = malloc(x)
  store %2, %1
  ..
  ..
  %11 = load %1
  free(%11)

The alloca `%1` must be there even if the frame is not aligned. Then in the CoroFrame, we have to check if the frame is really overaligned and if not, reverse the above patterns. Otherwise, the alloca `%1` would be in the frame needlessly and the following optimizations could not reliably remove it.

This patch defers adding the alloca until CoroFrame where we know for sure that the frame is aligned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739