On Thu, Oct 16, 2014 at 11:03 AM, Marc Glisse <marc.gli...@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
