Re: [patch] Optimize empty class copies within a C++ return statement

[Aldy Hernandez <aldyh at redhat dot com>]

On 03/06/2015 02:01 PM, Jason Merrill wrote:
> On 03/06/2015 05:01 PM, Jason Merrill wrote:
> > On 03/06/2015 04:54 PM, Aldy Hernandez wrote:
> > > > But doesn't this still involve a MODIFY_EXPR, i.e. return retval =
> > > > D.2349?
> > >
> > > If I understand you correct, no.
> > >
> > > gimplify_return_expr creates a new temporary and uses that instead of
> > > <retval>:
> > >
> > >    else if (gimplify_ctxp->return_temp)
> > >      result = gimplify_ctxp->return_temp;
> > >    else
> > >      {
> > >        result = create_tmp_reg (TREE_TYPE (result_decl));
> > >        ...
> > >      }
> > > ...
> > > ...
> > >    /* Smash the lhs of the MODIFY_EXPR to the temporary we plan to use.
> > >       Then gimplify the whole thing.  */
> > >    if (result != result_decl)
> > >      TREE_OPERAND (ret_expr, 0) = result;
> >
> > Sounds like ret_expr is a MODIFY_EXPR.
>
> Oh, but with the wrong lhs, I see.

I know you want to reuse the MODIFY_EXPR case in cp_gimplify_expr, but 
after playing around with it, I think it requires too much special 
casing to make it clean.

For instance, the MODIFY_EXPR case returns the RHS of expression which 
is the opposite of what we want.  For this:

	return retval = <obj>

...the MODIFY_EXPR case would build a COMPOUND_EXPR with "return 
<<<retval, <obj>>>>", which would return <obj>, not retval.  And what we 
probably want is a statement list with:

	<evaluation of obj>
	return retval

Also, the actual case we're dealing with here is a  bit more 
complicated, as it involves a COMPOUND_EXPR in the RHS, which we'd have 
to adapt MODIFY_EXPR to handle:

	return retval = <<<TARGET_EXPR, D.9999>>

IMHO, adding a special case for all this is a lot messier than what I 
originally suggested.

What do you think?

Aldy