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).
Fair enough.
I had to tweak the condition a bit, as the returns I'm seeing contain a
COMPOUND_EXPR. I also unfortunately had to leave that ugly special case
for TREE_THIS_VOLATILE, as the comment is correct :), gimplify_expr goes
into an infinite loop gimplifying a volatile temporary.
Finally, I am using a goto instead of recursing, as the case for
INIT_EXPR degrades INIT_EXPRs containing AGGR_INIT_EXPRs incorrectly.
The attached patch was tested on x86-64 Linux. How does it look?
Aldy
commit 451ffe1998bd26175677fe62e9449313da642fbe
Author: Aldy Hernandez <al...@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.
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 4233a64..115cbaa 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -528,6 +528,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 +620,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 +640,20 @@ 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);
+ if (TREE_SIDE_EFFECTS (op1))
+ {
+ /* Drop volatile variables on the RHS to avoid
+ infinite recursion from gimplify_expr trying to
+ load the value. */
+ 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 = op0;
}
}
ret = GS_OK;
@@ -740,6 +753,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" } } */
