JunMa <ju...@linux.alibaba.com> wrote: > 在 2020/1/20 下午8:21, Iain Sandoe 写道: >> JunMa <ju...@linux.alibaba.com> wrote: >> >>> 在 2020/1/20 下午7:55, Iain Sandoe 写道: >>>> 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. >>> I do think this. It's just need more work on front end. >> >> I think we already did the work, and know the answer (about param uses in >> the body), right? > Yes, we may need extra work on copy parameters, I'll do this.
Having discussed this with Nathan (and I’ve also mailed Gor for clarification). I think it might be a good idea to wait for those responses before revising (it could be that your original reading of the wording is correct, and the clang impl. needs to be updated). thanks Iain >>>> 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). >>> No, clang build frame in middle end. In FE, it just generate local variable >>> to do the copy, and let the middle end to >>> do the optimization which can see more opportunity. >> >> Yes, i understand that clang does this in the ME not in the FE but, so long >> as the principle is correct, it is equivalent for GCC to do this in the FE. > Yes, just less situation we can handle, that's why we still need coroutine > frame reduction pass. > > Regards > JunMa >> thanks >> Iain >> >>> Regards >>> JunMa >>>> 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>
