diff mbox

Fix PR47691: always run scev_const_prop before graphite

Message ID CAFk3UF8RjnwhFN1rG+1o+SazwPDbS6dwwmg3JCD0=+6iqpPSaw@mail.gmail.com
State New
Headers show

Commit Message

Sebastian Pop July 26, 2011, 3:41 p.m. UTC
On Tue, Jul 26, 2011 at 09:33, Richard Guenther <rguenther@suse.de> wrote:
> 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.

Here is the patch.  Ok for trunk after regstrap?

Thanks,
Sebastian

Comments

Richard Biener July 26, 2011, 3:43 p.m. UTC | #1
On Tue, 26 Jul 2011, Sebastian Pop wrote:

> On Tue, Jul 26, 2011 at 09:33, Richard Guenther <rguenther@suse.de> wrote:
> > 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.
> 
> Here is the patch.  Ok for trunk after regstrap?

Required?  Do you mean you generate wrong code in graphite if it is
not run?  What if it just fails to handle things or does not handle
them because things are too expensive?

Please make graphite more robust instead.

Thanks,
Richard.
Sebastian Pop July 26, 2011, 3:56 p.m. UTC | #2
On Tue, Jul 26, 2011 at 10:43, Richard Guenther <rguenther@suse.de> wrote:
> Required?  Do you mean you generate wrong code in graphite if it is
> not run?

Graphite won't generate wrong code if scev_cprop is not run: it will
fail on an assert in the code generation part.  We used to know the
scev for the induction variable (in the scop detection), and after the
detection of reductions we just decided that the IV was a reduction
(its value is used after the loop in the loop close phi), so we decide
to rewrite the IV using arrays to expose this "reduction" to the
graphite representation.  Finally when we arrive in the code gen,
and asking again for the scev of the IV, we get a "dont_know", and
so we are puzzled and abort.

> What if it just fails to handle things or does not handle
> them because things are too expensive?
>
> Please make graphite more robust instead.

Ok, in this case, what about setting gloog_error and stopping the code
generation instead of failing on this gcc_assert.

Thanks for your insistence ;-)

Sebastian
diff mbox

Patch

From 5c1f59fa7e9c5fb5e5960975153cd040d51baab2 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Sat, 23 Jul 2011 23:29:30 -0500
Subject: [PATCH] Fix PR47691: do not run graphite if scev_const_prop has not run before

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

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

	* gfortran.dg/graphite/id-pr47691.f: New.
---
 gcc/ChangeLog                                   |    6 ++++++
 gcc/graphite.c                                  |    4 ++++
 gcc/testsuite/ChangeLog                         |    5 +++++
 gcc/testsuite/gfortran.dg/graphite/id-pr47691.f |    7 +++++++
 4 files changed, 22 insertions(+), 0 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/graphite/id-pr47691.f

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9cfa21b..abb5f77 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): Return false 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..caba926 100644
--- a/gcc/graphite.c
+++ b/gcc/graphite.c
@@ -191,6 +191,10 @@  graphite_initialize (void)
   int ppl_initialized;
 
   if (number_of_loops () <= 1
+
+      /* scev constant propagation is required for Graphite.  */
+      || !flag_tree_scev_cprop
+
       /* FIXME: This limit on the number of basic blocks of a function
 	 should be removed when the SCOP detection is faster.  */
       || n_basic_blocks > PARAM_VALUE (PARAM_GRAPHITE_MAX_BBS_PER_FUNCTION))
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