Patchwork Fix PR47691: always run scev_const_prop before graphite

login
register
mail settings
Submitter Sebastian Pop
Date July 24, 2011, 4:40 a.m.
Message ID <1311482408-22717-1-git-send-email-sebpop@gmail.com>
Download mbox | patch
Permalink /patch/106505/
State New
Headers show

Comments

Sebastian Pop - July 24, 2011, 4:40 a.m.
This patch makes graphite run the scev_const_prop systematically even
when using -fno-tree-scev-cprop.  When scev_const_prop is not applied,
there exist close_phi nodes for the main induction variable, making it
impossible for graphite to distinguish between reductions and the IVs.
So the main IV is translated as it was a reduction, i.e., using copies
into temporary arrays, and that makes the scev analysis impossible
during code generation.

Bootstrapped and tested on amd64-linux.

2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>

	PR middle-end/47691
	* graphite.c (graphite_initialize): Call scev_const_prop when
	flag_tree_scev_cprop is not set.

	* gfortran.dg/graphite/id-pr47691.f: New.
---
 gcc/ChangeLog                                   |    6 ++++++
 gcc/graphite.c                                  |    3 +++
 gcc/testsuite/ChangeLog                         |    5 +++++
 gcc/testsuite/gfortran.dg/graphite/id-pr47691.f |    7 +++++++
 4 files changed, 21 insertions(+), 0 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
Sebastian Pop - July 26, 2011, 2:24 p.m.
Ping.
Any opinions on this patch?

Thanks,
Sebastian

On Sat, Jul 23, 2011 at 23:40, Sebastian Pop <sebpop@gmail.com> wrote:
> This patch makes graphite run the scev_const_prop systematically even
> when using -fno-tree-scev-cprop.  When scev_const_prop is not applied,
> there exist close_phi nodes for the main induction variable, making it
> impossible for graphite to distinguish between reductions and the IVs.
> So the main IV is translated as it was a reduction, i.e., using copies
> into temporary arrays, and that makes the scev analysis impossible
> during code generation.
>
> Bootstrapped and tested on amd64-linux.
>
> 2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
>
>        PR middle-end/47691
>        * graphite.c (graphite_initialize): Call scev_const_prop when
>        flag_tree_scev_cprop is not set.
>
>        * gfortran.dg/graphite/id-pr47691.f: New.
> ---
>  gcc/ChangeLog                                   |    6 ++++++
>  gcc/graphite.c                                  |    3 +++
>  gcc/testsuite/ChangeLog                         |    5 +++++
>  gcc/testsuite/gfortran.dg/graphite/id-pr47691.f |    7 +++++++
>  4 files changed, 21 insertions(+), 0 deletions(-)
>  create mode 100644 gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 9cfa21b..36347d6 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
> +
> +       PR middle-end/47691
> +       * graphite.c (graphite_initialize): Call scev_const_prop when
> +       flag_tree_scev_cprop is not set.
> +
>  2011-07-21  Sebastian Pop  <sebastian.pop@amd.com>
>
>        PR middle-end/47654
> diff --git a/gcc/graphite.c b/gcc/graphite.c
> index b013447..dfe9ca7 100644
> --- a/gcc/graphite.c
> +++ b/gcc/graphite.c
> @@ -201,6 +201,9 @@ graphite_initialize (void)
>       return false;
>     }
>
> +  if (!flag_tree_scev_cprop)
> +    scev_const_prop ();
> +
>   scev_reset ();
>   recompute_all_dominators ();
>   initialize_original_copy_tables ();
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index a63b647..5f9b79d 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
> +
> +       PR middle-end/47691
> +       * gfortran.dg/graphite/id-pr47691.f: New.
> +
>  2011-07-21  Sebastian Pop  <sebastian.pop@amd.com>
>
>        PR middle-end/47654
> diff --git a/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f b/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
> new file mode 100644
> index 0000000..0abbd55
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
> @@ -0,0 +1,7 @@
> +! { dg-options "-O -fgraphite-identity -ffast-math -fno-tree-scev-cprop" }
> +      dimension b(12,8)
> +      do i=1,norb
> +      end do
> +      b(i,j) = 0
> +      call rdrsym(b)
> +      end
> --
> 1.7.4.1
>
>
Richard Guenther - July 26, 2011, 2:33 p.m.
On Tue, 26 Jul 2011, Sebastian Pop wrote:

> Ping.
> Any opinions on this patch?

Well, I don't think we should do this.  Why does the user disable
scev-const-prop when enabling graphite?  If he does so, fine - he
has to live with worse codegen.

Richard.

> Thanks,
> Sebastian
> 
> On Sat, Jul 23, 2011 at 23:40, Sebastian Pop <sebpop@gmail.com> wrote:
> > This patch makes graphite run the scev_const_prop systematically even
> > when using -fno-tree-scev-cprop.  When scev_const_prop is not applied,
> > there exist close_phi nodes for the main induction variable, making it
> > impossible for graphite to distinguish between reductions and the IVs.
> > So the main IV is translated as it was a reduction, i.e., using copies
> > into temporary arrays, and that makes the scev analysis impossible
> > during code generation.
> >
> > Bootstrapped and tested on amd64-linux.
> >
> > 2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
> >
> >        PR middle-end/47691
> >        * graphite.c (graphite_initialize): Call scev_const_prop when
> >        flag_tree_scev_cprop is not set.
> >
> >        * gfortran.dg/graphite/id-pr47691.f: New.
> > ---
> >  gcc/ChangeLog                                   |    6 ++++++
> >  gcc/graphite.c                                  |    3 +++
> >  gcc/testsuite/ChangeLog                         |    5 +++++
> >  gcc/testsuite/gfortran.dg/graphite/id-pr47691.f |    7 +++++++
> >  4 files changed, 21 insertions(+), 0 deletions(-)
> >  create mode 100644 gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
> >
> > diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> > index 9cfa21b..36347d6 100644
> > --- a/gcc/ChangeLog
> > +++ b/gcc/ChangeLog
> > @@ -1,3 +1,9 @@
> > +2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
> > +
> > +       PR middle-end/47691
> > +       * graphite.c (graphite_initialize): Call scev_const_prop when
> > +       flag_tree_scev_cprop is not set.
> > +
> >  2011-07-21  Sebastian Pop  <sebastian.pop@amd.com>
> >
> >        PR middle-end/47654
> > diff --git a/gcc/graphite.c b/gcc/graphite.c
> > index b013447..dfe9ca7 100644
> > --- a/gcc/graphite.c
> > +++ b/gcc/graphite.c
> > @@ -201,6 +201,9 @@ graphite_initialize (void)
> >       return false;
> >     }
> >
> > +  if (!flag_tree_scev_cprop)
> > +    scev_const_prop ();
> > +
> >   scev_reset ();
> >   recompute_all_dominators ();
> >   initialize_original_copy_tables ();
> > diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> > index a63b647..5f9b79d 100644
> > --- a/gcc/testsuite/ChangeLog
> > +++ b/gcc/testsuite/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
> > +
> > +       PR middle-end/47691
> > +       * gfortran.dg/graphite/id-pr47691.f: New.
> > +
> >  2011-07-21  Sebastian Pop  <sebastian.pop@amd.com>
> >
> >        PR middle-end/47654
> > diff --git a/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f b/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
> > new file mode 100644
> > index 0000000..0abbd55
> > --- /dev/null
> > +++ b/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
> > @@ -0,0 +1,7 @@
> > +! { dg-options "-O -fgraphite-identity -ffast-math -fno-tree-scev-cprop" }
> > +      dimension b(12,8)
> > +      do i=1,norb
> > +      end do
> > +      b(i,j) = 0
> > +      call rdrsym(b)
> > +      end
> > --
> > 1.7.4.1
> >
> >
> 
>

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9cfa21b..36347d6 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
+
+	PR middle-end/47691
+	* graphite.c (graphite_initialize): Call scev_const_prop when
+	flag_tree_scev_cprop is not set.
+
 2011-07-21  Sebastian Pop  <sebastian.pop@amd.com>
 
 	PR middle-end/47654
diff --git a/gcc/graphite.c b/gcc/graphite.c
index b013447..dfe9ca7 100644
--- a/gcc/graphite.c
+++ b/gcc/graphite.c
@@ -201,6 +201,9 @@  graphite_initialize (void)
       return false;
     }
 
+  if (!flag_tree_scev_cprop)
+    scev_const_prop ();
+
   scev_reset ();
   recompute_all_dominators ();
   initialize_original_copy_tables ();
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a63b647..5f9b79d 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2011-07-23  Sebastian Pop  <sebastian.pop@amd.com>
+
+	PR middle-end/47691
+	* gfortran.dg/graphite/id-pr47691.f: New.
+
 2011-07-21  Sebastian Pop  <sebastian.pop@amd.com>
 
 	PR middle-end/47654
diff --git a/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f b/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
new file mode 100644
index 0000000..0abbd55
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/graphite/id-pr47691.f
@@ -0,0 +1,7 @@ 
+! { dg-options "-O -fgraphite-identity -ffast-math -fno-tree-scev-cprop" }
+      dimension b(12,8)
+      do i=1,norb
+      end do
+      b(i,j) = 0
+      call rdrsym(b)
+      end