This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 03/09/2015 02:33 PM, Aldy Hernandez wrote:
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

Agreed, we want (op1, op0) in this case. I think this demonstrates that the existing code is wrong to sometimes return (op0, op1), since we might be using the result as an lvalue. Better would be to always gimplify op0 to an lvalue, gimplify_and_add the rhs, and finally return the gimplified lhs, rather than mess with building up a COMPOUND_EXPR (or STATEMENT_LIST, as in your patch).

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.

I would expect the current code to handle this fine.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]