On 12/10/24 10:58 AM, Arsen Arsenović wrote:
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.
Soon, I hope.
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?
It can supersede, this is experimental code anyway.
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.
Ah, yes, please also move it to the top of the file.
On 12/10/24 11:26 AM, Iain Sandoe wrote:
I think it is useful to fix the under-aligned frames (using the existing
ABI for the builtin and the placement of the padding). That brings
actual immediate benefit.
It islikely that the process of deciding if the combined set of ‘vendors’
still want to adopt the ‘padding before’ change will take some time.
(and I’d not be surprised if the end decision was to stick with what we
have for Itanium).
Makes sense.
Jason
