Optimize empty class copies within a C++ return statement
diff mbox

Message ID 54FF9E29.8030704@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez March 11, 2015, 1:45 a.m. UTC
On 03/10/2015 05:55 PM, Jason Merrill wrote:
> On 03/10/2015 08:07 PM, Aldy Hernandez wrote:
>> On 03/10/2015 04:59 PM, Jason Merrill wrote:
>>> On 03/10/2015 07:10 PM, Aldy Hernandez wrote:
>>>> -        /* Remove any copies of empty classes.  We check that the RHS
>>>> -           has a simple form so that TARGET_EXPRs and non-empty
>>>> -           CONSTRUCTORs get reduced properly, and we leave the return
>>>> -           slot optimization alone because it isn't a copy (FIXME
>>>> so it
>>>> -           shouldn't be represented as one).
>>
>> The comment is now in the abstracted function simple_empty_class_p
>> header.  Although, I did remove the "remove any copies of empty classes"
>> bit, as well as the FIXME.  I can put both of those parts back in if
>> desired.  Presumably, the "remove any copies" back where it was, and the
>> FIXME in the abstracted function?
>
> Sounds good.
>
> Jason
>
>

I would've thought the actual toplevel gimplification (from 
gimplify_expr) would've handled the LHS side-effects, but I've done as 
you requested.

How does this look?

Aldy
commit 2c819732ff6cd611c05082f8522a1e6aa452dc65
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Mar 5 12:23:27 2015 -0800

    	* cp-gimplify.c (simple_empty_class_p): New.
    	* cp-gimplify.c (cp_gimplify_expr): Handle RETURN_EXPR.  Abstract
    	the code for empty class copies into simple_empty_class_p, and
    	adapt it to handle COMPOUND_EXPRs.

Comments

Jason Merrill March 11, 2015, 12:06 p.m. UTC | #1
On 03/10/2015 09:45 PM, Aldy Hernandez wrote:
> +	    gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
...
> +	    *expr_p = TREE_OPERAND (*expr_p, 0);

Can't you use op0 here, since you're about to throw away the MODIFY_EXPR?

OK either way.

Jason

Patch
diff mbox

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 4233a64..70645b5 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -53,6 +53,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "target.h"
 #include "c-family/c-ubsan.h"
 #include "cilk.h"
+#include "gimplify.h"
+#include "gimple-expr.h"
 
 /* Forward declarations.  */
 
@@ -528,6 +530,29 @@  gimplify_must_not_throw_expr (tree *expr_p, gimple_seq *pre_p)
   return GS_ALL_DONE;
 }
 
+/* Return TRUE if an operand (OP) of a given TYPE being copied is
+   really just an empty class copy.
+
+   Check that the operand has a simple form so that TARGET_EXPRs and
+   non-empty CONSTRUCTORs get reduced properly, and we leave the
+   return slot optimization alone because it isn't a copy.  */
+
+static bool
+simple_empty_class_p (tree type, tree op)
+{
+  return
+    ((TREE_CODE (op) == COMPOUND_EXPR
+      && simple_empty_class_p (type, TREE_OPERAND (op, 1)))
+     || is_gimple_lvalue (op)
+     || INDIRECT_REF_P (op)
+     || (TREE_CODE (op) == CONSTRUCTOR
+	 && CONSTRUCTOR_NELTS (op) == 0
+	 && !TREE_CLOBBER_P (op))
+     || (TREE_CODE (op) == CALL_EXPR
+	 && !CALL_EXPR_RETURN_SLOT_OPT (op)))
+    && is_really_empty_class (type);
+}
+
 /* Do C++-specific gimplification.  Args are as for gimplify_expr.  */
 
 int
@@ -597,6 +622,7 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	return GS_OK;
       /* Otherwise fall through.  */
     case MODIFY_EXPR:
+    modify_expr_case:
       {
 	if (fn_contains_cilk_spawn_p (cfun)
 	    && cilk_detect_spawn_and_unwrap (expr_p)
@@ -616,31 +642,22 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	  TREE_OPERAND (*expr_p, 1) = build1 (VIEW_CONVERT_EXPR,
 					      TREE_TYPE (op0), op1);
 
-	else if ((is_gimple_lvalue (op1) || INDIRECT_REF_P (op1)
-		  || (TREE_CODE (op1) == CONSTRUCTOR
-		      && CONSTRUCTOR_NELTS (op1) == 0
-		      && !TREE_CLOBBER_P (op1))
-		  || (TREE_CODE (op1) == CALL_EXPR
-		      && !CALL_EXPR_RETURN_SLOT_OPT (op1)))
-		 && is_really_empty_class (TREE_TYPE (op0)))
+	else if (simple_empty_class_p (TREE_TYPE (op0), op1))
 	  {
-	    /* Remove any copies of empty classes.  We check that the RHS
-	       has a simple form so that TARGET_EXPRs and non-empty
-	       CONSTRUCTORs get reduced properly, and we leave the return
-	       slot optimization alone because it isn't a copy (FIXME so it
-	       shouldn't be represented as one).
-
-	       Also drop volatile variables on the RHS to avoid infinite
-	       recursion from gimplify_expr trying to load the value.  */
-	    if (!TREE_SIDE_EFFECTS (op1))
-	      *expr_p = op0;
-	    else if (TREE_THIS_VOLATILE (op1)
-		     && (REFERENCE_CLASS_P (op1) || DECL_P (op1)))
-	      *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
-				build_fold_addr_expr (op1), op0);
-	    else
-	      *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
-				op0, op1);
+	    /* Remove any copies of empty classes.  Also drop volatile
+	       variables on the RHS to avoid infinite recursion from
+	       gimplify_expr trying to load the value.  */
+	    gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+			   is_gimple_lvalue, fb_lvalue);
+	    if (TREE_SIDE_EFFECTS (op1))
+	      {
+		if (TREE_THIS_VOLATILE (op1)
+		    && (REFERENCE_CLASS_P (op1) || DECL_P (op1)))
+		  op1 = build_fold_addr_expr (op1);
+
+		gimplify_and_add (op1, pre_p);
+	      }
+	    *expr_p = TREE_OPERAND (*expr_p, 0);
 	  }
       }
       ret = GS_OK;
@@ -740,6 +757,19 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	}
       break;
 
+    case RETURN_EXPR:
+      if (TREE_OPERAND (*expr_p, 0)
+	  && (TREE_CODE (TREE_OPERAND (*expr_p, 0)) == INIT_EXPR
+	      || TREE_CODE (TREE_OPERAND (*expr_p, 0)) == MODIFY_EXPR))
+	{
+	  expr_p = &TREE_OPERAND (*expr_p, 0);
+	  code = TREE_CODE (*expr_p);
+	  /* Avoid going through the INIT_EXPR case, which can
+	     degrade INIT_EXPRs into AGGR_INIT_EXPRs.  */
+	  goto modify_expr_case;
+	}
+      /* Fall through.  */
+
     default:
       ret = (enum gimplify_status) c_gimplify_expr (expr_p, pre_p, post_p);
       break;
diff --git a/gcc/testsuite/g++.dg/other/empty-class.C b/gcc/testsuite/g++.dg/other/empty-class.C
new file mode 100644
index 0000000..a14c437
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/empty-class.C
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-gimple" } */
+
+/* Test that we return retval directly, instead of going through an
+   intermediate temporary, when returning an empty class.  */
+
+class obj {
+  public:
+   obj(int);
+};
+
+obj funky(){
+    return obj(555);
+}
+
+/* { dg-final { scan-tree-dump-times "return <retval>;" 1 "gimple" } } */
+/* { dg-final { cleanup-tree-dump "gimple" } } */