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

Reply via email to