On 9/18/24 4:36 PM, Arsen Arsenović wrote:
This patch implements support for frames and promises with new-extended
alignment.

There are two kinds of alignment to worry about here:
- Promise alignment, which causes "internal" padding inside the frame
   struct.  The reason this is a problem is because the (yet to be
   formalized, but agreed upon) coroutine ABI requires us to place the

Where is this "agreed upon" ABI described? AFAICT there's nothing publicly visible. Could someone add something to https://github.com/itanium-cxx-abi/cxx-abi/issues even if it isn't well drafted yet?

   resume and destroy pointers right before the promise of a coroutine,
   so that they are at offset -sizeof(void*)B and -2*sizeof(void*)B.  To
   this end, we might need to insert padding to the start of the frame
   type in order to ensure that these pointers are aligned correctly.
- Frame alignment (either as a result of the above or some other field in
   the frame), which is the alignment of the entire finished frame type.
   This alignment currently (up to the standardization of P2014) requires
   us to over-allocate and then round the result.

Can't we go ahead with implementing P2014R2 even though the wording isn't finalized yet? The direction was unanimously approved by EWG.

In addition, this patch also alters __builtin_coro_promise, the builtin
that turns a promise pointer into a _M_fr_ptr of a coroutine handle.
This builtin took an additional alignment parameter it used to
compensate for the padding that'd happen between the resume/destroy
pointer and the promise.  This is no longer necessary, as now these
pointers directly precede the frame.

Could we add a bit of documentation of the frame struct to the comment at the top of the file? Is this the same thing that the standard refers to as "the coroutine state"?

Even just Iain's sketch from https://github.com/llvm/llvm-project/issues/58397#issuecomment-1281983337 would be an improvement over the nothing we have now.

As a result of the resume pointer no longer being at the start of the
coroutine frame, our actors and destroys are changed.  They now take
&frame->_Coro_resume_fn as the argument ("resume pointer"), and need to
adjust it to be the start of the frame before use.

The way each of the kinds of padding above are handled is as follows:

- For promise alignment, we emit extra members before the resume
   pointer: an "allocation pointer", if there's room for it, and a char[]
   of appropriate size to ensure there's no room between the resume
   pointer and the padding, and
- For frame alignment, we allocate extra memory in order to perform
   alignment on our own and store the result in the allocation pointer,
   which is after the frame if it could not be placed as part of the
   process above.

The location and necessity of the allocation pointer is dictated by
coroutine_info::allocptr_expr and coroutine_info::alloc_store_pointer,
the former provides an lvalue for the allocation pointer based on the
frame pointer, iff rounding is to be performed (nullptr otherwise), and
the latter decides whether to allocate extra space for the allocation
pointer (if stored after the frame).  This allows us flexibility to
later add support for P2014.

I don't understand the need for the allocation pointer in the first place. If we know the offset from &resume_ptr when we create the object, why don't we also know it when we destroy the object? Isn't that already what build_resume_to_frame_expr does?

Jason

Reply via email to