diff mbox

[PR51491] add CLOBBER before __builtin_stack_restore when converting vla alloca_with_align to array decl

Message ID 4EE9BC8F.8020803@mentor.com
State New
Headers show

Commit Message

Tom de Vries Dec. 15, 2011, 9:23 a.m. UTC
On 13/12/11 14:13, Jakub Jelinek wrote:
> On Tue, Dec 13, 2011 at 01:26:42PM +0100, Tom de Vries wrote:
>> 2011-12-13  Tom de Vries  <tom@codesourcery.com>
>>
>> 	PR tree-optimization/51491
>> 	* tree-ssa-ccp.c (insert_clobber_before_stack_restore): New function.
>> 	(ccp_fold_stmt): Use insert_clobber_before_stack_restore after a
>> 	successful fold_builtin_alloca_with_align.
> 
> I don't think this is safe.  You don't want to look for any following
> __builtin_stack_restore, but for the matching one.  Consider:
> 
> int g (int *);
> 
> int
> f (int n)
> {
>   int tt = 0;
>   int t = 4;
>   {
>     int a[t
> #ifdef DIFFERENT_BB2
>           + (tt != 0 ? 6 : 0)
> #endif
>          ];
>     tt = g (a);
>     {
>       int b[n];
>       tt += g (b);
> #ifdef DIFFERENT_BB
>       if (n > 20)
> 	tt += 148 * g (b);
> #endif      
>       tt += b[0];
>     }
>     tt += a[0];
>   }
>   {
>     int a[4];
>     tt += g (a);
>     tt += a[0];
>   }
>   return tt;
> }
> 
> Without any defines, this shows that looking for the first
> BUILT_IN_STACK_RESTORE is wrong if you ignore BUILT_IN_STACK_SAVE calls.
> And with the various defines it shows that neither the corresponding
> __builtin_stack_save nor __builtin_stack_restore have to be in the same
> bb as __builtin_alloca_with_align that is being folded.

Jakub,

thanks for pointing that out. Above example is now included as test-case.

> Perhaps you want to look for the closest enclosing __builtin_stack_save
> (search backwards in current basic block, its immediate dominator etc.?),
> remember what SSA_NAME it stores its result into and then just look at
> where is the (single?) user of that, which ought to be
> __builtin_stack_restore.
> 

this new patch looks for the closest dominating STACK_SAVE with result, and
inserts a clobber before each corresponding STACK_RESTORE.

The value restored by a STACK_RESTORE may be a phi result, so the patch also
deals with this, in a recursive manner, while keeping tracking of visited phi
nodes to avoid infinite recursion.

bootstrapped and reg-tested (ada inclusive) on x86_64.

Ok for stage3?

Thanks,
- Tom

2011-12-15  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/51491
	* tree-ssa-ccp.c (insert_clobber_before_stack_restore)
	(gsi_prev_dom_bb_nondebug, insert_clobbers_for_var): New function.
	(ccp_fold_stmt): Use insert_clobbers_for_var after a successful
	fold_builtin_alloca_with_align.
	(ccp_visit_stmt): Calculate and free dominator info.

	* gcc.dg/pr51491.c: New test.
	* gcc.dg/pr51491-2.c: Same.

Comments

Richard Henderson Dec. 16, 2011, 10:39 p.m. UTC | #1
On 12/15/2011 01:23 AM, Tom de Vries wrote:
> 	PR tree-optimization/51491
> 	* tree-ssa-ccp.c (insert_clobber_before_stack_restore)
> 	(gsi_prev_dom_bb_nondebug, insert_clobbers_for_var): New function.
> 	(ccp_fold_stmt): Use insert_clobbers_for_var after a successful
> 	fold_builtin_alloca_with_align.
> 	(ccp_visit_stmt): Calculate and free dominator info.
> 
> 	* gcc.dg/pr51491.c: New test.
> 	* gcc.dg/pr51491-2.c: Same.

Ok.


r~
diff mbox

Patch

Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c (revision 182098)
+++ gcc/tree-ssa-ccp.c (working copy)
@@ -1690,6 +1690,96 @@  evaluate_stmt (gimple stmt)
   return val;
 }
 
+/* Given a BUILT_IN_STACK_SAVE value SAVED_VAL, insert a clobber of VAR before
+   each matching BUILT_IN_STACK_RESTORE.  Mark visited phis in VISITED.  */
+
+static void
+insert_clobber_before_stack_restore (tree saved_val, tree var, htab_t *visited)
+{
+  gimple stmt, clobber_stmt;
+  tree clobber;
+  imm_use_iterator iter;
+  gimple_stmt_iterator i;
+  gimple *slot;
+
+  FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
+    if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
+      {
+	clobber = build_constructor (TREE_TYPE (var), NULL);
+	TREE_THIS_VOLATILE (clobber) = 1;
+	clobber_stmt = gimple_build_assign (var, clobber);
+
+	i = gsi_for_stmt (stmt);
+	gsi_insert_before (&i, clobber_stmt, GSI_SAME_STMT);
+      }
+    else if (gimple_code (stmt) == GIMPLE_PHI)
+      {
+	if (*visited == NULL)
+	  *visited = htab_create (10, htab_hash_pointer, htab_eq_pointer, NULL);
+
+	slot = (gimple *)htab_find_slot (*visited, stmt, INSERT);
+	if (*slot != NULL)
+	  continue;
+
+	*slot = stmt;
+	insert_clobber_before_stack_restore (gimple_phi_result (stmt), var,
+					     visited);
+      }
+    else
+      gcc_assert (is_gimple_debug (stmt));
+}
+
+/* Advance the iterator to the previous non-debug gimple statement in the same
+   or dominating basic block.  */
+
+static inline void
+gsi_prev_dom_bb_nondebug (gimple_stmt_iterator *i)
+{
+  basic_block dom;
+
+  gsi_prev_nondebug (i);
+  while (gsi_end_p (*i))
+    {
+      dom = get_immediate_dominator (CDI_DOMINATORS, i->bb);
+      if (dom == NULL || dom == ENTRY_BLOCK_PTR)
+	return;
+
+      *i = gsi_last_bb (dom);
+    }
+}
+
+/* Find a BUILT_IN_STACK_SAVE dominating gsi_stmt (I), and insert
+   a clobber of VAR before each matching BUILT_IN_STACK_RESTORE.  */
+
+static void
+insert_clobbers_for_var (gimple_stmt_iterator i, tree var)
+{
+  bool save_found;
+  gimple stmt;
+  tree saved_val;
+  htab_t visited = NULL;
+
+  for (save_found = false; !gsi_end_p (i); gsi_prev_dom_bb_nondebug (&i))
+    {
+      stmt = gsi_stmt (i);
+
+      if (!gimple_call_builtin_p (stmt, BUILT_IN_STACK_SAVE))
+	continue;
+      save_found = true;
+
+      saved_val = gimple_call_lhs (stmt);
+      if (saved_val == NULL_TREE)
+	continue;
+
+      insert_clobber_before_stack_restore (saved_val, var, &visited);
+      break;
+    }
+
+  if (visited != NULL)
+    htab_delete (visited);
+  gcc_assert (save_found);
+}
+
 /* Detects a __builtin_alloca_with_align with constant size argument.  Declares
    fixed-size array and returns the address, if found, otherwise returns
    NULL_TREE.  */
@@ -1824,7 +1914,9 @@  ccp_fold_stmt (gimple_stmt_iterator *gsi
             if (new_rhs)
 	      {
 		bool res = update_call_from_tree (gsi, new_rhs);
+		tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
 		gcc_assert (res);
+		insert_clobbers_for_var (*gsi, var);
 		return true;
 	      }
           }
@@ -2024,12 +2116,14 @@  ccp_visit_stmt (gimple stmt, edge *taken
 static unsigned int
 do_ssa_ccp (void)
 {
+  unsigned int todo = 0;
+  calculate_dominance_info (CDI_DOMINATORS);
   ccp_initialize ();
   ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node);
   if (ccp_finalize ())
-    return (TODO_cleanup_cfg | TODO_update_ssa | TODO_remove_unused_locals);
-  else
-    return 0;
+    todo = (TODO_cleanup_cfg | TODO_update_ssa | TODO_remove_unused_locals);
+  free_dominance_info (CDI_DOMINATORS);
+  return todo;
 }
 
 
Index: gcc/testsuite/gcc.dg/pr51491-2.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/pr51491-2.c (revision 0)
@@ -0,0 +1,34 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-ccp1" } */
+
+int g (int *);
+
+int
+f (int n)
+{
+  int tt = 0;
+  int t = 4;
+  {
+    int a[t
+          + (tt != 0 ? 6 : 0)
+         ];
+    tt = g (a);
+    {
+      int b[n];
+      tt += g (b);
+      if (n > 20)
+	tt += 148 * g (b);
+      tt += b[0];
+    }
+    tt += a[0];
+  }
+  {
+    int a[4];
+    tt += g (a);
+    tt += a[0];
+  }
+  return tt;
+}
+
+/* { dg-final { scan-tree-dump-times "CLOBBER" 2 "ccp1"} } */
+/* { dg-final { cleanup-treee-dump "ccp1" } } */
Index: gcc/testsuite/gcc.dg/pr51491.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/pr51491.c (revision 0)
@@ -0,0 +1,25 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-rtl-expand" } */
+
+
+int g(int*);
+
+int f(void)
+{
+  int tt = 0;
+  int t = 4;
+  {
+    int a[t];
+    tt = g(a);
+    tt += a[0];
+  }
+  {
+    int a[4];
+    tt += g(a);
+    tt += a[0];
+  }
+  return tt;
+}
+
+/* { dg-final { scan-rtl-dump-times "Partition" 1 "expand"} } */
+/* { dg-final { cleanup-rtl-dump "expand" } } */