diff mbox

fix PR67700

Message ID 1443213563-16853-1-git-send-email-s.pop@samsung.com
State New
Headers show

Commit Message

Sebastian Pop Sept. 25, 2015, 8:39 p.m. UTC
The patch makes the detection of scop parameters in parameter_index_in_region a
bit more conservative by discarding scalar variables defined in function of data
references defined in the scop.

2015-09-25  Aditya Kumar  <aditya.k7@samsung.com>
                Sebastian Pop  <s.pop@samsung.com>

                PR tree-optimization/67700
                * graphite-sese-to-poly.c (parameter_index_in_region): Call
                invariant_in_sese_p_rec.
                (extract_affine): Same.
                (rewrite_cross_bb_scalar_deps): Call update_ssa.
                * sese.c (invariant_in_sese_p_rec): Export.  Handle vdefs and vuses.
                * sese.h (invariant_in_sese_p_rec): Declare.

                * testsuite/gcc.dg/graphite/run-id-pr67700.c: New.
---
 gcc/graphite-sese-to-poly.c                    |  8 +++++-
 gcc/sese.c                                     | 14 ++++++++--
 gcc/sese.h                                     |  1 +
 gcc/testsuite/gcc.dg/graphite/run-id-pr67700.c | 36 ++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr67700.c

Comments

Tobias Grosser Sept. 26, 2015, 10:34 a.m. UTC | #1
On 09/25/2015 10:39 PM, Sebastian Pop wrote:
> The patch makes the detection of scop parameters in parameter_index_in_region a
> bit more conservative by discarding scalar variables defined in function of data
> references defined in the scop.
>
> 2015-09-25  Aditya Kumar  <aditya.k7@samsung.com>
>                  Sebastian Pop  <s.pop@samsung.com>
>
>                  PR tree-optimization/67700
>                  * graphite-sese-to-poly.c (parameter_index_in_region): Call
>                  invariant_in_sese_p_rec.
>                  (extract_affine): Same.
>                  (rewrite_cross_bb_scalar_deps): Call update_ssa.
>                  * sese.c (invariant_in_sese_p_rec): Export.  Handle vdefs and vuses.
>                  * sese.h (invariant_in_sese_p_rec): Declare.
>
>                  * testsuite/gcc.dg/graphite/run-id-pr67700.c: New.

LGTM.

Tobias
H.J. Lu Sept. 28, 2015, 7:39 p.m. UTC | #2
On Sat, Sep 26, 2015 at 3:34 AM, Tobias Grosser <tobias@grosser.es> wrote:
> On 09/25/2015 10:39 PM, Sebastian Pop wrote:
>>
>> The patch makes the detection of scop parameters in
>> parameter_index_in_region a
>> bit more conservative by discarding scalar variables defined in function
>> of data
>> references defined in the scop.
>>
>> 2015-09-25  Aditya Kumar  <aditya.k7@samsung.com>
>>                  Sebastian Pop  <s.pop@samsung.com>
>>
>>                  PR tree-optimization/67700
>>                  * graphite-sese-to-poly.c (parameter_index_in_region):
>> Call
>>                  invariant_in_sese_p_rec.
>>                  (extract_affine): Same.
>>                  (rewrite_cross_bb_scalar_deps): Call update_ssa.
>>                  * sese.c (invariant_in_sese_p_rec): Export.  Handle vdefs
>> and vuses.
>>                  * sese.h (invariant_in_sese_p_rec): Declare.
>>
>>                  * testsuite/gcc.dg/graphite/run-id-pr67700.c: New.

It breaks bootstrap on x86:

https://gcc.gnu.org/ml/gcc-regression/2015-09/msg00382.html

../../src-trunk/gcc/sese.c: In function âbool
invariant_in_sese_p_rec(tree, sese)â:
../../src-trunk/gcc/sese.c:781:12: error: unused variable âvdefâ
[-Werror=unused-variable]
   if (tree vdef = gimple_vdef (stmt))
            ^
Sebastian Pop Sept. 28, 2015, 10:48 p.m. UTC | #3
I fixed this in a follow-up patch.

Sebastian

-----Original Message-----
From: H.J. Lu [mailto:hjl.tools@gmail.com] 
Sent: Monday, September 28, 2015 2:39 PM
To: Tobias Grosser
Cc: Sebastian Pop; GCC Patches; Sebastian Pop; aditya.k7@samsung.com; Richard Biener
Subject: Re: [PATCH] fix PR67700

On Sat, Sep 26, 2015 at 3:34 AM, Tobias Grosser <tobias@grosser.es> wrote:
> On 09/25/2015 10:39 PM, Sebastian Pop wrote:
>>
>> The patch makes the detection of scop parameters in
>> parameter_index_in_region a
>> bit more conservative by discarding scalar variables defined in function
>> of data
>> references defined in the scop.
>>
>> 2015-09-25  Aditya Kumar  <aditya.k7@samsung.com>
>>                  Sebastian Pop  <s.pop@samsung.com>
>>
>>                  PR tree-optimization/67700
>>                  * graphite-sese-to-poly.c (parameter_index_in_region):
>> Call
>>                  invariant_in_sese_p_rec.
>>                  (extract_affine): Same.
>>                  (rewrite_cross_bb_scalar_deps): Call update_ssa.
>>                  * sese.c (invariant_in_sese_p_rec): Export.  Handle vdefs
>> and vuses.
>>                  * sese.h (invariant_in_sese_p_rec): Declare.
>>
>>                  * testsuite/gcc.dg/graphite/run-id-pr67700.c: New.

It breaks bootstrap on x86:

https://gcc.gnu.org/ml/gcc-regression/2015-09/msg00382.html

../../src-trunk/gcc/sese.c: In function âbool
invariant_in_sese_p_rec(tree, sese)â:
../../src-trunk/gcc/sese.c:781:12: error: unused variable âvdefâ
[-Werror=unused-variable]
   if (tree vdef = gimple_vdef (stmt))
            ^
H.J. Lu Sept. 28, 2015, 11:57 p.m. UTC | #4
On Mon, Sep 28, 2015 at 3:48 PM, Sebastian Paul Pop <s.pop@samsung.com> wrote:
> I fixed this in a follow-up patch.
>

Now I got

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67754

FAIL: gcc.dg/graphite/interchange-10.c execution test
FAIL: gcc.dg/graphite/interchange-11.c execution test
FAIL: gcc.dg/graphite/interchange-1.c execution test
FAIL: gcc.dg/graphite/interchange-3.c execution test
FAIL: gcc.dg/graphite/interchange-4.c execution test
FAIL: gcc.dg/graphite/interchange-7.c execution test
FAIL: gcc.dg/graphite/pr46185.c execution test
FAIL: gcc.dg/graphite/uns-block-1.c execution test
FAIL: gcc.dg/graphite/uns-interchange-12.c execution test
FAIL: gcc.dg/graphite/uns-interchange-14.c execution test
FAIL: gcc.dg/graphite/uns-interchange-15.c execution test
FAIL: gcc.dg/graphite/uns-interchange-9.c execution test
FAIL: gcc.dg/graphite/uns-interchange-mvt.c execution test
FAIL: gfortran.dg/graphite/block-1.f90   -O  (internal compiler error)
FAIL: gfortran.dg/graphite/block-1.f90   -O  (test for excess errors)

on Linux/x86 with ISL 0.14.
diff mbox

Patch

diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index 09a2f91..3b8dd56 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -758,6 +758,9 @@  parameter_index_in_region (tree name, sese region)
   if (TREE_CODE (TREE_TYPE (name)) != INTEGER_TYPE)
     return -1;
 
+  if (!invariant_in_sese_p_rec (name, region))
+    return -1;
+
   i = parameter_index_in_region_1 (name, region);
   if (i != -1)
     return i;
@@ -813,7 +816,8 @@  extract_affine (scop_p s, tree e, __isl_take isl_space *space)
       break;
 
     case SSA_NAME:
-      gcc_assert (-1 != parameter_index_in_region_1 (e, SCOP_REGION (s)));
+      gcc_assert (-1 != parameter_index_in_region_1 (e, s->region)
+		  || !invariant_in_sese_p_rec (e, s->region));
       res = extract_affine_name (s, e, space);
       break;
 
@@ -2462,6 +2466,8 @@  rewrite_cross_bb_scalar_deps (scop_p scop, gimple_stmt_iterator *gsi)
 					    def, use_stmt);
       }
 
+  update_ssa (TODO_update_ssa);
+
   return res;
 }
 
diff --git a/gcc/sese.c b/gcc/sese.c
index db8c629..2050e2d 100644
--- a/gcc/sese.c
+++ b/gcc/sese.c
@@ -760,9 +760,10 @@  set_ifsese_condition (ifsese if_region, tree condition)
   gsi_insert_after (&gsi, cond_stmt, GSI_NEW_STMT);
 }
 
-/* Return false if T is completely defined outside REGION.  */
+/* Return true when T is defined outside REGION or when no definitions are
+   variant in REGION.  */
 
-static bool
+bool
 invariant_in_sese_p_rec (tree t, sese region)
 {
   ssa_op_iter iter;
@@ -776,9 +777,18 @@  invariant_in_sese_p_rec (tree t, sese region)
       || gimple_code (stmt) == GIMPLE_CALL)
     return false;
 
+  /* VDEF is variant when it is in the region.  */
+  if (tree vdef = gimple_vdef (stmt))
+    return false;
+
+  /* A VUSE may or may not be variant following the VDEFs.  */
+  if (tree vuse = gimple_vuse (stmt))
+    return invariant_in_sese_p_rec (vuse, region);
+
   FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_USE)
     {
       tree use = USE_FROM_PTR (use_p);
+
       if (!defined_in_sese_p (use, region))
 	continue;
 
diff --git a/gcc/sese.h b/gcc/sese.h
index 6ce5cc8..0d13d87 100644
--- a/gcc/sese.h
+++ b/gcc/sese.h
@@ -64,6 +64,7 @@  extern edge copy_bb_and_scalar_dependences (basic_block, sese, edge,
 					    vec<tree> , bool *);
 extern struct loop *outermost_loop_in_sese (sese, basic_block);
 extern tree scalar_evolution_in_region (sese, loop_p, tree);
+extern bool invariant_in_sese_p_rec (tree, sese);
 
 /* Check that SESE contains LOOP.  */
 
diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-pr67700.c b/gcc/testsuite/gcc.dg/graphite/run-id-pr67700.c
new file mode 100644
index 0000000..81d9e7f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/run-id-pr67700.c
@@ -0,0 +1,36 @@ 
+#include <stdlib.h>
+#include <assert.h>
+
+struct abc {
+  int a[81];
+} *abcd;
+
+#define FPMATH_SSE 2
+int global;
+
+void __attribute__ ((noinline)) foo()
+{
+  int pos = 0;
+  int i;
+
+  if (!((global & FPMATH_SSE) != 0))
+    for (i = 8; i <= 15; i++)
+      abcd->a[pos++] = i;
+
+  for (i = 29; i <= 36; i++)
+    abcd->a[pos++] = i;
+}
+
+int main()
+{
+  int i;
+  abcd = (struct abc*) malloc (sizeof (struct abc));
+  for (i = 0; i <= 80; i++)
+    abcd->a[i] = 0;
+
+  foo();
+
+  assert (abcd->a[8] == 29);
+
+  return 0;
+}