This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RFC: PATCH for 19317
- From: Jason Merrill <jason at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Sat, 18 Jun 2005 02:38:16 -0400
- Subject: RFC: PATCH for 19317
Here's a patch to fix 19317 while preserving the optimization in safe
cases. The trouble with my earlier patches led me to the conclusion that
trying to add the return slot address to the argument list was a mistake;
it requires too much special handling, and obscures the assignment
semantics. This patch changes the optimization to just use a flag on the
CALL_EXPR.
When the target is not addressable, the optimization is obviously safe. If
it is addressable, we need to wait until after alias analysis to decide so
that we know whether or not the address escapes. So I've added a small
optimization pass to scan the trees for relevant CALL_EXPRs and set the
flag appropriately.
In my testing, the bootstrap with the patch took about 0.7% longer. Would
you expect this to cause that much slowdown? Another possible approach
would be to build a list of relevant calls during gimplification and walk
that instead of the function trees. Or add this check to another
optimization pass. Thoughts?
2005-06-14 Jason Merrill <jason@redhat.com>
PR c++/19317
Leave the return slot target in the MODIFY_EXPR rather than making
it an argument, but only use it if the CALL_EXPR has a flag set.
* tree.h (CALL_EXPR_HAS_RETURN_SLOT_ADDR): Rename to
CALL_EXPR_RETURN_SLOT_OPT.
* calls.c (expand_call): Adjust.
* tree-inline.c (expand_call_inline): Adjust.
* tree-pretty-print.c (dump_generic_node): Adjust.
And set the flag as appropriate.
* gimplify.c (gimplify_modify_expr_rhs): Set
CALL_EXPR_HAS_RETURN_SLOT_ADDR where the LHS is obviously safe.
* tree-nrv.c (execute_return_slot_opt): Set
CALL_EXPR_HAS_RETURN_SLOT_ADDR based on escape analysis.
* cp/semantics.c (simplify_aggr_init_expr): Use
CALL_EXPR_RETURN_SLOT_OPT, not CALL_EXPR_HAS_RETURN_SLOT_ADDR.
*** ./calls.c.~1~ 2005-06-14 10:39:11.418117391 -0400
--- ./calls.c 2005-05-18 18:44:19.000000000 -0400
*************** expand_call (tree exp, rtx target, int i
*** 1964,1980 ****
{
struct_value_size = int_size_in_bytes (TREE_TYPE (exp));
! if (CALL_EXPR_HAS_RETURN_SLOT_ADDR (exp))
! {
! /* The structure value address arg is already in actparms.
! Pull it out. It might be nice to just leave it there, but
! we need to set structure_value_addr. */
! tree return_arg = TREE_VALUE (actparms);
! actparms = TREE_CHAIN (actparms);
! structure_value_addr = expand_expr (return_arg, NULL_RTX,
! VOIDmode, EXPAND_NORMAL);
! }
! else if (target && MEM_P (target))
structure_value_addr = XEXP (target, 0);
else
{
--- 1964,1970 ----
{
struct_value_size = int_size_in_bytes (TREE_TYPE (exp));
! if (target && MEM_P (target) && CALL_EXPR_RETURN_SLOT_OPT (exp))
structure_value_addr = XEXP (target, 0);
else
{
*** ./cp/semantics.c.~1~ 2005-06-14 10:39:11.466108341 -0400
--- ./cp/semantics.c 2005-06-11 15:51:25.000000000 -0400
*************** simplify_aggr_init_expr (tree *tp)
*** 2904,2930 ****
style = arg;
}
! if (style == ctor || style == arg)
{
! /* Pass the address of the slot. If this is a constructor, we
! replace the first argument; otherwise, we tack on a new one. */
tree addr;
! if (style == ctor)
! args = TREE_CHAIN (args);
!
cxx_mark_addressable (slot);
addr = build1 (ADDR_EXPR, build_pointer_type (type), slot);
- if (style == arg)
- {
- /* The return type might have different cv-quals from the slot. */
- tree fntype = TREE_TYPE (TREE_TYPE (fn));
-
- gcc_assert (TREE_CODE (fntype) == FUNCTION_TYPE
- || TREE_CODE (fntype) == METHOD_TYPE);
- addr = convert (build_pointer_type (TREE_TYPE (fntype)), addr);
- }
-
args = tree_cons (NULL_TREE, addr, args);
}
--- 2904,2918 ----
style = arg;
}
! if (style == ctor)
{
! /* Replace the first argument to the ctor with the address of the
! slot. */
tree addr;
! args = TREE_CHAIN (args);
cxx_mark_addressable (slot);
addr = build1 (ADDR_EXPR, build_pointer_type (type), slot);
args = tree_cons (NULL_TREE, addr, args);
}
*************** simplify_aggr_init_expr (tree *tp)
*** 2933,2941 ****
fn, args, NULL_TREE);
if (style == arg)
! /* Tell the backend that we've added our return slot to the argument
! list. */
! CALL_EXPR_HAS_RETURN_SLOT_ADDR (call_expr) = 1;
else if (style == pcc)
{
/* If we're using the non-reentrant PCC calling convention, then we
--- 2921,2933 ----
fn, args, NULL_TREE);
if (style == arg)
! {
! /* Just mark it addressable here, and leave the rest to
! expand_call{,_inline}. */
! cxx_mark_addressable (slot);
! CALL_EXPR_RETURN_SLOT_OPT (call_expr) = true;
! call_expr = build2 (MODIFY_EXPR, TREE_TYPE (call_expr), slot, call_expr);
! }
else if (style == pcc)
{
/* If we're using the non-reentrant PCC calling convention, then we
*************** simplify_aggr_init_expr (tree *tp)
*** 2945,2950 ****
--- 2937,2943 ----
call_expr = build_aggr_init (slot, call_expr,
DIRECT_BIND | LOOKUP_ONLYCONVERTING);
pop_deferring_access_checks ();
+ call_expr = build (COMPOUND_EXPR, TREE_TYPE (slot), call_expr, slot);
}
*tp = call_expr;
*** ./tree.h.~1~ 2005-06-14 10:39:11.448111735 -0400
--- ./tree.h 2005-06-11 15:51:14.000000000 -0400
*************** struct tree_common GTY(())
*** 337,343 ****
TREE_PRIVATE in
..._DECL
! CALL_EXPR_HAS_RETURN_SLOT_ADDR in
CALL_EXPR
DECL_BY_REFERENCE in
PARM_DECL, RESULT_DECL
--- 337,343 ----
TREE_PRIVATE in
..._DECL
! CALL_EXPR_RETURN_SLOT_OPT in
CALL_EXPR
DECL_BY_REFERENCE in
PARM_DECL, RESULT_DECL
*************** extern void tree_operand_check_failed (i
*** 980,988 ****
an exception. In a CALL_EXPR, nonzero means the call cannot throw. */
#define TREE_NOTHROW(NODE) ((NODE)->common.nothrow_flag)
! /* In a CALL_EXPR, means that the address of the return slot is part of the
! argument list. */
! #define CALL_EXPR_HAS_RETURN_SLOT_ADDR(NODE) ((NODE)->common.private_flag)
/* In a RESULT_DECL or PARM_DECL, means that it is passed by invisible
reference (and the TREE_TYPE is a pointer to the true type). */
--- 980,988 ----
an exception. In a CALL_EXPR, nonzero means the call cannot throw. */
#define TREE_NOTHROW(NODE) ((NODE)->common.nothrow_flag)
! /* In a CALL_EXPR, means that it's safe to use the target of the call
! expansion as the return slot for a call that returns in memory. */
! #define CALL_EXPR_RETURN_SLOT_OPT(NODE) ((NODE)->common.private_flag)
/* In a RESULT_DECL or PARM_DECL, means that it is passed by invisible
reference (and the TREE_TYPE is a pointer to the true type). */
*** ./gimplify.c.~1~ 2005-06-14 10:39:11.428115506 -0400
--- ./gimplify.c 2005-06-11 18:36:05.000000000 -0400
*************** gimplify_modify_expr_rhs (tree *expr_p,
*** 3015,3020 ****
--- 3015,3060 ----
ret = GS_UNHANDLED;
break;
+ case CALL_EXPR:
+ /* For calls that return in memory, give *to_p as the CALL_EXPR's
+ return slot so that we don't generate a temporary. */
+ if (!CALL_EXPR_RETURN_SLOT_OPT (*from_p)
+ && aggregate_value_p (*from_p, *from_p))
+ {
+ bool use_target;
+
+ if (TREE_CODE (*to_p) == RESULT_DECL
+ && needs_to_live_in_memory (*to_p))
+ /* It's always OK to use the return slot directly. */
+ use_target = true;
+ else if (!is_gimple_non_addressable (*to_p))
+ /* Don't use the original target if it's already addressable;
+ if its address escapes, and the called function uses the
+ NRV optimization, a conforming program could see *to_p
+ change before the called function returns; see c++/19317.
+ When optimizing, the return_slot pass marks more functions
+ as safe after we have escape info. */
+ use_target = false;
+ else if (DECL_GIMPLE_FORMAL_TEMP_P (*to_p))
+ /* Don't use the original target if it's a formal temp; we
+ don't want to take their addresses. */
+ use_target = false;
+ else if (is_gimple_reg_type (TREE_TYPE (*to_p)))
+ /* Also don't force regs into memory. */
+ use_target = false;
+ else
+ use_target = true;
+
+ if (use_target)
+ {
+ CALL_EXPR_RETURN_SLOT_OPT (*from_p) = 1;
+ lang_hooks.mark_addressable (*to_p);
+ }
+ }
+
+ ret = GS_UNHANDLED;
+ break;
+
default:
ret = GS_UNHANDLED;
break;
*** ./tree-nrv.c.~1~ 2005-06-14 10:39:11.435114186 -0400
--- ./tree-nrv.c 2005-06-13 11:21:37.000000000 -0400
*************** struct tree_opt_pass pass_nrv =
*** 218,220 ****
--- 218,296 ----
TODO_dump_func | TODO_ggc_collect, /* todo_flags_finish */
0 /* letter */
};
+
+ /* Walk through the function looking for MODIFY_EXPRs with calls that
+ return in memory on the RHS. For each of these, determine whether it is
+ safe to pass the address of the LHS as the return slot, and mark the
+ call appropriately if so.
+
+ The NRV shares the return slot with a local variable in the callee; this
+ optimization shares the return slot with the target of the call within
+ the caller. If the NRV is performed (which we can't know in general),
+ this optimization is safe if the address of the target has not
+ escaped prior to the call. If it has, modifications to the local
+ variable will produce visible changes elsewhere, as in PR c++/19317. */
+
+ static void
+ execute_return_slot_opt (void)
+ {
+ basic_block bb;
+
+ FOR_EACH_BB (bb)
+ {
+ block_stmt_iterator i;
+ for (i = bsi_start (bb); !bsi_end_p (i); bsi_next (&i))
+ {
+ tree stmt = bsi_stmt (i);
+ tree call;
+
+ if (TREE_CODE (stmt) == MODIFY_EXPR
+ && (call = TREE_OPERAND (stmt, 1),
+ TREE_CODE (call) == CALL_EXPR)
+ && !CALL_EXPR_RETURN_SLOT_OPT (call)
+ && aggregate_value_p (call, call))
+ {
+ def_operand_p def_p;
+ ssa_op_iter op_iter;
+
+ /* We determine whether or not the LHS address escapes by
+ asking whether it is call clobbered. When the LHS isn't a
+ simple decl, we need to check the VDEFs, so it's simplest
+ to just loop through all the DEFs. */
+ FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
+ {
+ tree def = DEF_FROM_PTR (def_p);
+ if (TREE_CODE (def) == SSA_NAME)
+ def = SSA_NAME_VAR (def);
+ if (is_call_clobbered (def))
+ goto unsafe;
+ }
+
+ /* No defs are call clobbered, so the optimization is safe. */
+ CALL_EXPR_RETURN_SLOT_OPT (call) = 1;
+ /* This is too late to mark the target addressable like we do
+ in gimplify_modify_expr_rhs, but that's OK; anything that
+ wasn't already addressable was handled there. */
+
+ unsafe:;
+ }
+ }
+ }
+ }
+
+ struct tree_opt_pass pass_return_slot =
+ {
+ "retslot", /* name */
+ NULL, /* gate */
+ execute_return_slot_opt, /* execute */
+ NULL, /* sub */
+ NULL, /* next */
+ 0, /* static_pass_number */
+ 0, /* tv_id */
+ PROP_ssa | PROP_alias, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ 0, /* todo_flags_finish */
+ 0 /* letter */
+ };
*** ./tree-inline.c.~1~ 2005-06-14 10:39:11.433114563 -0400
--- ./tree-inline.c 2005-06-11 15:51:11.000000000 -0400
*************** expand_call_inline (basic_block bb, tree
*** 2018,2030 ****
/* Initialize the parameters. */
args = TREE_OPERAND (t, 1);
- if (CALL_EXPR_HAS_RETURN_SLOT_ADDR (t))
- {
- return_slot_addr = TREE_VALUE (args);
- args = TREE_CHAIN (args);
- }
- else
- return_slot_addr = NULL_TREE;
initialize_inlined_parameters (id, args, TREE_OPERAND (t, 2), fn, bb);
--- 2018,2023 ----
*************** expand_call_inline (basic_block bb, tree
*** 2038,2047 ****
gcc_assert (TREE_CODE (DECL_INITIAL (fn)) == BLOCK);
/* Find the lhs to which the result of this call is assigned. */
! modify_dest = stmt;
! if (TREE_CODE (modify_dest) == MODIFY_EXPR)
{
! modify_dest = TREE_OPERAND (modify_dest, 0);
/* The function which we are inlining might not return a value,
in which case we should issue a warning that the function
--- 2031,2040 ----
gcc_assert (TREE_CODE (DECL_INITIAL (fn)) == BLOCK);
/* Find the lhs to which the result of this call is assigned. */
! return_slot_addr = NULL;
! if (TREE_CODE (stmt) == MODIFY_EXPR)
{
! modify_dest = TREE_OPERAND (stmt, 0);
/* The function which we are inlining might not return a value,
in which case we should issue a warning that the function
*************** expand_call_inline (basic_block bb, tree
*** 2051,2056 ****
--- 2044,2054 ----
uninitialized variable. */
if (DECL_P (modify_dest))
TREE_NO_WARNING (modify_dest) = 1;
+ if (CALL_EXPR_RETURN_SLOT_OPT (t))
+ {
+ return_slot_addr = build_fold_addr_expr (modify_dest);
+ modify_dest = NULL;
+ }
}
else
modify_dest = NULL;
*** ./tree-optimize.c.~1~ 2005-06-14 10:39:11.437113809 -0400
--- ./tree-optimize.c 2005-06-11 18:37:20.000000000 -0400
*************** init_tree_optimization_passes (void)
*** 398,403 ****
--- 398,404 ----
NEXT_PASS (pass_build_ssa);
NEXT_PASS (pass_build_pta);
NEXT_PASS (pass_may_alias);
+ NEXT_PASS (pass_return_slot);
NEXT_PASS (pass_del_pta);
NEXT_PASS (pass_rename_ssa_copies);
NEXT_PASS (pass_early_warn_uninitialized);
*** ./tree-pass.h.~1~ 2005-06-14 10:39:11.439113432 -0400
--- ./tree-pass.h 2005-06-11 18:37:31.000000000 -0400
*************** extern struct tree_opt_pass pass_create_
*** 224,229 ****
--- 224,230 ----
extern struct tree_opt_pass pass_build_pta;
extern struct tree_opt_pass pass_del_pta;
extern struct tree_opt_pass pass_uncprop;
+ extern struct tree_opt_pass pass_return_slot;
extern struct tree_opt_pass pass_reassoc;
/* IPA Passes */
*** ./tree-pretty-print.c.~1~ 2005-06-14 10:39:11.442112866 -0400
--- ./tree-pretty-print.c 2005-06-11 15:51:12.000000000 -0400
*************** dump_generic_node (pretty_printer *buffe
*** 1003,1010 ****
pp_character (buffer, ']');
}
! if (CALL_EXPR_HAS_RETURN_SLOT_ADDR (node))
! pp_string (buffer, " [return slot addr]");
if (CALL_EXPR_TAILCALL (node))
pp_string (buffer, " [tail call]");
break;
--- 1003,1010 ----
pp_character (buffer, ']');
}
! if (CALL_EXPR_RETURN_SLOT_OPT (node))
! pp_string (buffer, " [return slot optimization]");
if (CALL_EXPR_TAILCALL (node))
pp_string (buffer, " [tail call]");
break;