From fa47911c5600725d7d72e224e4a61829267b00b9 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Thu, 23 Jun 2005 10:44:21 -0400 Subject: [PATCH] re PR c++/19317 (removing a temporary return value when we cannot) 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. * tree-pass.h: Declare pass_return_slot. * tree-optimize.c (init_tree_optimization_passes): Add it. * cp/semantics.c (simplify_aggr_init_expr): Use CALL_EXPR_RETURN_SLOT_OPT, not CALL_EXPR_HAS_RETURN_SLOT_ADDR. From-SVN: r101269 --- gcc/ChangeLog | 19 +++++++++++ gcc/calls.c | 12 +------ gcc/cp/ChangeLog | 6 ++++ gcc/cp/semantics.c | 31 +++++++---------- gcc/gimplify.c | 40 ++++++++++++++++++++++ gcc/testsuite/g++.dg/opt/nrv9.C | 28 +++++++++++++++ gcc/tree-inline.c | 18 +++++----- gcc/tree-nrv.c | 76 +++++++++++++++++++++++++++++++++++++++++ gcc/tree-optimize.c | 1 + gcc/tree-pass.h | 1 + gcc/tree-pretty-print.c | 4 +-- gcc/tree.h | 8 ++--- 12 files changed, 198 insertions(+), 46 deletions(-) create mode 100644 gcc/testsuite/g++.dg/opt/nrv9.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 29087f5..c7aaa16 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,22 @@ +2005-06-23 Jason Merrill + + 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. + * tree-pass.h: Declare pass_return_slot. + * tree-optimize.c (init_tree_optimization_passes): Add it. + 2005-06-23 David Edelsohn PR target/21760 diff --git a/gcc/calls.c b/gcc/calls.c index 3c4cbf8..043888e 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1964,17 +1964,7 @@ expand_call (tree exp, rtx target, int ignore) { 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)) + if (target && MEM_P (target) && CALL_EXPR_RETURN_SLOT_OPT (exp)) structure_value_addr = XEXP (target, 0); else { diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index d6fc512..61b8a03 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2005-06-23 Jason Merrill + + PR c++/19317 + * semantics.c (simplify_aggr_init_expr): Use + CALL_EXPR_RETURN_SLOT_OPT, not CALL_EXPR_HAS_RETURN_SLOT_ADDR. + 2005-06-23 Volker Reichelt * pt.c (register_specialization): Remove superfluous assertion. diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 0c55975..157ec9c 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -2904,27 +2904,15 @@ simplify_aggr_init_expr (tree *tp) style = arg; } - if (style == ctor || style == arg) + if (style == ctor) { - /* Pass the address of the slot. If this is a constructor, we - replace the first argument; otherwise, we tack on a new one. */ + /* Replace the first argument to the ctor with the address of the + slot. */ tree addr; - if (style == ctor) - args = TREE_CHAIN (args); - + 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); } @@ -2933,9 +2921,13 @@ simplify_aggr_init_expr (tree *tp) 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; + { + /* 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 @@ -2945,6 +2937,7 @@ simplify_aggr_init_expr (tree *tp) 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; diff --git a/gcc/gimplify.c b/gcc/gimplify.c index df93bc5..8151272 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -3015,6 +3015,46 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p, tree *pre_p, 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; diff --git a/gcc/testsuite/g++.dg/opt/nrv9.C b/gcc/testsuite/g++.dg/opt/nrv9.C new file mode 100644 index 0000000..6f8d549 --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/nrv9.C @@ -0,0 +1,28 @@ +// PR c++/19317 +// If we do both NRV and caller-side return slot opt for ma = ma.f, +// constructing fa incorrectly sets ma.i to 0. + +extern "C" void abort(); + +struct A +{ + int i; + int pad[32]; // force return in memory + A(): i(0) {} + A(int ia): i(ia) {} +}; + +A ga(42); + +A f() +{ + A la; + if (ga.i != 42) + abort(); + return la; +} + +int main() +{ + ga = f (); +} diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index c670c57..8f9e278 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -2026,13 +2026,6 @@ expand_call_inline (basic_block bb, tree stmt, tree *tp, void *data) /* 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); @@ -2046,10 +2039,10 @@ expand_call_inline (basic_block bb, tree stmt, tree *tp, void *data) 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) + return_slot_addr = NULL; + if (TREE_CODE (stmt) == MODIFY_EXPR) { - modify_dest = TREE_OPERAND (modify_dest, 0); + 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 @@ -2059,6 +2052,11 @@ expand_call_inline (basic_block bb, tree stmt, tree *tp, void *data) 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; diff --git a/gcc/tree-nrv.c b/gcc/tree-nrv.c index 64e9636..a88c977 100644 --- a/gcc/tree-nrv.c +++ b/gcc/tree-nrv.c @@ -218,3 +218,79 @@ struct tree_opt_pass pass_nrv = 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 */ +}; diff --git a/gcc/tree-optimize.c b/gcc/tree-optimize.c index a33a710..c2f4fc9 100644 --- a/gcc/tree-optimize.c +++ b/gcc/tree-optimize.c @@ -397,6 +397,7 @@ init_tree_optimization_passes (void) 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); diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index 2c405d2..eb1ad31 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -224,6 +224,7 @@ extern struct tree_opt_pass pass_create_structure_vars; 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 */ diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c index 04cc8fa..251c03c 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -1003,8 +1003,8 @@ dump_generic_node (pretty_printer *buffer, tree node, int spc, int flags, pp_character (buffer, ']'); } - if (CALL_EXPR_HAS_RETURN_SLOT_ADDR (node)) - pp_string (buffer, " [return slot addr]"); + if (CALL_EXPR_RETURN_SLOT_OPT (node)) + pp_string (buffer, " [return slot optimization]"); if (CALL_EXPR_TAILCALL (node)) pp_string (buffer, " [tail call]"); break; diff --git a/gcc/tree.h b/gcc/tree.h index c434f2f..831856d 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -337,7 +337,7 @@ struct tree_common GTY(()) TREE_PRIVATE in ..._DECL - CALL_EXPR_HAS_RETURN_SLOT_ADDR in + CALL_EXPR_RETURN_SLOT_OPT in CALL_EXPR DECL_BY_REFERENCE in PARM_DECL, RESULT_DECL @@ -980,9 +980,9 @@ extern void tree_operand_check_failed (int, enum tree_code, 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 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). */ -- 2.7.4