public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR79622
@ 2017-09-18  7:31 Richard Biener
  2017-09-19 15:54 ` Tamar Christina
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2017-09-18  7:31 UTC (permalink / raw)
  To: gcc-patches


The following patch fixes the other known wrong-code bug in GRAPHITE
which shows we're mishandling PHIs in not properly considering the
edge copies they represent as living outside of the black-box we're
analyzing.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Note the testcase still fails with ISL 0.16.1 but passes with 0.18
for me.  I'll update the version in download_prerequesites to 0.18.

Richard.

2017-09-18  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/79622
	* graphite-scop-detection.c (build_cross_bb_scalars_def): Properly
	handle PHIs.
	(build_cross_bb_scalars_use): Likewise.

	* gcc.dg/graphite/pr79622.c: New testcase.

Index: gcc/graphite-scop-detection.c
===================================================================
--- gcc/graphite-scop-detection.c	(revision 252806)
+++ gcc/graphite-scop-detection.c	(working copy)
@@ -1744,7 +1744,9 @@ build_cross_bb_scalars_def (scop_p scop,
   gimple *use_stmt;
   imm_use_iterator imm_iter;
   FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, def)
-    if (def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
+    if ((def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
+	/* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
+	|| gimple_code (SSA_NAME_DEF_STMT (def)) == GIMPLE_PHI)
       {
 	writes->safe_push (def);
 	DEBUG_PRINT (dp << "Adding scalar write: ";
@@ -1758,7 +1760,8 @@ build_cross_bb_scalars_def (scop_p scop,
       }
 }
 
-/* Record DEF if it is used in other bbs different than DEF_BB in the SCOP.  */
+/* Record USE if it is defined in other bbs different than USE_STMT
+   in the SCOP.  */
 
 static void
 build_cross_bb_scalars_use (scop_p scop, tree use, gimple *use_stmt,
@@ -1774,7 +1777,9 @@ build_cross_bb_scalars_use (scop_p scop,
     return;
 
   gimple *def_stmt = SSA_NAME_DEF_STMT (use);
-  if (gimple_bb (def_stmt) != gimple_bb (use_stmt))
+  if (gimple_bb (def_stmt) != gimple_bb (use_stmt)
+      /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
+      || gimple_code (def_stmt) == GIMPLE_PHI)
     {
       DEBUG_PRINT (dp << "Adding scalar read: ";
 		   print_generic_expr (dump_file, use);
Index: gcc/testsuite/gcc.dg/graphite/pr79622.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/pr79622.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/graphite/pr79622.c	(working copy)
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -floop-nest-optimize" } */
+
+int bf;
+
+int
+main (void)
+{
+  int dc[5];
+
+  for (bf = 0; bf < 2; ++bf)
+    {
+      int l9, g5 = -1;
+
+      for (l9 = 0; l9 < 5; ++l9)
+	{
+	  dc[l9] = g5;
+	  g5 = (dc[l9] > 0);
+	}
+    }
+
+  if (dc[0] != -1)
+    __builtin_abort ();
+
+  return 0;
+}

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

* Re: [PATCH] Fix PR79622
  2017-09-18  7:31 [PATCH] Fix PR79622 Richard Biener
@ 2017-09-19 15:54 ` Tamar Christina
  2017-09-20 16:52   ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Tamar Christina @ 2017-09-19 15:54 UTC (permalink / raw)
  To: Richard Biener, gcc-patches; +Cc: nd

-- sorry for the duplicate, forgot to post to list as well first time --

Hi Richard,

The testcase seems to fail on aarch64-none-elf when -O1 or -O2,

-O0, -Os and -O3 seem to work fine.

dc[0] ends up being 0 for the cases that fail.

Kind regards,
Tamar
________________________________________
From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Richard Biener <rguenther@suse.de>
Sent: Monday, September 18, 2017 8:31 AM
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] Fix PR79622

The following patch fixes the other known wrong-code bug in GRAPHITE
which shows we're mishandling PHIs in not properly considering the
edge copies they represent as living outside of the black-box we're
analyzing.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Note the testcase still fails with ISL 0.16.1 but passes with 0.18
for me.  I'll update the version in download_prerequesites to 0.18.

Richard.

2017-09-18  Richard Biener  <rguenther@suse.de>

        PR tree-optimization/79622
        * graphite-scop-detection.c (build_cross_bb_scalars_def): Properly
        handle PHIs.
        (build_cross_bb_scalars_use): Likewise.

        * gcc.dg/graphite/pr79622.c: New testcase.

Index: gcc/graphite-scop-detection.c
===================================================================
--- gcc/graphite-scop-detection.c       (revision 252806)
+++ gcc/graphite-scop-detection.c       (working copy)
@@ -1744,7 +1744,9 @@ build_cross_bb_scalars_def (scop_p scop,
   gimple *use_stmt;
   imm_use_iterator imm_iter;
   FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, def)
-    if (def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
+    if ((def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
+       /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
+       || gimple_code (SSA_NAME_DEF_STMT (def)) == GIMPLE_PHI)
       {
        writes->safe_push (def);
        DEBUG_PRINT (dp << "Adding scalar write: ";
@@ -1758,7 +1760,8 @@ build_cross_bb_scalars_def (scop_p scop,
       }
 }

-/* Record DEF if it is used in other bbs different than DEF_BB in the SCOP.  */
+/* Record USE if it is defined in other bbs different than USE_STMT
+   in the SCOP.  */

 static void
 build_cross_bb_scalars_use (scop_p scop, tree use, gimple *use_stmt,
@@ -1774,7 +1777,9 @@ build_cross_bb_scalars_use (scop_p scop,
     return;

   gimple *def_stmt = SSA_NAME_DEF_STMT (use);
-  if (gimple_bb (def_stmt) != gimple_bb (use_stmt))
+  if (gimple_bb (def_stmt) != gimple_bb (use_stmt)
+      /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
+      || gimple_code (def_stmt) == GIMPLE_PHI)
     {
       DEBUG_PRINT (dp << "Adding scalar read: ";
                   print_generic_expr (dump_file, use);
Index: gcc/testsuite/gcc.dg/graphite/pr79622.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/pr79622.c     (nonexistent)
+++ gcc/testsuite/gcc.dg/graphite/pr79622.c     (working copy)
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -floop-nest-optimize" } */
+
+int bf;
+
+int
+main (void)
+{
+  int dc[5];
+
+  for (bf = 0; bf < 2; ++bf)
+    {
+      int l9, g5 = -1;
+
+      for (l9 = 0; l9 < 5; ++l9)
+       {
+         dc[l9] = g5;
+         g5 = (dc[l9] > 0);
+       }
+    }
+
+  if (dc[0] != -1)
+    __builtin_abort ();
+
+  return 0;
+}

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

* Re: [PATCH] Fix PR79622
  2017-09-19 15:54 ` Tamar Christina
@ 2017-09-20 16:52   ` Richard Biener
  2017-09-21 10:39     ` Tamar Christina
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2017-09-20 16:52 UTC (permalink / raw)
  To: Tamar Christina; +Cc: Richard Biener, gcc-patches, nd

On Tue, Sep 19, 2017 at 5:54 PM, Tamar Christina
<Tamar.Christina@arm.com> wrote:
> -- sorry for the duplicate, forgot to post to list as well first time --
>
> Hi Richard,
>
> The testcase seems to fail on aarch64-none-elf when -O1 or -O2,
>
> -O0, -Os and -O3 seem to work fine.
>
> dc[0] ends up being 0 for the cases that fail.

What ISL version are you using?

Richard.

> Kind regards,
> Tamar
> ________________________________________
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Richard Biener <rguenther@suse.de>
> Sent: Monday, September 18, 2017 8:31 AM
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH] Fix PR79622
>
> The following patch fixes the other known wrong-code bug in GRAPHITE
> which shows we're mishandling PHIs in not properly considering the
> edge copies they represent as living outside of the black-box we're
> analyzing.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> Note the testcase still fails with ISL 0.16.1 but passes with 0.18
> for me.  I'll update the version in download_prerequesites to 0.18.
>
> Richard.
>
> 2017-09-18  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/79622
>         * graphite-scop-detection.c (build_cross_bb_scalars_def): Properly
>         handle PHIs.
>         (build_cross_bb_scalars_use): Likewise.
>
>         * gcc.dg/graphite/pr79622.c: New testcase.
>
> Index: gcc/graphite-scop-detection.c
> ===================================================================
> --- gcc/graphite-scop-detection.c       (revision 252806)
> +++ gcc/graphite-scop-detection.c       (working copy)
> @@ -1744,7 +1744,9 @@ build_cross_bb_scalars_def (scop_p scop,
>    gimple *use_stmt;
>    imm_use_iterator imm_iter;
>    FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, def)
> -    if (def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
> +    if ((def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
> +       /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
> +       || gimple_code (SSA_NAME_DEF_STMT (def)) == GIMPLE_PHI)
>        {
>         writes->safe_push (def);
>         DEBUG_PRINT (dp << "Adding scalar write: ";
> @@ -1758,7 +1760,8 @@ build_cross_bb_scalars_def (scop_p scop,
>        }
>  }
>
> -/* Record DEF if it is used in other bbs different than DEF_BB in the SCOP.  */
> +/* Record USE if it is defined in other bbs different than USE_STMT
> +   in the SCOP.  */
>
>  static void
>  build_cross_bb_scalars_use (scop_p scop, tree use, gimple *use_stmt,
> @@ -1774,7 +1777,9 @@ build_cross_bb_scalars_use (scop_p scop,
>      return;
>
>    gimple *def_stmt = SSA_NAME_DEF_STMT (use);
> -  if (gimple_bb (def_stmt) != gimple_bb (use_stmt))
> +  if (gimple_bb (def_stmt) != gimple_bb (use_stmt)
> +      /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
> +      || gimple_code (def_stmt) == GIMPLE_PHI)
>      {
>        DEBUG_PRINT (dp << "Adding scalar read: ";
>                    print_generic_expr (dump_file, use);
> Index: gcc/testsuite/gcc.dg/graphite/pr79622.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/graphite/pr79622.c     (nonexistent)
> +++ gcc/testsuite/gcc.dg/graphite/pr79622.c     (working copy)
> @@ -0,0 +1,26 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -floop-nest-optimize" } */
> +
> +int bf;
> +
> +int
> +main (void)
> +{
> +  int dc[5];
> +
> +  for (bf = 0; bf < 2; ++bf)
> +    {
> +      int l9, g5 = -1;
> +
> +      for (l9 = 0; l9 < 5; ++l9)
> +       {
> +         dc[l9] = g5;
> +         g5 = (dc[l9] > 0);
> +       }
> +    }
> +
> +  if (dc[0] != -1)
> +    __builtin_abort ();
> +
> +  return 0;
> +}

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

* RE: [PATCH] Fix PR79622
  2017-09-20 16:52   ` Richard Biener
@ 2017-09-21 10:39     ` Tamar Christina
  0 siblings, 0 replies; 4+ messages in thread
From: Tamar Christina @ 2017-09-21 10:39 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches, nd



> -----Original Message-----
> From: Richard Biener [mailto:richard.guenther@gmail.com]
> Sent: 20 September 2017 17:52
> To: Tamar Christina
> Cc: Richard Biener; gcc-patches@gcc.gnu.org; nd
> Subject: Re: [PATCH] Fix PR79622
> 
> On Tue, Sep 19, 2017 at 5:54 PM, Tamar Christina
> <Tamar.Christina@arm.com> wrote:
> > -- sorry for the duplicate, forgot to post to list as well first time
> > --
> >
> > Hi Richard,
> >
> > The testcase seems to fail on aarch64-none-elf when -O1 or -O2,
> >
> > -O0, -Os and -O3 seem to work fine.
> >
> > dc[0] ends up being 0 for the cases that fail.
> 
> What ISL version are you using?

Ah, it seems we had it set to 0.15.

Sorry for the noise,
Tamar

> 
> Richard.
> 
> > Kind regards,
> > Tamar
> > ________________________________________
> > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> on
> > behalf of Richard Biener <rguenther@suse.de>
> > Sent: Monday, September 18, 2017 8:31 AM
> > To: gcc-patches@gcc.gnu.org
> > Subject: [PATCH] Fix PR79622
> >
> > The following patch fixes the other known wrong-code bug in GRAPHITE
> > which shows we're mishandling PHIs in not properly considering the
> > edge copies they represent as living outside of the black-box we're
> > analyzing.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> >
> > Note the testcase still fails with ISL 0.16.1 but passes with 0.18 for
> > me.  I'll update the version in download_prerequesites to 0.18.
> >
> > Richard.
> >
> > 2017-09-18  Richard Biener  <rguenther@suse.de>
> >
> >         PR tree-optimization/79622
> >         * graphite-scop-detection.c (build_cross_bb_scalars_def): Properly
> >         handle PHIs.
> >         (build_cross_bb_scalars_use): Likewise.
> >
> >         * gcc.dg/graphite/pr79622.c: New testcase.
> >
> > Index: gcc/graphite-scop-detection.c
> >
> ==========================================================
> =========
> > --- gcc/graphite-scop-detection.c       (revision 252806)
> > +++ gcc/graphite-scop-detection.c       (working copy)
> > @@ -1744,7 +1744,9 @@ build_cross_bb_scalars_def (scop_p scop,
> >    gimple *use_stmt;
> >    imm_use_iterator imm_iter;
> >    FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, def)
> > -    if (def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
> > +    if ((def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))
> > +       /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
> > +       || gimple_code (SSA_NAME_DEF_STMT (def)) == GIMPLE_PHI)
> >        {
> >         writes->safe_push (def);
> >         DEBUG_PRINT (dp << "Adding scalar write: "; @@ -1758,7 +1760,8
> > @@ build_cross_bb_scalars_def (scop_p scop,
> >        }
> >  }
> >
> > -/* Record DEF if it is used in other bbs different than DEF_BB in the
> > SCOP.  */
> > +/* Record USE if it is defined in other bbs different than USE_STMT
> > +   in the SCOP.  */
> >
> >  static void
> >  build_cross_bb_scalars_use (scop_p scop, tree use, gimple *use_stmt,
> > @@ -1774,7 +1777,9 @@ build_cross_bb_scalars_use (scop_p scop,
> >      return;
> >
> >    gimple *def_stmt = SSA_NAME_DEF_STMT (use);
> > -  if (gimple_bb (def_stmt) != gimple_bb (use_stmt))
> > +  if (gimple_bb (def_stmt) != gimple_bb (use_stmt)
> > +      /* PHIs have their effect at "BBs" on the edges.  See PR79622.  */
> > +      || gimple_code (def_stmt) == GIMPLE_PHI)
> >      {
> >        DEBUG_PRINT (dp << "Adding scalar read: ";
> >                    print_generic_expr (dump_file, use);
> > Index: gcc/testsuite/gcc.dg/graphite/pr79622.c
> >
> ==========================================================
> =========
> > --- gcc/testsuite/gcc.dg/graphite/pr79622.c     (nonexistent)
> > +++ gcc/testsuite/gcc.dg/graphite/pr79622.c     (working copy)
> > @@ -0,0 +1,26 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-O2 -floop-nest-optimize" } */
> > +
> > +int bf;
> > +
> > +int
> > +main (void)
> > +{
> > +  int dc[5];
> > +
> > +  for (bf = 0; bf < 2; ++bf)
> > +    {
> > +      int l9, g5 = -1;
> > +
> > +      for (l9 = 0; l9 < 5; ++l9)
> > +       {
> > +         dc[l9] = g5;
> > +         g5 = (dc[l9] > 0);
> > +       }
> > +    }
> > +
> > +  if (dc[0] != -1)
> > +    __builtin_abort ();
> > +
> > +  return 0;
> > +}

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

end of thread, other threads:[~2017-09-21 10:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-18  7:31 [PATCH] Fix PR79622 Richard Biener
2017-09-19 15:54 ` Tamar Christina
2017-09-20 16:52   ` Richard Biener
2017-09-21 10:39     ` Tamar Christina

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).