[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added a comment.

In D97915#2605398 <https://reviews.llvm.org/D97915#2605398>, @ChuanqiXu wrote:

> I am a little confusing about the problem. The example in the link tells the align of the `promise` instead of the `frame`. The address of `promise` and `frame` is not same. It looks like you're trying to do:
>
>   +               +-----------------------------------+
>   |               |                                   |
>   +---------------+          frame                    |
>   | pedding       |                                   |
>   +               +-----------------------------------+
>                   ^
>                   |
>                   |
>                   |
>                   |
>                   |
>                   +
>   
>                 The address of frame matches the offset of promise.
>
> However, what we should do is:
>
>   +               +-----------------------------------+
>   |               |       +--------------+            |
>   +---------------+frame  | promise      |            |
>   | pedding       |       <--------------+            |
>   +               +-----------------------------------+
>                   ^       |
>                   |       |
>                   |       |
>                   |       |
>                   |       +
>                   |       This is what we really want
>                   +
>   
>                 The address of frame matches the offset of promise.
>
> If I get the problem problems, I think we can handle this problem in the middle end if the information for the promise remains.

Not sure I follow. Inside the frame, the promise is in its desired position. It is not properly aligned because the frame start address is underaligned - `malloc` usually only returns 16 bytes aligned memory whereas `alignas` could make the preferred alignment larger than that.



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16756
 };
-} // namespace
 
----------------
ChuanqiXu wrote:
> Why we remove the anonymous namespace here?
I added a common/helper function that takes `BuiltinAlignArgs` as an argument. Need to move it out of the anonymous namespace to forward declare it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915