diff mbox

Fix PR80539

Message ID alpine.LSU.2.20.1704271121400.17885@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener April 27, 2017, 9:25 a.m. UTC 
SCEV analysis requires us to be in loop-closed SSA form to be able
to compute overall effects of inner loops when required.  Unfortunately
we have too many places it is used where we are not in loop-closed SSA
form.  The following patch makes us more conservative in two places
where we previously ICEd.

I'm not 100% sure we'll get away w/ not being in loop-closed SSA form
(code correctness wise) but unless we get a testcase showing this the
following is a sane approach (esp. on release branches).

Bootstrap and regtest in progress on x86_64-unknown-linux-gnu.

Richard.

2017-04-27  Richard Biener  <rguenther@suse.de>

	PR middle-end/80539
	* tree-chrec.c (chrec_fold_plus_poly_poly): Deal with not
	being in loop-closed SSA form conservatively.
	(chrec_fold_multiply_poly_poly): Likewise.

	* gcc.dg/torture/pr80539.c: New testcase.

Comments

Bin.Cheng April 27, 2017, 9:32 a.m. UTC | #1 
On Thu, Apr 27, 2017 at 10:25 AM, Richard Biener <rguenther@suse.de> wrote:
>
> SCEV analysis requires us to be in loop-closed SSA form to be able
> to compute overall effects of inner loops when required.  Unfortunately
> we have too many places it is used where we are not in loop-closed SSA
> form.  The following patch makes us more conservative in two places
> where we previously ICEd.
>
> I'm not 100% sure we'll get away w/ not being in loop-closed SSA form
> (code correctness wise) but unless we get a testcase showing this the
> following is a sane approach (esp. on release branches).
>
> Bootstrap and regtest in progress on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2017-04-27  Richard Biener  <rguenther@suse.de>
>
>         PR middle-end/80539
>         * tree-chrec.c (chrec_fold_plus_poly_poly): Deal with not
>         being in loop-closed SSA form conservatively.
>         (chrec_fold_multiply_poly_poly): Likewise.
>
>         * gcc.dg/torture/pr80539.c: New testcase.
>
> Index: gcc/tree-chrec.c
> ===================================================================
> --- gcc/tree-chrec.c    (revision 247293)
> +++ gcc/tree-chrec.c    (working copy)
> @@ -149,7 +149,12 @@ chrec_fold_plus_poly_poly (enum tree_cod
>
>    /* This function should never be called for chrecs of loops that
>       do not belong to the same loop nest.  */
> -  gcc_assert (loop0 == loop1);
> +  if (loop0 != loop1)
> +    {
> +      /* It still can happen if we are not in loop-closed SSA form.  */
> +      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
> +      return chrec_dont_know;
> +    }
But it returns chrec_dont_know in the case it didn't if LOOP_CLOSED_SSA?

Thanks,
bin
>
>    if (code == PLUS_EXPR || code == POINTER_PLUS_EXPR)
>      {
> @@ -211,7 +216,12 @@ chrec_fold_multiply_poly_poly (tree type
>         chrec_fold_multiply (type, CHREC_LEFT (poly0), poly1),
>         CHREC_RIGHT (poly0));
>
> -  gcc_assert (loop0 == loop1);
> +  if (loop0 != loop1)
> +    {
> +      /* It still can happen if we are not in loop-closed SSA form.  */
> +      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
> +      return chrec_dont_know;
> +    }
>
>    /* poly0 and poly1 are two polynomials in the same variable,
>       {a, +, b}_x * {c, +, d}_x  ->  {a*c, +, a*d + b*c + b*d, +, 2*b*d}_x.  */
> Index: gcc/testsuite/gcc.dg/torture/pr80539.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr80539.c      (nonexistent)
> +++ gcc/testsuite/gcc.dg/torture/pr80539.c      (working copy)
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +
> +signed char a, b;
> +void fn1()
> +{
> +  signed char c, e;
> +  short d;
> +  if (0) {
> +      for (; d;) {
> +l1:
> +         for (c = 7; a; c++)
> +           ;
> +         e = 6;
> +         for (; b; e++)
> +           ;
> +      }
> +      c -= e;
> +  }
> +  if (d == 7)
> +    goto l1;
> +  a = c;
> +}
Richard Biener April 27, 2017, 10:08 a.m. UTC | #2 
On Thu, 27 Apr 2017, Bin.Cheng wrote:

> On Thu, Apr 27, 2017 at 10:25 AM, Richard Biener <rguenther@suse.de> wrote:
> >
> > SCEV analysis requires us to be in loop-closed SSA form to be able
> > to compute overall effects of inner loops when required.  Unfortunately
> > we have too many places it is used where we are not in loop-closed SSA
> > form.  The following patch makes us more conservative in two places
> > where we previously ICEd.
> >
> > I'm not 100% sure we'll get away w/ not being in loop-closed SSA form
> > (code correctness wise) but unless we get a testcase showing this the
> > following is a sane approach (esp. on release branches).
> >
> > Bootstrap and regtest in progress on x86_64-unknown-linux-gnu.
> >
> > Richard.
> >
> > 2017-04-27  Richard Biener  <rguenther@suse.de>
> >
> >         PR middle-end/80539
> >         * tree-chrec.c (chrec_fold_plus_poly_poly): Deal with not
> >         being in loop-closed SSA form conservatively.
> >         (chrec_fold_multiply_poly_poly): Likewise.
> >
> >         * gcc.dg/torture/pr80539.c: New testcase.
> >
> > Index: gcc/tree-chrec.c
> > ===================================================================
> > --- gcc/tree-chrec.c    (revision 247293)
> > +++ gcc/tree-chrec.c    (working copy)
> > @@ -149,7 +149,12 @@ chrec_fold_plus_poly_poly (enum tree_cod
> >
> >    /* This function should never be called for chrecs of loops that
> >       do not belong to the same loop nest.  */
> > -  gcc_assert (loop0 == loop1);
> > +  if (loop0 != loop1)
> > +    {
> > +      /* It still can happen if we are not in loop-closed SSA form.  */
> > +      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
> > +      return chrec_dont_know;
> > +    }
> But it returns chrec_dont_know in the case it didn't if LOOP_CLOSED_SSA?

if ! LOOP_CLOSED_SSA, if in LOOP_CLOSED_SSA it continues to ICE.

Richard.

> Thanks,
> bin
> >
> >    if (code == PLUS_EXPR || code == POINTER_PLUS_EXPR)
> >      {
> > @@ -211,7 +216,12 @@ chrec_fold_multiply_poly_poly (tree type
> >         chrec_fold_multiply (type, CHREC_LEFT (poly0), poly1),
> >         CHREC_RIGHT (poly0));
> >
> > -  gcc_assert (loop0 == loop1);
> > +  if (loop0 != loop1)
> > +    {
> > +      /* It still can happen if we are not in loop-closed SSA form.  */
> > +      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
> > +      return chrec_dont_know;
> > +    }
> >
> >    /* poly0 and poly1 are two polynomials in the same variable,
> >       {a, +, b}_x * {c, +, d}_x  ->  {a*c, +, a*d + b*c + b*d, +, 2*b*d}_x.  */
> > Index: gcc/testsuite/gcc.dg/torture/pr80539.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/torture/pr80539.c      (nonexistent)
> > +++ gcc/testsuite/gcc.dg/torture/pr80539.c      (working copy)
> > @@ -0,0 +1,22 @@
> > +/* { dg-do compile } */
> > +
> > +signed char a, b;
> > +void fn1()
> > +{
> > +  signed char c, e;
> > +  short d;
> > +  if (0) {
> > +      for (; d;) {
> > +l1:
> > +         for (c = 7; a; c++)
> > +           ;
> > +         e = 6;
> > +         for (; b; e++)
> > +           ;
> > +      }
> > +      c -= e;
> > +  }
> > +  if (d == 7)
> > +    goto l1;
> > +  a = c;
> > +}
> 
>
Sebastian Pop April 27, 2017, 2:05 p.m. UTC | #3 
The patch looks good to me.

Thanks,
Sebastian

On Thu, Apr 27, 2017 at 4:25 AM, Richard Biener <rguenther@suse.de> wrote:
>
> SCEV analysis requires us to be in loop-closed SSA form to be able
> to compute overall effects of inner loops when required.  Unfortunately
> we have too many places it is used where we are not in loop-closed SSA
> form.  The following patch makes us more conservative in two places
> where we previously ICEd.
>
> I'm not 100% sure we'll get away w/ not being in loop-closed SSA form
> (code correctness wise) but unless we get a testcase showing this the
> following is a sane approach (esp. on release branches).
>
> Bootstrap and regtest in progress on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2017-04-27  Richard Biener  <rguenther@suse.de>
>
>         PR middle-end/80539
>         * tree-chrec.c (chrec_fold_plus_poly_poly): Deal with not
>         being in loop-closed SSA form conservatively.
>         (chrec_fold_multiply_poly_poly): Likewise.
>
>         * gcc.dg/torture/pr80539.c: New testcase.
>
> Index: gcc/tree-chrec.c
> ===================================================================
> --- gcc/tree-chrec.c    (revision 247293)
> +++ gcc/tree-chrec.c    (working copy)
> @@ -149,7 +149,12 @@ chrec_fold_plus_poly_poly (enum tree_cod
>
>    /* This function should never be called for chrecs of loops that
>       do not belong to the same loop nest.  */
> -  gcc_assert (loop0 == loop1);
> +  if (loop0 != loop1)
> +    {
> +      /* It still can happen if we are not in loop-closed SSA form.  */
> +      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
> +      return chrec_dont_know;
> +    }
>
>    if (code == PLUS_EXPR || code == POINTER_PLUS_EXPR)
>      {
> @@ -211,7 +216,12 @@ chrec_fold_multiply_poly_poly (tree type
>         chrec_fold_multiply (type, CHREC_LEFT (poly0), poly1),
>         CHREC_RIGHT (poly0));
>
> -  gcc_assert (loop0 == loop1);
> +  if (loop0 != loop1)
> +    {
> +      /* It still can happen if we are not in loop-closed SSA form.  */
> +      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
> +      return chrec_dont_know;
> +    }
>
>    /* poly0 and poly1 are two polynomials in the same variable,
>       {a, +, b}_x * {c, +, d}_x  ->  {a*c, +, a*d + b*c + b*d, +, 2*b*d}_x.  */
> Index: gcc/testsuite/gcc.dg/torture/pr80539.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr80539.c      (nonexistent)
> +++ gcc/testsuite/gcc.dg/torture/pr80539.c      (working copy)
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +
> +signed char a, b;
> +void fn1()
> +{
> +  signed char c, e;
> +  short d;
> +  if (0) {
> +      for (; d;) {
> +l1:
> +         for (c = 7; a; c++)
> +           ;
> +         e = 6;
> +         for (; b; e++)
> +           ;
> +      }
> +      c -= e;
> +  }
> +  if (d == 7)
> +    goto l1;
> +  a = c;
> +}
diff mbox

Patch

Index: gcc/tree-chrec.c
===================================================================
--- gcc/tree-chrec.c	(revision 247293)
+++ gcc/tree-chrec.c	(working copy)
@@ -149,7 +149,12 @@  chrec_fold_plus_poly_poly (enum tree_cod
 
   /* This function should never be called for chrecs of loops that
      do not belong to the same loop nest.  */
-  gcc_assert (loop0 == loop1);
+  if (loop0 != loop1)
+    {
+      /* It still can happen if we are not in loop-closed SSA form.  */
+      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
+      return chrec_dont_know;
+    }
 
   if (code == PLUS_EXPR || code == POINTER_PLUS_EXPR)
     {
@@ -211,7 +216,12 @@  chrec_fold_multiply_poly_poly (tree type
        chrec_fold_multiply (type, CHREC_LEFT (poly0), poly1),
        CHREC_RIGHT (poly0));
 
-  gcc_assert (loop0 == loop1);
+  if (loop0 != loop1)
+    {
+      /* It still can happen if we are not in loop-closed SSA form.  */
+      gcc_assert (! loops_state_satisfies_p (LOOP_CLOSED_SSA));
+      return chrec_dont_know;
+    }
 
   /* poly0 and poly1 are two polynomials in the same variable,
      {a, +, b}_x * {c, +, d}_x  ->  {a*c, +, a*d + b*c + b*d, +, 2*b*d}_x.  */
Index: gcc/testsuite/gcc.dg/torture/pr80539.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr80539.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr80539.c	(working copy)
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+
+signed char a, b;
+void fn1()
+{
+  signed char c, e;
+  short d;
+  if (0) {
+      for (; d;) {
+l1:
+	  for (c = 7; a; c++)
+	    ;
+	  e = 6;
+	  for (; b; e++)
+	    ;
+      }
+      c -= e;
+  }
+  if (d == 7)
+    goto l1;
+  a = c;
+}