Re: [PATCH] c++: coroutines - Overlap variables in frame [PR105989]

["Michal Jankovič" <michal.jankovic59@gmail.com>]

Hi Iain,

Thanks for the reply, this is my first time contributing and I am
looking forward to your input.

One other related thing I would like to investigate is reducing the
number of compiler generated variables in the frame, particularly
_Coro_destroy_fn and _Coro_self_handle.  

As I understand it, _Coro_destroy_fn just sets a flag in
_Coro_resume_index and calls _Coro_resume_fn; it should be possible to
move this logic to __builtin_coro_destroy, so that only _Coro_resume_fn
is stored in the frame; this would however change the coroutine ABI - I
don't know if that's a problem.

The _Coro_self_handle should be constructible on-demand from the frame address.

Do you have any advice / opinions on this before I try to implement it?

Michal

On Jul 12 2022, at 4:08 pm, Iain Sandoe <iain@sandoe.co.uk> wrote:

> Hi Michal,
>  
>> On 12 Jul 2022, at 14:35, Michal Jankovič via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>  
>> Currently, coroutine frames store all variables of a coroutine separately,
>> even if their lifetime does not overlap (they are in distinct
>> scopes). This
>> patch implements overlapping distinct variable scopes in the
>> coroutine frame,
>> by storing the frame fields in nested unions of structs. This lowers
>> the size
>> of the frame for larger coroutines significantly, and makes them more usable
>> on systems with limited memory.
>  
> not a review (I will try to take a look at the weekend).
>  
> but … this is one of the two main optimisations on my TODO - so cool
> for doing it.
>  
> (the other related optimisation is to eliminate frame entries for
> scopes without any suspend
> points - which has the potential to save even more space for code with
> sparse use of co_xxxx)
>  
> Iain
>  
>> Bootstrapped and regression tested on x86_64-pc-linux-gnu; new test fails
>> before the patch and succeeds after with no regressions.
>>  
>> 	PR c++/105989
>>  
>> gcc/cp/ChangeLog:
>>  
>> 	* coroutines.cc (struct local_var_info): Add field_access_path.
>> 	(build_local_var_frame_access_expr): New.
>> 	(transform_local_var_uses): Use build_local_var_frame_access_expr.
>> 	(coro_make_frame_entry_id): New.
>> 	(coro_make_frame_entry): Delegate to coro_make_frame_entry_id.
>> 	(struct local_vars_frame_data): Add orig, field_access_path.
>> 	(register_local_var_uses): Generate new frame layout. Create access
>> 	paths to vars.
>> 	(morph_fn_to_coro): Set new fields in local_vars_frame_data.  
>>  
>> gcc/testsuite/ChangeLog:
>>  
>> 	* g++.dg/coroutines/pr105989.C: New test.
>>  
>> <pr105989.patch>
>  
>