Comment? David
On Mon, Jul 2, 2012 at 4:30 PM, Xinliang David Li <davi...@google.com> wrote: > I extended the patch a little so that the option can be used to set > multiple stack reuse levels: -fstack-reuse=[all|name_vars|none] > > all: enable stack reuse for all local vars (named vars and compiler > generated temporaries) which live in memory; > name_vars: enable stack reuse only for user declared local vars with names; > none: disable stack reuse completely. > > Note the patch still chooses to suppress clobber statement generation > instead of just ignoring them in stack layout. This has the additional > advantage of allowing more aggressive code motion when stack use is > disabled. > > The documentation will be updated when the patch is agreed upon. > > thanks, > > David > > > On Thu, Jun 28, 2012 at 10:43 PM, Xinliang David Li <davi...@google.com> > wrote: >> (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