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

12 files changed, 198 insertions, 46 deletions

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  <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.
+	* tree-pass.h: Declare pass_return_slot.
+	* tree-optimize.c (init_tree_optimization_passes): Add it.
+	
 2005-06-23  David Edelsohn  <edelsohn@gnu.org>
 
 	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  <jason@redhat.com>
+
+	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  <reichelt@igpm.rwth-aachen.de>
 
 	* 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).  */