Patchwork Fix ipa-split if there are extra PHIs on the return_bb (PR tree-optimization/46130)

login
register
mail settings
Submitter Jakub Jelinek
Date Jan. 19, 2011, 8:02 p.m.
Message ID <20110119200205.GU2724@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/79585/
State New
Headers show

Comments

Jakub Jelinek - Jan. 19, 2011, 8:02 p.m.
Hi!

ipa-split fixes the (single) virtual operand PHI and at most one normal PHI
- if it returns a gimple value from the *.part.* function.  If there are
other PHIs, it will ICE because nothing fixes them up (we'd need function
with multiple return values for them).  Generally this should happen only
if DCE didn't do a good job.  In whole x86_64-linux and i686-linux
bootstraps/regtests this triggered just on the added testcases.

Bootstrapped/regtested on those 2 targets, ok for trunk?

2011-01-19  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/46130
	* ipa-split.c (consider_split): If return_bb contains non-virtual
	PHIs other than for retval or if split_function would not adjust it,
	refuse to split.

	* gcc.dg/pr46130-1.c: New test.
	* gcc.dg/pr46130-2.c: New test.


	Jakub
Richard Guenther - Jan. 20, 2011, 10:14 a.m.
On Wed, Jan 19, 2011 at 9:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> ipa-split fixes the (single) virtual operand PHI and at most one normal PHI
> - if it returns a gimple value from the *.part.* function.  If there are
> other PHIs, it will ICE because nothing fixes them up (we'd need function
> with multiple return values for them).  Generally this should happen only
> if DCE didn't do a good job.  In whole x86_64-linux and i686-linux
> bootstraps/regtests this triggered just on the added testcases.
>
> Bootstrapped/regtested on those 2 targets, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-01-19  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/46130
>        * ipa-split.c (consider_split): If return_bb contains non-virtual
>        PHIs other than for retval or if split_function would not adjust it,
>        refuse to split.
>
>        * gcc.dg/pr46130-1.c: New test.
>        * gcc.dg/pr46130-2.c: New test.
>
> --- gcc/ipa-split.c.jj  2011-01-19 15:29:25.000000000 +0100
> +++ gcc/ipa-split.c     2011-01-19 18:17:34.000000000 +0100
> @@ -411,9 +411,6 @@ consider_split (struct split_point *curr
>                 "  Refused: split part has non-ssa uses\n");
>       return;
>     }
> -  if (dump_file && (dump_flags & TDF_DETAILS))
> -    fprintf (dump_file, "  Accepted!\n");
> -
>   /* See if retval used by return bb is computed by header or split part.
>      When it is computed by split part, we need to produce return statement
>      in the split part and add code to header to pass it around.
> @@ -451,6 +448,30 @@ consider_split (struct split_point *curr
>   else
>     current->split_part_set_retval = true;
>
> +  /* split_function fixes up at most one PHI non-virtual PHI node in return_bb,
> +     for the return value.  If there are other PHIs, give up.  */
> +  if (return_bb != EXIT_BLOCK_PTR)
> +    {
> +      gimple_stmt_iterator psi;
> +
> +      for (psi = gsi_start_phis (return_bb); !gsi_end_p (psi); gsi_next (&psi))
> +       if (is_gimple_reg (gimple_phi_result (gsi_stmt (psi)))
> +           && !(retval
> +                && current->split_part_set_retval
> +                && TREE_CODE (retval) == SSA_NAME
> +                && !DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))
> +                && SSA_NAME_DEF_STMT (retval) == gsi_stmt (psi)))
> +         {
> +           if (dump_file && (dump_flags & TDF_DETAILS))
> +             fprintf (dump_file,
> +                      "  Refused: return bb has extra PHIs\n");
> +           return;
> +         }
> +    }
> +
> +  if (dump_file && (dump_flags & TDF_DETAILS))
> +    fprintf (dump_file, "  Accepted!\n");
> +
>   /* At the moment chose split point with lowest frequency and that leaves
>      out smallest size of header.
>      In future we might re-consider this heuristics.  */
> --- gcc/testsuite/gcc.dg/pr46130-1.c.jj 2011-01-19 18:24:13.000000000 +0100
> +++ gcc/testsuite/gcc.dg/pr46130-1.c    2011-01-19 18:32:07.000000000 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/46130 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-dce" } */
> +
> +#include <stdarg.h>
> +
> +static void
> +foo (va_list ap)
> +{
> +  va_arg (ap, char *)[0];
> +}
> +
> +void
> +bar (va_list ap)
> +{
> +  foo (ap);
> +}
> +
> +void
> +baz (va_list ap)
> +{
> +  foo (ap);
> +}
> --- gcc/testsuite/gcc.dg/pr46130-2.c.jj 2011-01-19 18:24:10.000000000 +0100
> +++ gcc/testsuite/gcc.dg/pr46130-2.c    2011-01-19 18:24:05.000000000 +0100
> @@ -0,0 +1,32 @@
> +/* PR tree-optimization/46130 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-dce" } */
> +
> +extern int bar (int);
> +
> +static int foo (int x)
> +{
> +  int z, w;
> +  if (x <= 1024)
> +    {
> +      z = 16;
> +      w = 17;
> +    }
> +  else
> +    {
> +      bar (bar (bar (bar (bar (bar (bar (bar (bar (16)))))))));
> +      if (x > 131072)
> +       w = 19;
> +      else
> +       w = 21;
> +      z = 32;
> +    }
> +  w = w + 121;
> +  return z;
> +}
> +
> +int
> +baz (int x)
> +{
> +  return foo (x + 6) + foo (x + 15) + foo (x + 24);
> +}
>
>        Jakub
>

Patch

--- gcc/ipa-split.c.jj	2011-01-19 15:29:25.000000000 +0100
+++ gcc/ipa-split.c	2011-01-19 18:17:34.000000000 +0100
@@ -411,9 +411,6 @@  consider_split (struct split_point *curr
 		 "  Refused: split part has non-ssa uses\n");
       return;
     }
-  if (dump_file && (dump_flags & TDF_DETAILS))
-    fprintf (dump_file, "  Accepted!\n");
-
   /* See if retval used by return bb is computed by header or split part.
      When it is computed by split part, we need to produce return statement
      in the split part and add code to header to pass it around.
@@ -451,6 +448,30 @@  consider_split (struct split_point *curr
   else
     current->split_part_set_retval = true;
 
+  /* split_function fixes up at most one PHI non-virtual PHI node in return_bb,
+     for the return value.  If there are other PHIs, give up.  */
+  if (return_bb != EXIT_BLOCK_PTR)
+    {
+      gimple_stmt_iterator psi;
+
+      for (psi = gsi_start_phis (return_bb); !gsi_end_p (psi); gsi_next (&psi))
+	if (is_gimple_reg (gimple_phi_result (gsi_stmt (psi)))
+	    && !(retval
+		 && current->split_part_set_retval
+		 && TREE_CODE (retval) == SSA_NAME
+		 && !DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))
+		 && SSA_NAME_DEF_STMT (retval) == gsi_stmt (psi)))
+	  {
+	    if (dump_file && (dump_flags & TDF_DETAILS))
+	      fprintf (dump_file,
+		       "  Refused: return bb has extra PHIs\n");
+	    return;
+	  }
+    }
+
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    fprintf (dump_file, "  Accepted!\n");
+
   /* At the moment chose split point with lowest frequency and that leaves
      out smallest size of header.
      In future we might re-consider this heuristics.  */
--- gcc/testsuite/gcc.dg/pr46130-1.c.jj	2011-01-19 18:24:13.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr46130-1.c	2011-01-19 18:32:07.000000000 +0100
@@ -0,0 +1,23 @@ 
+/* PR tree-optimization/46130 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-dce" } */
+
+#include <stdarg.h>
+
+static void
+foo (va_list ap)
+{
+  va_arg (ap, char *)[0];
+}
+
+void
+bar (va_list ap)
+{
+  foo (ap);
+}
+
+void
+baz (va_list ap)
+{
+  foo (ap);
+}
--- gcc/testsuite/gcc.dg/pr46130-2.c.jj	2011-01-19 18:24:10.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr46130-2.c	2011-01-19 18:24:05.000000000 +0100
@@ -0,0 +1,32 @@ 
+/* PR tree-optimization/46130 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-dce" } */
+
+extern int bar (int);
+
+static int foo (int x)
+{
+  int z, w;
+  if (x <= 1024)
+    {
+      z = 16;
+      w = 17;
+    }
+  else
+    {
+      bar (bar (bar (bar (bar (bar (bar (bar (bar (16)))))))));
+      if (x > 131072)
+	w = 19;
+      else
+	w = 21;
+      z = 32;
+    }
+  w = w + 121;
+  return z;
+}
+
+int
+baz (int x)
+{
+  return foo (x + 6) + foo (x + 15) + foo (x + 24);
+}