Jason Merrill <ja...@redhat.com> writes: > 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?
Iain answered this bit - I trust he'll post that issue eventually. >> 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. This is OK by me (I prefer P2014 myself) - I am not sure when I'd get time to implement it since we have an extended work week this Sunday, but in either case, would it need to wait for the next stage 1 or should it supersede this patch? >> 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. There's one in the source code right now that I updated at coroutines.cc ~L5550. It can be made prettier and more informative. I can do so. >> 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 WRT the question of Clang compatibility: the layout of the frame is changed when internal padding is present (i.e. when the promise type has an alignment greater than 2 * sizeof(void*)), the padding used to be present between the resume/destroy pointers and promise: struct frame_type { void (*resume)(frame_type*); } -- Arsen Arsenović
signature.asc
Description: PGP signature
