Patchwork Fix bug 59586

login
register
mail settings
Submitter Roman Gareev
Date March 8, 2014, 10:09 a.m.
Message ID <CABGF_ge7KFZPc=-Hm7kd2xi47y+gZhTZF5UVWcKPzfWT4f3KbA@mail.gmail.com>
Download mbox | patch
Permalink /patch/328172/
State New
Headers show

Comments

Roman Gareev - March 8, 2014, 10:09 a.m.
This patch fixes the following bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59586.
The segfault is caused by NULL arguments passed to compute_deps by
loop_level_carries_dependences.
This causes an assignment of NULL values to the no_source parameters
of compute_deps.
They are passed to subtract_commutative_associative_deps and dereferenced.

However, this NULL arguments are appropriate for the algorithm used
in loop_level_carries_dependences. It uses compute_deps
for finding RAW, WAR and WAW dependences of all basic blocks
in the body of the given loop. Subsequently, it tries to
determine presence of these dependences at the given level.
Therefore it maps the relation of the dependences to the relation
of the corresponding time-stamps and intersects the result with
the relation in which all the inputs before the DEPTH occur at the
same time as the output, and the input at the DEPTH occurs before output.
If the intersection is not empty, some dependences are carried
by the DEPTH we currently check and the loop is consequently not parallel.

This patch tries to avoid the problem by addition to
subtract_commutative_associative_deps
of NULL checking of the no_source statements.

Tested x86_64-unknown-linux-gnu, applying to 4.8.3 and trunk.

2014-03-07  Roman Gareev  <gareevroman@gmail.com>

                gcc/
                * graphite-dependences.c
(subtract_commutative_associative_deps):
                Add NULL checking of the following variables:
must_raw_no_source, may_raw_no_source,
                must_war_no_source, may_war_no_source,
                must_waw_no_source, may_waw_no_source.

                gcc/testsuite/gfortran.dg/graphite/graphite.exp: Run
corresponding
                tests in new mode.


gcc/testsuite/gfortran.dg/graphite/parallelize-all-1.f: New testcase.
Roman Gareev - March 8, 2014, 11:07 a.m.
Sorry, an error occurred somewhere. Below is an attachment with the
patch and ChangeLog entry.

> This patch fixes the following bug:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59586.
> The segfault is caused by NULL arguments passed to compute_deps by
> loop_level_carries_dependences.
> This causes an assignment of NULL values to the no_source parameters
> of compute_deps.
> They are passed to subtract_commutative_associative_deps and dereferenced.
>
> However, this NULL arguments are appropriate for the algorithm used
> in loop_level_carries_dependences. It uses compute_deps
> for finding RAW, WAR and WAW dependences of all basic blocks
> in the body of the given loop. Subsequently, it tries to
> determine presence of these dependences at the given level.
> Therefore it maps the relation of the dependences to the relation
> of the corresponding time-stamps and intersects the result with
> the relation in which all the inputs before the DEPTH occur at the
> same time as the output, and the input at the DEPTH occurs before output.
> If the intersection is not empty, some dependences are carried
> by the DEPTH we currently check and the loop is consequently not parallel.
>
> This patch tries to avoid the problem by addition to
> subtract_commutative_associative_deps
> of NULL checking of the no_source statements.
>
> Tested x86_64-unknown-linux-gnu, applying to 4.8.3 and trunk.

Patch

diff --git a/gcc/graphite-dependences.c b/gcc/graphite-dependences.c
index b0f8680..f382233 100644
--- a/gcc/graphite-dependences.c
+++ b/gcc/graphite-dependences.c
@@ -426,22 +426,48 @@  subtract_commutative_associative_deps (scop_p scop,

  *must_raw = isl_union_map_subtract (*must_raw, x_must_raw);
  *may_raw = isl_union_map_subtract (*may_raw, x_may_raw);
- *must_raw_no_source = isl_union_map_subtract (*must_raw_no_source,
-      x_must_raw_no_source);
- *may_raw_no_source = isl_union_map_subtract (*may_raw_no_source,
-     x_may_raw_no_source);
+
+ if (must_raw_no_source)
+  *must_raw_no_source = isl_union_map_subtract (*must_raw_no_source,
+          x_must_raw_no_source);
+ else
+  isl_union_map_free (x_must_raw_no_source);
+
+ if (may_raw_no_source)
+  *may_raw_no_source = isl_union_map_subtract (*may_raw_no_source,
+         x_may_raw_no_source);
+ else
+  isl_union_map_free (x_may_raw_no_source);
+
  *must_war = isl_union_map_subtract (*must_war, x_must_war);
  *may_war = isl_union_map_subtract (*may_war, x_may_war);
- *must_war_no_source = isl_union_map_subtract (*must_war_no_source,
-      x_must_war_no_source);
- *may_war_no_source = isl_union_map_subtract (*may_war_no_source,
-     x_may_war_no_source);
+
+ if (must_war_no_source)
+  *must_war_no_source = isl_union_map_subtract (*must_war_no_source,
+          x_must_war_no_source);
+ else
+  isl_union_map_free (x_must_war_no_source);
+
+ if (may_war_no_source)
+  *may_war_no_source = isl_union_map_subtract (*may_war_no_source,
+         x_may_war_no_source);
+ else
+  isl_union_map_free (x_may_war_no_source);
+
  *must_waw = isl_union_map_subtract (*must_waw, x_must_waw);
  *may_waw = isl_union_map_subtract (*may_waw, x_may_waw);
- *must_waw_no_source = isl_union_map_subtract (*must_waw_no_source,
-      x_must_waw_no_source);
- *may_waw_no_source = isl_union_map_subtract (*may_waw_no_source,
-     x_may_waw_no_source);
+
+ if (must_waw_no_source)
+  *must_waw_no_source = isl_union_map_subtract (*must_waw_no_source,
+          x_must_waw_no_source);
+ else
+  isl_union_map_free (x_must_waw_no_source);
+
+ if (may_waw_no_source)
+  *may_waw_no_source = isl_union_map_subtract (*may_waw_no_source,
+         x_may_waw_no_source);
+ else
+  isl_union_map_free (x_may_waw_no_source);
       }

   isl_union_map_free (original);
diff --git a/gcc/testsuite/gfortran.dg/graphite/graphite.exp
b/gcc/testsuite/gfortran.dg/graphite/graphite.exp
index c3aad13..3833e43 100644
--- a/gcc/testsuite/gfortran.dg/graphite/graphite.exp
+++ b/gcc/testsuite/gfortran.dg/graphite/graphite.exp
@@ -44,6 +44,7 @@  set interchange_files [lsort [glob -nocomplain
$srcdir/$subdir/interchange-*.\[f
 set scop_files        [lsort [glob -nocomplain
$srcdir/$subdir/scop-*.\[fF\]{,90,95,03,08} ] ]
 set run_id_files      [lsort [glob -nocomplain
$srcdir/$subdir/run-id-*.\[fF\]{,90,95,03,08} ] ]
 set vect_files        [lsort [glob -nocomplain
$srcdir/$subdir/vect-*.\[fF\]{,90,95,03,08} ] ]
+set parallelize_files [lsort [glob -nocomplain
$srcdir/$subdir/parallelize-all-*.\[fF\]{,90,95,03,08} ] ]

 # Tests to be compiled.
 set dg-do-what-default compile
@@ -51,6 +52,7 @@  gfortran-dg-runtest $scop_files        "-O2
-fgraphite -fdump-tree-graphite-all"
 gfortran-dg-runtest $id_files          "-O2 -fgraphite-identity -ffast-math"
 gfortran-dg-runtest $interchange_files "-O2 -floop-interchange
-fno-loop-block -fno-loop-strip-mine -ffast-math
-fdump-tree-graphite-all"
 gfortran-dg-runtest $block_files       "-O2 -floop-block
-fno-loop-strip-mine -fno-loop-interchange -ffast-math
-fdump-tree-graphite-all"
+gfortran-dg-runtest $parallelize_files "-Ofast  -floop-parallelize-all"

 # Vectorizer tests, to be run or compiled, depending on target capabilities.
 if [check_vect_support_and_set_flags] {
diff --git a/gcc/testsuite/gfortran.dg/graphite/parallelize-all-1.f
b/gcc/testsuite/gfortran.dg/graphite/parallelize-all-1.f
new file mode 100644
index 0000000..e1156f8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/graphite/parallelize-all-1.f
@@ -0,0 +1,9 @@ 
+      subroutine subsm ( n, x, xp, xx)
+      integer        n, m, x(n),xp(n), xx(n), gg(n), dd_p
+      do 55 i=1, n
+         dd_p  = dd_p + (x(i) - xx(i))*gg(i)
+ 55   continue
+      if ( dd_p .gt. 0  ) then
+         call dcopy( n, xp, 1, x, 1 )
+      endif
+      end