(re-post in plain text) Moving this to cfgexpand time is simple and it can also be extended to handle scoped variables. However Jakub raised a good point about this being too late as stack space overlay is not the only way to cause trouble when the lifetime of a stack object is extended beyond the clobber stmt.
thanks, David On Tue, Jun 26, 2012 at 1:28 AM, Richard Guenther <richard.guent...@gmail.com> wrote: > On Mon, Jun 25, 2012 at 6:25 PM, Xinliang David Li <davi...@google.com> wrote: >> Are there any more concerns about this patch? If not, I'd like to check it >> in. > > No - the fact that the flag is C++ specific but in common.opt is odd enough > and -ftemp-reuse-stack sounds very very generic - which in fact it is not, > it's a no-op in C. Is there a more formal phrase for the temporary kind that > is affected? For me "temp" is synonymous to "auto" so I'd have expected > the switch to turn off stack slot sharing for > > { > int a[5]; > } > { > int a[5]; > } > > but that is not what it does. So - a little kludgy but probably more to what > I'd like it to be would be to move the option to c-family/c.opt enabled only > for C++ and Obj-C++ and export it to the middle-end via a new langhook > (the gimplifier code should be in Frontend code that lowers to GENERIC > really and the WITH_CLEANUP_EXPR code should be C++ frontend specific ...). > > Thanks, > Richard. > >> thanks, >> >> David >> >> On Fri, Jun 22, 2012 at 8:51 AM, Xinliang David Li <davi...@google.com> >> wrote: >>> On Fri, Jun 22, 2012 at 2:39 AM, Richard Guenther >>> <richard.guent...@gmail.com> wrote: >>>> On Fri, Jun 22, 2012 at 11:29 AM, Jason Merrill <ja...@redhat.com> wrote: >>>>> On 06/22/2012 01:30 AM, Richard Guenther wrote: >>>>>>> >>>>>>> What other issues? It enables more potential code motion, but on the >>>>>>> other hand, causes more conservative stack reuse. As far I can tell, >>>>>>> the handling of temporaries is added independently after the clobber >>>>>>> for scoped variables are introduced. This option can be used to >>>>>>> restore the older behavior (in handling temps). >>>>>> >>>>>> >>>>>> Well, it does not really restore the old behavior (if you mean before >>>>>> adding >>>>>> CLOBBERS, not before the single patch that might have used those for >>>>>> gimplifying WITH_CLEANUP_EXPR). You say it disables stack-slot sharing >>>>>> for those decls but it also does other things via side-effects of no >>>>>> longer >>>>>> emitting the CLOBBER. I say it's better to disable the stack-slot >>>>>> sharing. >>>>> >>>>> >>>>> The patch exactly restores the behavior of temporaries from before my >>>>> change >>>>> to add CLOBBERs for temporaries. The primary effect of that change was to >>>>> provide stack-slot sharing, but if there are other effects they are >>>>> probably >>>>> desirable as well, since the broken code depended on the old behavior. >>>> >>>> So you see it as workaround option, like -fno-strict-aliasing, rather than >>>> debugging aid? >>> >>> It can be used for both purposes -- if the violations are as pervasive >>> as strict-aliasing cases (which looks like so). >>> >>> thanks, >>> >>> David >>> >>>> >>>> Richard. >>>> >>>>> Jason