On Thu, Oct 16, 2014 at 07:37:18AM +0200, Marc Glisse wrote: > Hello, > > the attached one-liner passed bootstrap+testsuite (really all languages) on > x86_64-linux-gnu (I got an extra pass of unix/-m32: os but I assume that the > failure with trunk was random). > > The current code is a bit weird: we bail out if either result or found is > TREE_ADDRESSABLE, but then the variable replacement includes: > > TREE_ADDRESSABLE (result) |= TREE_ADDRESSABLE (found); > > (modified "recently", it was a plain assignment before) > > I mostly ran the testsuite to find a testcase showing why found should not > have its address taken, so if someone wants to add one (or at least a > comment in tree-nrv.c), that would be good.
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. > 2014-10-16 Marc Glisse <marc.gli...@inria.fr> > > * tree-nrv.c (pass_nrv::execute): Don't disable when address is taken. > > -- > Marc Glisse > Index: gcc/tree-nrv.c > =================================================================== > --- gcc/tree-nrv.c (revision 216286) > +++ gcc/tree-nrv.c (working copy) > @@ -210,21 +210,20 @@ pass_nrv::execute (function *fun) > return 0; > } > else > found = rhs; > > /* The returned value must be a local automatic variable of the > same type and alignment as the function's result. */ > if (TREE_CODE (found) != VAR_DECL > || TREE_THIS_VOLATILE (found) > || !auto_var_in_fn_p (found, current_function_decl) > - || TREE_ADDRESSABLE (found) > || DECL_ALIGN (found) > DECL_ALIGN (result) > || !useless_type_conversion_p (result_type, > TREE_TYPE (found))) > return 0; > } > else if (gimple_has_lhs (stmt)) > { > tree addr = get_base_address (gimple_get_lhs (stmt)); > /* If there's any MODIFY of component of RESULT, > then bail out. */ Jakub