This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix part of pr25505


Diego Novillo wrote:

> Yeah, that should work.  You may want to handle INDIRECT_REF by
> recursing with the memory tag associated with its underlying pointer.
> Definitely not 4.2 material, though (unless the memory savings on this
> PR are sufficiently compelling and you can make a good case for the
> safety of this patch).

Well, it's tough to say.  On the instance provided in the PR, the
savings is 2.5k of stack space.  However, this code was generated by
automated code-generation tools, and as a result the code contains a few
constructs repeated frequently.

I would argue that it's much cleaner and better understood than what is
in there now -- the discussions seem to support this.  If you're not
comfortable with this patch, perhaps you could consider the original
patch that only affects VAR_DECLs.

> Also, dest_safe_for_nrv_p ought to return 'bool'.

Thanks.

A test run of the attached patch passed with no regressions, although it
was before I changed the return type to bool.  I verified after changing
it to bool that it ran the new tests without failure.

Where should I go from here?

- Josh

2006-09-01  Josh Conner  <jconner@apple.com>

	PR c++/25505
	* tree-nrv.c (dest_safe_for_nrv_p): New function.
	(execute_return_slot_opt): Use it.

2006-09-01  Josh Conner  <jconner@apple.com>

	PR c++/25505
	gcc.dg/nrv3.c: New test.
	gcc.dg/nrv4.c: New test.
	gcc.dg/nrv5.c: New test.

Index: gcc/tree-nrv.c
===================================================================
--- gcc/tree-nrv.c	(revision 116353)
+++ gcc/tree-nrv.c	(working copy)
@@ -231,6 +231,39 @@ struct tree_opt_pass pass_nrv = 
   0					/* letter */
 };
 
+/* Determine (pessimistically) whether DEST is available for NRV
+   optimization, where DEST is expected to be the LHS of a modify
+   expression where the RHS is a function returning an aggregate.
+
+   We search for a base VAR_DECL and look to see if it, or any of its
+   subvars are clobbered.  Note that we could do better, for example, by
+   attempting to doing points-to analysis on INDIRECT_REFs.  */
+
+static bool
+dest_safe_for_nrv_p (tree dest)
+{
+  switch (TREE_CODE (dest))
+    {
+      case VAR_DECL:
+	{
+	  subvar_t subvar;
+	  if (is_call_clobbered (dest))
+	    return false;
+	  for (subvar = get_subvars_for_var (dest);
+	       subvar;
+	       subvar = subvar->next)
+	    if (is_call_clobbered (subvar->var))
+	      return false;
+	  return true;
+	}
+      case ARRAY_REF:
+      case COMPONENT_REF:
+	return dest_safe_for_nrv_p (TREE_OPERAND (dest, 0));
+      default:
+	return false;
+    }
+}
+
 /* 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
@@ -261,31 +294,10 @@ execute_return_slot_opt (void)
 		  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:;
-	    }
+	    /* Check if the location being assigned to is
+	       call-clobbered.  */
+	    CALL_EXPR_RETURN_SLOT_OPT (call) =
+	      dest_safe_for_nrv_p (TREE_OPERAND (stmt, 0)) ? 1 : 0;
 	}
     }
   return 0;
Index: gcc/testsuite/gcc.dg/nrv3.c
===================================================================
--- gcc/testsuite/gcc.dg/nrv3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/nrv3.c	(revision 0)
@@ -0,0 +1,30 @@
+/* Verify that gimple-level NRV is occurring when values other than the
+   return slot are call-clobbered.  */
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+typedef struct { int x; void *y; } S;
+typedef struct { int a; S b; } T;
+S nrv_candidate (void);
+void use_result (S, int);
+int *ptr;
+void foo (void)
+{
+  S result;
+  T result_arr[10][5];
+
+  int i;
+
+  ptr = &i;
+
+  /* i is call-clobbered for these calls, but result and result_arr
+     aren't.  */
+  result = nrv_candidate ();
+  result_arr[3][4].b = nrv_candidate ();
+
+  use_result (result, i);
+  use_result (result_arr[3][4].b, i);
+}
+
+/* { dg-final { scan-tree-dump-times "return slot optimization" 2 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: gcc/testsuite/gcc.dg/nrv4.c
===================================================================
--- gcc/testsuite/gcc.dg/nrv4.c	(revision 0)
+++ gcc/testsuite/gcc.dg/nrv4.c	(revision 0)
@@ -0,0 +1,33 @@
+/* Verify that NRV optimizations are prohibited when the LHS is an
+   indirect reference to something that may be call-clobbered. */
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+typedef struct { int x; void *y; } S;
+S nrv_candidate (void);
+void use_result (S);
+void make_escape (S *);
+S global_S;
+void foo (void)
+{
+  S *result;
+  S local_S;
+
+  /* We can't perform return slot optimization because global_S is
+     global and may be clobbered by nrv_candidate.  */
+  result = &global_S;
+  *result = nrv_candidate ();
+  use_result (*result);
+
+  /* We can't perform return slot optimization because local_S is
+     call_clobbered (its address escapes prior to invoking
+     nrv_candidate).  */
+  make_escape (&local_S);
+  result = &local_S;
+  *result = nrv_candidate ();
+  use_result (*result);
+}
+
+/* { dg-final { scan-tree-dump-times "return slot optimization" 0 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+
Index: gcc/testsuite/gcc.dg/nrv5.c
===================================================================
--- gcc/testsuite/gcc.dg/nrv5.c	(revision 0)
+++ gcc/testsuite/gcc.dg/nrv5.c	(revision 0)
@@ -0,0 +1,28 @@
+/* Verify that NRV optimizations are prohibited when the LHS is
+   something that may be call-clobbered. */
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+typedef struct { int x; void *y; } S;
+typedef struct { int a; S b; } T;
+S nrv_candidate (void);
+void use_result (S);
+void make_escape (S *);
+void foo (void)
+{
+  S result;
+  T result_arr[10][5];
+
+  make_escape (&result);
+  make_escape (&(result_arr[3][4].b));
+
+  /* Neither call should be allowed to use NRV optimization.  */
+  result = nrv_candidate ();
+  result_arr[3][4].b = nrv_candidate ();
+
+  use_result (result);
+  use_result (result_arr[3][4].b);
+}
+
+/* { dg-final { scan-tree-dump-times "return slot optimization" 0 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]