Hi JunMa, JunMa <ju...@linux.alibaba.com> wrote:
> 在 2020/1/20 下午6:07, Iain Sandoe 写道: >> Hi JunMa, >> >> JunMa <ju...@linux.alibaba.com> wrote: >> >>> Hi >>> Accroding to N4835: When a coroutine is invoked, a copy is created >>> for each coroutine parameter. While in current implementation, we >>> only collect used parameters. This may lost behavior inside constructor >>> and destructor of unused parameters. >>> >>> This patch change the implementation to collect all parameter. >> >> thanks for the patch. >> >> I am not convinced this is the right way to fix this, we do not want to >> increase >> the size of the coroutine frame unnecessarily. I would like to check the >> lifetime >> requirements; such copies might only need to be made local to the ramp >> function. >> >> Iain > For type with trivial destructor, There is no need to copy parameter if it is > never referenced after a reachable suspend point(return-to-caller/resume) in > the > coroutine. Since we are in front end, that should be inital_suspend point. so > we > can create field for type with non-trivial destructor first, and skip unused > parameters > in register_param_uses. I'll update the patch I think that, even if the DTOR is non-trivial, the copy only needs to be in the stack frame of the ramp, not in the coroutine frame. This is also what clang does for -O>0 (although it makes a frame copy at O0). Will clarify this (perhaps the wording could be slightly more specfic). thanks Iain > > Regards > JunMa >>> Bootstrap and test on X86_64, is it OK? >>> >>> Regards >>> JunMa >>> >>> gcc/cp >>> 2020-01-20 Jun Ma <ju...@linux.alibaba.com> >>> >>> * coroutines.cc (build_actor_fn): Skip rewrite when there is no >>> param references. >>> (register_param_uses): Only collect param references here. >>> (morph_fn_to_coro): Create coroutine frame field for each >>> function params. >>> >>> gcc/testsuite >>> 2020-01-20 Jun Ma <ju...@linux.alibaba.com> >>> >>> * g++.dg/coroutines/torture/func-params-07.C: New test. >>> <0001-Fix-issue-when-copy-function-parameters-into-corouti.patch>
