Skip to content

Commit

Permalink
isl_scheduler.c: graph_find_compressed_node: look up node in correct …
Browse files Browse the repository at this point in the history
…graph

The function graph_find_compressed_node has a fail-safe that checks
that the pointer recorded in the isl_id actually corresponds
to a node in the graph.  However, it failed to take into account
that the pointer refers to the original dependence graph,
while the graph in which the node is looked up may be subgraph
(with different node pointers).
Check that the node is valid for the original graph rather than
the current graph and if the current graph is not the same as
the original graph, then look up the corresponding node in
the current graph.

The new test case is different from the one that was
reported by Michael Kruse because the latter does not
appear to have a valid solution.
The test case is checked using the isl_schedule (and isl_schedule_cmp)
application(s) because it is easier to introduce test cases this way
(both the current test case and any future scheduling test cases).

Reported-by: Michael Kruse <[email protected]>
Signed-off-by: Sven Verdoolaege <[email protected]>
  • Loading branch information
Sven Verdoolaege committed Jul 30, 2017
1 parent 2dcebe4 commit d5b4535
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ noinst_PROGRAMS = isl_test isl_polyhedron_sample isl_pip \
isl_closure isl_bound isl_schedule isl_codegen isl_test_int \
isl_flow isl_flow_cmp isl_schedule_cmp
TESTS = isl_test codegen_test.sh pip_test.sh bound_test.sh isl_test_int \
flow_test.sh
flow_test.sh schedule_test.sh

if IMATH_FOR_MP

Expand Down
3 changes: 3 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ CXX11FLAGS=${CXX#$ac_save_CXX}
CXX="$ac_save_CXX"
CXXCPP="$ac_save_CXXCPP"

AC_PROG_GREP
AC_PROG_LIBTOOL
AC_PROG_SED

AC_CHECK_PROG(PERL, perl, perl, [])
AC_CHECK_PROG(PDFLATEX, pdflatex, pdflatex, [])
Expand Down Expand Up @@ -138,6 +140,7 @@ AC_CONFIG_FILES([bound_test.sh], [chmod +x bound_test.sh])
AC_CONFIG_FILES([codegen_test.sh], [chmod +x codegen_test.sh])
AC_CONFIG_FILES([pip_test.sh], [chmod +x pip_test.sh])
AC_CONFIG_FILES([flow_test.sh], [chmod +x flow_test.sh])
AC_CONFIG_FILES([schedule_test.sh], [chmod +x schedule_test.sh])
AC_CONFIG_COMMANDS_POST([
dnl pass on arguments to subdir configures, but don't
dnl add them to config.status
Expand Down
8 changes: 7 additions & 1 deletion isl_scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -3874,6 +3874,10 @@ static void isl_carry_clear(struct isl_carry *carry)
* If so, return that node.
* Otherwise, "space" was constructed by construct_compressed_id and
* contains a user pointer pointing to the node in the tuple id.
* However, this node belongs to the original dependence graph.
* If "graph" is a subgraph of this original dependence graph,
* then the node with the same space still needs to be looked up
* in the current graph.
*/
static struct isl_sched_node *graph_find_compressed_node(isl_ctx *ctx,
struct isl_sched_graph *graph, __isl_keep isl_space *space)
Expand All @@ -3895,9 +3899,11 @@ static struct isl_sched_node *graph_find_compressed_node(isl_ctx *ctx,
if (!node)
return NULL;

if (!is_node(graph, node))
if (!is_node(graph->root, node))
isl_die(ctx, isl_error_internal,
"space points to invalid node", return NULL);
if (graph != graph->root)
node = graph_find_node(ctx, graph, node->space);

return node;
}
Expand Down
21 changes: 21 additions & 0 deletions schedule_test.sh.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/bin/sh

EXEEXT=@EXEEXT@
GREP=@GREP@
SED=@SED@
srcdir=@srcdir@

failed=0

for i in $srcdir/test_inputs/schedule/*.sc; do
echo $i;
base=`basename $i .sc`
test=test-$base.st
dir=`dirname $i`
ref=$dir/$base.st
options=`$GREP 'OPTIONS:' $i | $SED 's/.*://'`
(./isl_schedule$EXEEXT $options < $i > $test &&
./isl_schedule_cmp$EXEEXT $ref $test && rm $test) || failed=1
done

test $failed -eq 0 || exit
5 changes: 5 additions & 0 deletions test_inputs/schedule/feautrier_compressed.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Check that the Feautrier schedule is not confused by
# compressed nodes in a subgraph of the original dependence graph.
# OPTIONS: --schedule-algorithm=feautrier
domain: { A[]; B[0]; C[] }
validity: { A[] -> B[0] }
11 changes: 11 additions & 0 deletions test_inputs/schedule/feautrier_compressed.st
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
domain: "{ B[0]; C[]; A[] }"
child:
set:
- filter: "{ B[i0]; A[] }"
child:
schedule: "[{ B[i0] -> [(1)]; A[] -> [(0)] }]"
child:
set:
- filter: "{ A[] }"
- filter: "{ B[i0] }"
- filter: "{ C[] }"

0 comments on commit d5b4535

Please sign in to comment.