On 03/05/2015 10:20 PM, Jason Merrill wrote:
On 03/05/2015 06:25 PM, Aldy Hernandez wrote:
+    tree ret = TREE_OPERAND (*expr_p, 0);
+    if (ret && (TREE_CODE (ret) == INIT_EXPR
+            || TREE_CODE (ret) == MODIFY_EXPR)
+        && TREE_CODE (TREE_OPERAND (ret, 0)) == RESULT_DECL
+        && is_gimple_lvalue (TREE_OPERAND (ret, 0))
+        && is_really_empty_class (TREE_TYPE (TREE_OPERAND (ret, 0))))
+      {
+        tree result_decl = TREE_OPERAND (ret, 0);
+        tree list = alloc_stmt_list ();
+        append_to_statement_list (TREE_OPERAND (ret, 1), &list);
+        append_to_statement_list (build1 (RETURN_EXPR, void_type_node,
+                          result_decl), &list);
+        *expr_p = list;
+        return GS_OK;
+      }

This should really use the MODIFY_EXPR case rather than duplicate it
here.  Actually, why don't we already hit that case when processing the
RETURN_EXPR?

We are hitting the MODIFY_EXPR case. Indeed, _because_ we hit the MODIFY_EXPR is that we return the uninitialized temporary.

For example, we start with:

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

which becomes:

        <stuff with &D.2347>
        return D.2349 = D.2347;

which the aforementioned MODIFY_EXPR case turns into:

        <stuff with &D.2347>
        return D.2349;

My patch was trying to notice this pattern and get rid of the temporary, thereby generating:

        <stuff with &D.2347>
        return retval;

If you still think it's profitable, I can come up with an alternative using the MODIFY_EXPR code ??.

Aldy

Reply via email to