fab: rewrite optimize_stack_restore call check [PR122033]

The call check in optimize_stack_restore is not the same as
what is described in the comment before the function. It has never
been the same even. It has always allowed all nromal builtins but not
target builtins while the comment says no calls.

This rewrites it to allow the following.
* a restore right before a noreturn is fine to be removed as the noreturn
  function will do the restore (or exit the program).
* Internal functions are ok because they will turn into an instruction or 2
* Still specifically reject alloca and __scrub_leave
* simple or inexpensive builtins (including target builtins)

This will both reject some more locations but also accepts more locations due
to the internal and target and noreturn function calls checks.

Bootstrapped and tested on x86_64-linux-gnu.

Changes since v1:
* v2: Add comment about why restoring before calls is important.

	PR tree-optimization/122033
gcc/ChangeLog:

	* tree-ssa-ccp.cc (optimize_stack_restore): Rewrite the call check.
	Update comment in the front to new rules on calls.
	* tree.h (fndecl_builtin_alloc_p): New function.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/pr122033-1.c: New test.
	* gcc.dg/tree-ssa/pr122033-2.c: New test.

Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr122033-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr122033-1.c
new file mode 100644 (file)
index 0000000..4ef8c6c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr122033-1.c
@@ -0,0 +1,18 @@
+/* PR middle-end/122033 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+void bar1 (char *, int);
+void bar3(void) __attribute__((noreturn));
+void foo1 (int size)
+{
+  {
+    char temp[size];
+    temp[size-1] = '\0';
+    bar1 (temp, size);
+  }
+  bar3 ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_stack_save" "optimized"} } */
+/* { dg-final { scan-tree-dump-not "__builtin_stack_restore" "optimized"} } */

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr122033-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr122033-2.c
new file mode 100644 (file)
index 0000000..f429324
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr122033-2.c
@@ -0,0 +1,23 @@
+/* PR middle-end/122033 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+void g(int*);
+void h();
+double t;
+void f(int a, int b)
+{
+  {
+    int array0[a];
+    {
+      int array1[b];
+      g(array0);
+      g(array1);
+    }
+    t = __builtin_sin(t);
+  }
+  h ();
+}
+
+/* { dg-final { scan-tree-dump-times "__builtin_stack_save" 2 "optimized"} } */
+/* { dg-final { scan-tree-dump-times "__builtin_stack_restore" 2 "optimized"} } */

diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index 6d9cbd82d07e072ad8382b43a9e303ff6b8389e8..51e133e860de9541bd62e5a3028c217a2d061230 100644 (file)
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -3091,7 +3091,12 @@ make_pass_ccp (gcc::context *ctxt)
    if there is another __builtin_stack_restore in the same basic
    block and no calls or ASM_EXPRs are in between, or if this block's
    only outgoing edge is to EXIT_BLOCK and there are no calls or
-   ASM_EXPRs after this __builtin_stack_restore.  */
+   ASM_EXPRs after this __builtin_stack_restore.
+   Note restore right before a noreturn function is not needed.
+   And skip some cheap calls that will most likely become an instruction.
+   Restoring the stack before a call is important to be able to keep
+   stack usage down so that call does not run out of stack.  */
+
 
 static tree
 optimize_stack_restore (gimple_stmt_iterator i)
@@ -3111,26 +3116,43 @@ optimize_stack_restore (gimple_stmt_iterator i)
   for (gsi_next (&i); !gsi_end_p (i); gsi_next (&i))
     {
       stmt = gsi_stmt (i);
-      if (gimple_code (stmt) == GIMPLE_ASM)
+      if (is_a<gasm*> (stmt))
        return NULL_TREE;
-      if (gimple_code (stmt) != GIMPLE_CALL)
+      gcall *call = dyn_cast<gcall*>(stmt);
+      if (!call)
+       continue;
+
+      /* We can remove the restore in front of noreturn
+        calls.  Since the restore will happen either
+        via an unwind/longjmp or not at all. */
+      if (gimple_call_noreturn_p (call))
+       break;
+
+      /* Internal calls are ok, to bypass
+        check first since fndecl will be null. */
+      if (gimple_call_internal_p (call))
        continue;
 
-      callee = gimple_call_fndecl (stmt);
+      callee = gimple_call_fndecl (call);
+      /* Non-builtin calls are not ok. */
       if (!callee
-         || !fndecl_built_in_p (callee, BUILT_IN_NORMAL)
-         /* All regular builtins are ok, just obviously not alloca.  */
-         || ALLOCA_FUNCTION_CODE_P (DECL_FUNCTION_CODE (callee))
-         /* Do not remove stack updates before strub leave.  */
-         || fndecl_built_in_p (callee, BUILT_IN___STRUB_LEAVE))
+         || !fndecl_built_in_p (callee))
+       return NULL_TREE;
+
+      /* Do not remove stack updates before strub leave.  */
+      if (fndecl_built_in_p (callee, BUILT_IN___STRUB_LEAVE)
+         /* Alloca calls are not ok either. */
+         || fndecl_builtin_alloc_p (callee))
        return NULL_TREE;
 
       if (fndecl_built_in_p (callee, BUILT_IN_STACK_RESTORE))
        goto second_stack_restore;
-    }
 
-  if (!gsi_end_p (i))
-    return NULL_TREE;
+      /* If not a simple or inexpensive builtin, then it is not ok either. */
+      if (!is_simple_builtin (callee)
+         && !is_inexpensive_builtin (callee))
+       return NULL_TREE;
+    }
 
   /* Allow one successor of the exit block, or zero successors.  */
   switch (EDGE_COUNT (bb->succs))

diff --git a/gcc/tree.h b/gcc/tree.h
index 4c8ad9842ff03dd0d8ddcf9eb72e6282a7133d8d..6e46374357c10b99a61856cf4070aa0ff5cfc161 100644 (file)
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -7005,6 +7005,15 @@ fndecl_built_in_p (const_tree node, built_in_function name1, F... names)
                                        name1, names...));
 }
 
+/* Returns true if the function decl NODE is an alloca. */
+inline bool
+fndecl_builtin_alloc_p (const_tree node)
+{
+  if (!fndecl_built_in_p (node, BUILT_IN_NORMAL))
+    return false;
+  return ALLOCA_FUNCTION_CODE_P (DECL_FUNCTION_CODE (node));
+}
+
 /* A struct for encapsulating location information about an operator
    and the operation built from it.