| Message ID | CABGF_gcUVb5z7GRf2Vg0yKojcGbX_U0TJbVsjrtVOEGRa_EU3A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 05/08/2014 06:02, Roman Gareev wrote: > I've attached the patch, which sets the separate option for all > dimensions. Is it fine for trunk? LGTM. Tobias
I've tested the modified version of Graphite using the gcc test suite
and haven't found out new failed tests.
However, pr35356-2.c is still not suitable for testing. The ISL AST
generated from its source code doesn't contain MIN or MAX.
if (k <= -1) {
for (int c1 = 0; c1 < n; c1 += 1)
S_7(c1);
} else if (k >= n) {
for (int c1 = 0; c1 < n; c1 += 1)
S_7(c1);
} else {
for (int c1 = 0; c1 < k; c1 += 1)
S_7(c1);
S_6(k);
for (int c1 = k + 1; c1 < n; c1 += 1)
S_7(c1);
}
Should we make pr35356-2.c to be similar to isl-codegen-loop-dumping.c
by replacing “MIN_EXPR\[^\\n\\r]*;" and "MAX_EXPR\[^\\n\\r]*;" with
the regexp, which contains the the above-mentioned isl ast?
On 06/08/2014 17:21, Roman Gareev wrote: > I've tested the modified version of Graphite using the gcc test suite > and haven't found out new failed tests. > > However, pr35356-2.c is still not suitable for testing. The ISL AST > generated from its source code doesn't contain MIN or MAX. > > if (k <= -1) { > for (int c1 = 0; c1 < n; c1 += 1) > S_7(c1); > } else if (k >= n) { > for (int c1 = 0; c1 < n; c1 += 1) > S_7(c1); > } else { > for (int c1 = 0; c1 < k; c1 += 1) > S_7(c1); > S_6(k); > for (int c1 = k + 1; c1 < n; c1 += 1) > S_7(c1); > } > > Should we make pr35356-2.c to be similar to isl-codegen-loop-dumping.c > by replacing “MIN_EXPR\[^\\n\\r]*;" and "MAX_EXPR\[^\\n\\r]*;" with > the regexp, which contains the the above-mentioned isl ast? Checking for this specific AST may cause failures with future versions of isl that choose a different schedule. Could you write a regular expression that checks that there is no if-condition contained in a for loop? I think this best models the issue that was addressed in the original bug report. (Also as this test then becomes isl specific I propose to just force the use of isl in the run line). Cheers, Tobias
Index: gcc/graphite-isl-ast-to-gimple.c =================================================================== --- gcc/graphite-isl-ast-to-gimple.c (revision 213619) +++ gcc/graphite-isl-ast-to-gimple.c (working copy) @@ -867,6 +867,25 @@ return id; } +/* Set the separate option for all dimensions. + This helps to reduce control overhead. */ + +static __isl_give isl_ast_build * +set_options (__isl_take isl_ast_build *control, + __isl_keep isl_union_map *schedule) +{ + isl_ctx *ctx = isl_union_map_get_ctx (schedule); + isl_space *range_space = isl_space_set_alloc (ctx, 0, 1); + range_space = + isl_space_set_tuple_name (range_space, isl_dim_set, "separate"); + isl_union_set *range = + isl_union_set_from_set (isl_set_universe (range_space)); + isl_union_set *domain = isl_union_map_range (isl_union_map_copy (schedule)); + domain = isl_union_set_universe (domain); + isl_union_map *options = isl_union_map_from_domain_and_range (domain, range); + return isl_ast_build_set_options (control, options); +} + static __isl_give isl_ast_node * scop_to_isl_ast (scop_p scop, ivs_params &ip) { @@ -879,6 +898,7 @@ add_parameters_to_ivs_params (scop, ip); isl_union_map *schedule_isl = generate_isl_schedule (scop); isl_ast_build *context_isl = generate_isl_context (scop); + context_isl = set_options (context_isl, schedule_isl); isl_union_map *dependences = NULL; if (flag_loop_parallelize_all) {