Re: NRV with address taken

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Oct 16, 2014 at 11:03 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Thu, 16 Oct 2014, Richard Biener wrote:
>
>> Does this fix PR63537?
>
>
> PR63537 is already fine for me with trunk, NRV replaces ret with retval
> everywhere. It does so even if I add f(&ret); in the function with void
> f(vec*);
>
>>> I'd worry if both result and found are address taken before the pass,
>>> then
>>> trying to merge them together might mean something meant to have
>>> different
>>> addresses collapses into the same object.
>>
>>
>> I'd not worry about that.  But I think what the code tries to avoid is
>> failing
>> to adjust a use.  But I can't think of a case that isn't handled if it
>> properly
>> replaces uses in address-taking operations (and asms).
>>
>> For example it fails to walk PHI nodes where &var can appear as argument.
>>
>> Otherwise it relies on walk_gimple_op and walk_tree which should work.
>>
>> The other thing is aliasing though - if 'found' is TREE_ADDRESSABLE
>> then points-to sets may contain 'found' but they are not adjusted to
>> contain '<result>' afterwards.  Thus consider
>>
>> X a;
>> X *p = &a;
>> a.x = 1;
>> p->x = ...;
>> ... = a.x;
>> return a;
>>
>> where after replacing 'a' with '<result>' p->x will no longer alias the
>> store that now looks like <result>.x and thus we'd happily CSE
>> <result>.x across the pointer store.  Now NRV runs quite late
>> but we do preserve points-to information to RTL (and RTL expansion
>> handles stack slot sharing fine with points-to sets - but we'd need to
>> handle NRV the same here).
>
>
> Ah, ok. It would be great to paste some of this in tree-nrv.c, unless you
> think it will be too much.

I think it would be great to integrate NRV with RTL expansion instead
and thus handle the TREE_ADDRESSABLE case correct.  (simply
merge stack-slots of <retval> and 'found'!?)

Richard.

> --
> Marc Glisse