I worked with Roman on this patch and believe it is technically correct.
Some tiny comments inline.
Tobias
On 03/08/2014 10:57 AM, Roman Gareev wrote:
This patch fixes the following bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59586.
I would write PR59586. This should make this patch show up in the bug
tracker.
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.
by adding NULL checking of the no_source statements to subtract_...
Tested x86_64-unknown-linux-gnu, applying to 4.8.3 and trunk.
2014-03-07 Roman Gareev <gareevro...@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.
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"
I am slightly surprised why this change is necessary. Roman, can you
shade some light on this change.
# 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
