Re: New option to turn off stack reuse for temporaries

[Xinliang David Li <davidxl at google dot com>]

Comment?

David

On Mon, Jul 2, 2012 at 4:30 PM, Xinliang David Li <davidxl@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 <davidxl@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.guenther@gmail.com> wrote:
>>> On Mon, Jun 25, 2012 at 6:25 PM, Xinliang David Li <davidxl@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 <davidxl@google.com> wrote:
>>>>> On Fri, Jun 22, 2012 at 2:39 AM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Fri, Jun 22, 2012 at 11:29 AM, Jason Merrill <jason@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