public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964)
@ 2019-01-22 22:29 Jakub Jelinek
  2019-01-23  8:50 ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-01-22 22:29 UTC (permalink / raw)
  To: Richard Biener, Bin.Cheng; +Cc: gcc-patches

Hi!

SCEV can analyze not just integral/pointer IVs, but (scalar) float ones as
well.  Calling build_int_cst on such types results in ICE, build_zero_cst
works.  Though the loop invariant PHI IVs, if we represent them as using
+0.0 step, aren't correct if honoring signed zeros, as + 0.0 will make
a +0.0 out of -0.0.

Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?

2019-01-22  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/88964
	* gimple-loop-interchange.cc (loop_cand::analyze_induction_var): Use
	build_zero_cst instead of build_int_cst.  Return false for loop
	invariants which honor signed zeros.

	* gfortran.dg/pr88964.f90: New test.

--- gcc/gimple-loop-interchange.cc.jj	2019-01-01 12:37:17.416970701 +0100
+++ gcc/gimple-loop-interchange.cc	2019-01-22 11:34:42.303796570 +0100
@@ -688,11 +688,16 @@ loop_cand::analyze_induction_var (tree v
   /* Var is loop invariant, though it's unlikely to happen.  */
   if (tree_does_not_contain_chrecs (chrec))
     {
+      /* Punt on floating point invariants if honoring signed zeros,
+	 representing that as + 0.0 would change the result if init
+	 is -0.0.  */
+      if (HONOR_SIGNED_ZEROS (chrec))
+	return false;
       struct induction *iv = XCNEW (struct induction);
       iv->var = var;
       iv->init_val = init;
       iv->init_expr = chrec;
-      iv->step = build_int_cst (TREE_TYPE (chrec), 0);
+      iv->step = build_zero_cst (TREE_TYPE (chrec));
       m_inductions.safe_push (iv);
       return true;
     }
--- gcc/testsuite/gfortran.dg/pr88964.f90.jj	2019-01-22 11:54:46.046154065 +0100
+++ gcc/testsuite/gfortran.dg/pr88964.f90	2019-01-22 11:54:23.172526590 +0100
@@ -0,0 +1,57 @@
+! PR tree-optimization/88964
+! { dg-do compile }
+! { dg-options "-O3 -fno-tree-forwprop --param sccvn-max-alias-queries-per-access=1" }
+
+MODULE pr88964
+  INTEGER, PARAMETER :: dp=8
+  REAL(KIND=dp) :: p, q, o
+CONTAINS
+  SUBROUTINE foo(a,b,c,f,h)
+    IMPLICIT NONE
+    INTEGER :: a, b, c
+    REAL(KIND=dp) :: f(b*c), h(a*c)
+    CALL bar(h)
+    CALL baz(f)
+    CALL qux(h)
+  END SUBROUTINE foo
+  SUBROUTINE bar(h)
+    IMPLICIT NONE
+    REAL(KIND=dp) :: h(1*1)
+    INTEGER :: r, s, t, u
+    DO u = 1,3
+      DO t = 1,1
+        DO s = 1,3
+          DO r = 1,1
+            h((t-1)*1+r) = h((t-1)*1+r)-p*o
+          END DO
+        END DO
+      END DO
+    END DO
+  END SUBROUTINE bar
+  SUBROUTINE baz(f)
+    IMPLICIT NONE
+    REAL(KIND=dp) :: f(3*1)
+    INTEGER :: s, t, u
+    DO u = 1,4
+      DO t = 1,1
+        DO s = 1,3
+          f((t-1)*3+s) = f((t-1)*3+s) - q
+        END DO
+      END DO
+    END DO
+  END SUBROUTINE baz
+  SUBROUTINE qux(h)
+    IMPLICIT NONE
+    REAL(KIND=dp) :: h(1*1)
+    INTEGER :: r, s, t, u
+    DO u = 1,5
+      DO t = 1,1
+        DO s = 1,3
+          DO r = 1,1
+            h((t-1)*1+r) = h((t-1)*1+r)-p*o
+          END DO
+        END DO
+      END DO
+    END DO
+  END SUBROUTINE qux
+END MODULE pr88964

	Jakub

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964)
  2019-01-22 22:29 [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964) Jakub Jelinek
@ 2019-01-23  8:50 ` Richard Biener
  2019-01-24 19:22   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2019-01-23  8:50 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Bin.Cheng, gcc-patches

On Tue, 22 Jan 2019, Jakub Jelinek wrote:

> Hi!
> 
> SCEV can analyze not just integral/pointer IVs, but (scalar) float ones as
> well.  Calling build_int_cst on such types results in ICE, build_zero_cst
> works.  Though the loop invariant PHI IVs, if we represent them as using
> +0.0 step, aren't correct if honoring signed zeros, as + 0.0 will make
> a +0.0 out of -0.0.
> 
> Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?

OK.

Richard.

> 2019-01-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/88964
> 	* gimple-loop-interchange.cc (loop_cand::analyze_induction_var): Use
> 	build_zero_cst instead of build_int_cst.  Return false for loop
> 	invariants which honor signed zeros.
> 
> 	* gfortran.dg/pr88964.f90: New test.
> 
> --- gcc/gimple-loop-interchange.cc.jj	2019-01-01 12:37:17.416970701 +0100
> +++ gcc/gimple-loop-interchange.cc	2019-01-22 11:34:42.303796570 +0100
> @@ -688,11 +688,16 @@ loop_cand::analyze_induction_var (tree v
>    /* Var is loop invariant, though it's unlikely to happen.  */
>    if (tree_does_not_contain_chrecs (chrec))
>      {
> +      /* Punt on floating point invariants if honoring signed zeros,
> +	 representing that as + 0.0 would change the result if init
> +	 is -0.0.  */
> +      if (HONOR_SIGNED_ZEROS (chrec))
> +	return false;
>        struct induction *iv = XCNEW (struct induction);
>        iv->var = var;
>        iv->init_val = init;
>        iv->init_expr = chrec;
> -      iv->step = build_int_cst (TREE_TYPE (chrec), 0);
> +      iv->step = build_zero_cst (TREE_TYPE (chrec));
>        m_inductions.safe_push (iv);
>        return true;
>      }
> --- gcc/testsuite/gfortran.dg/pr88964.f90.jj	2019-01-22 11:54:46.046154065 +0100
> +++ gcc/testsuite/gfortran.dg/pr88964.f90	2019-01-22 11:54:23.172526590 +0100
> @@ -0,0 +1,57 @@
> +! PR tree-optimization/88964
> +! { dg-do compile }
> +! { dg-options "-O3 -fno-tree-forwprop --param sccvn-max-alias-queries-per-access=1" }
> +
> +MODULE pr88964
> +  INTEGER, PARAMETER :: dp=8
> +  REAL(KIND=dp) :: p, q, o
> +CONTAINS
> +  SUBROUTINE foo(a,b,c,f,h)
> +    IMPLICIT NONE
> +    INTEGER :: a, b, c
> +    REAL(KIND=dp) :: f(b*c), h(a*c)
> +    CALL bar(h)
> +    CALL baz(f)
> +    CALL qux(h)
> +  END SUBROUTINE foo
> +  SUBROUTINE bar(h)
> +    IMPLICIT NONE
> +    REAL(KIND=dp) :: h(1*1)
> +    INTEGER :: r, s, t, u
> +    DO u = 1,3
> +      DO t = 1,1
> +        DO s = 1,3
> +          DO r = 1,1
> +            h((t-1)*1+r) = h((t-1)*1+r)-p*o
> +          END DO
> +        END DO
> +      END DO
> +    END DO
> +  END SUBROUTINE bar
> +  SUBROUTINE baz(f)
> +    IMPLICIT NONE
> +    REAL(KIND=dp) :: f(3*1)
> +    INTEGER :: s, t, u
> +    DO u = 1,4
> +      DO t = 1,1
> +        DO s = 1,3
> +          f((t-1)*3+s) = f((t-1)*3+s) - q
> +        END DO
> +      END DO
> +    END DO
> +  END SUBROUTINE baz
> +  SUBROUTINE qux(h)
> +    IMPLICIT NONE
> +    REAL(KIND=dp) :: h(1*1)
> +    INTEGER :: r, s, t, u
> +    DO u = 1,5
> +      DO t = 1,1
> +        DO s = 1,3
> +          DO r = 1,1
> +            h((t-1)*1+r) = h((t-1)*1+r)-p*o
> +          END DO
> +        END DO
> +      END DO
> +    END DO
> +  END SUBROUTINE qux
> +END MODULE pr88964
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964)
  2019-01-23  8:50 ` Richard Biener
@ 2019-01-24 19:22   ` Jakub Jelinek
  2019-01-24 19:40     ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-01-24 19:22 UTC (permalink / raw)
  To: Richard Biener; +Cc: Bin.Cheng, gcc-patches

On Wed, Jan 23, 2019 at 09:23:53AM +0100, Richard Biener wrote:
> On Tue, 22 Jan 2019, Jakub Jelinek wrote:
> 
> > Hi!
> > 
> > SCEV can analyze not just integral/pointer IVs, but (scalar) float ones as
> > well.  Calling build_int_cst on such types results in ICE, build_zero_cst
> > works.  Though the loop invariant PHI IVs, if we represent them as using
> > +0.0 step, aren't correct if honoring signed zeros, as + 0.0 will make
> > a +0.0 out of -0.0.
> > 
> > Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?
> 
> OK.

Thinking about it, if we honor SNaNs, we should punt too, even if we are not
honoring signed zeros, because sNaN + 0.0 artificially added there will
raise an exception, where previously the code could not raise it.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2019-01-24  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/88964
	* gimple-loop-interchange.cc (loop_cand::analyze_induction_var): Also
	punt if HONOR_SNANS (chrec).

--- gcc/gimple-loop-interchange.cc.jj	2019-01-23 09:34:53.739433699 +0100
+++ gcc/gimple-loop-interchange.cc	2019-01-23 12:53:35.215848790 +0100
@@ -690,8 +690,8 @@ loop_cand::analyze_induction_var (tree v
     {
       /* Punt on floating point invariants if honoring signed zeros,
 	 representing that as + 0.0 would change the result if init
-	 is -0.0.  */
-      if (HONOR_SIGNED_ZEROS (chrec))
+	 is -0.0.  Similarly for SNaNs it can raise exception.  */
+      if (HONOR_SIGNED_ZEROS (chrec) || HONOR_SNANS (chrec))
 	return false;
       struct induction *iv = XCNEW (struct induction);
       iv->var = var;


	Jakub

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964)
  2019-01-24 19:22   ` Jakub Jelinek
@ 2019-01-24 19:40     ` Richard Biener
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2019-01-24 19:40 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Bin.Cheng, gcc-patches

On January 24, 2019 8:19:11 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Wed, Jan 23, 2019 at 09:23:53AM +0100, Richard Biener wrote:
>> On Tue, 22 Jan 2019, Jakub Jelinek wrote:
>> 
>> > Hi!
>> > 
>> > SCEV can analyze not just integral/pointer IVs, but (scalar) float
>ones as
>> > well.  Calling build_int_cst on such types results in ICE,
>build_zero_cst
>> > works.  Though the loop invariant PHI IVs, if we represent them as
>using
>> > +0.0 step, aren't correct if honoring signed zeros, as + 0.0 will
>make
>> > a +0.0 out of -0.0.
>> > 
>> > Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok
>for trunk?
>> 
>> OK.
>
>Thinking about it, if we honor SNaNs, we should punt too, even if we
>are not
>honoring signed zeros, because sNaN + 0.0 artificially added there will
>raise an exception, where previously the code could not raise it.
>
>Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
>for
>trunk?

OK. 

Richard. 

>2019-01-24  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/88964
>	* gimple-loop-interchange.cc (loop_cand::analyze_induction_var): Also
>	punt if HONOR_SNANS (chrec).
>
>--- gcc/gimple-loop-interchange.cc.jj	2019-01-23 09:34:53.739433699
>+0100
>+++ gcc/gimple-loop-interchange.cc	2019-01-23 12:53:35.215848790 +0100
>@@ -690,8 +690,8 @@ loop_cand::analyze_induction_var (tree v
>     {
>       /* Punt on floating point invariants if honoring signed zeros,
> 	 representing that as + 0.0 would change the result if init
>-	 is -0.0.  */
>-      if (HONOR_SIGNED_ZEROS (chrec))
>+	 is -0.0.  Similarly for SNaNs it can raise exception.  */
>+      if (HONOR_SIGNED_ZEROS (chrec) || HONOR_SNANS (chrec))
> 	return false;
>       struct induction *iv = XCNEW (struct induction);
>       iv->var = var;
>
>
>	Jakub

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-01-24 19:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 22:29 [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964) Jakub Jelinek
2019-01-23  8:50 ` Richard Biener
2019-01-24 19:22   ` Jakub Jelinek
2019-01-24 19:40     ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).