On 03/07/2014 19:24, Roman Gareev wrote:
However, this form doesn't have loop guards which are generated by
>>graphite_create_new_loop_guard in gcc/graphite-isl-ast-to-gimple.c and
>>by graphite_create_new_loop_guard in graphite-clast-to-gimple.c.
>
>
>Maybe the guards are directly constant folded? Can you try with:
I've tried this. It seems that the result is the same. If we consider
the following code, we'll see that the guards are generated:
int
main (int n, int *a)
{
int i;
for (i = n; i < 100; i++)
a[i] = i;
return 0;
}
[.. GIMPLE code ..]
OK. So this seems to fix your problem.
>>Below is the code of this generation (It still uses isl_int for
>>generation of isl_expr_int, because the error related to isl/val_gmp.h
>>still arises. I've tried to use isl 0.12.2 and 0.13, but gotten the
>>same error).
>
>
>Did using 'extern "C"' around the include statement not help?
I can't build gcc without using 'extern "C"'. After the successful
building the error is arising when I am trying to compile, for
example, the following code:
int
main (int n, int *a)
{
int i;
for (i = n; i < 100; i++)
a[i] = i;
return 0;
}
Sorry, I may have missed it. What kind of error was arising?
>Any reason this is not a simpel std::map<isl_id *, tree>, but you instead
>have this manually implemented hash table?
I think that using of std::map may create installation dependency,
which requires libstdc++. I've asked the community about this.
Great.
>Please explain why we need a special function to get the upper bound.
>Specifically, explain that isl in some configurations can generate loop
>exit conditions such as:
>
>for (i = ?; i + 2 >= 3 && 3 <= i + m; i++)
> ;
>
>This get upper bound assumes a certain shape of loop exit condition(
>
>for (i = ?; i < expr; i++)
>
>Also, you need to set the option --no-isl-ast-build-atomic-upper-bound
>in isl_ast_build to be able to rely on this behavior.
>(You did this below)
>
>Do you have a specific motivation, why you don't want to support
>arbitrary expressions? I assume this is necessary for the if - do-while
>optimization. If this is the case please state this.
I haven't found another function for generation loops in Gimple form.
The create_empty_loop_on_edge needs, which is being used now, requires
upper bound.
/* create_empty_loop_on_edge
|
| - pred_bb - -------- pred_bb ------
| | | | iv0 = initial_value |
| -----|----- --------------|-----------
| | ___ | entry_edge
| | entry_edge / | |
| | ====> / -----V---V- loop_header -------------
| V | | iv_before = phi (iv0, iv_after) |
| - succ_bb - |
----|-----------------------------------------
| | | | |
| ----------- | ---V--- loop_body
--------------------
| | | iv_after = iv_before + stride |
| | | if (iv_before < upper_bound) |
| |
---|--------------------------\--------------
| | |
\ exit_e
| | V \
| | - loop_latch -
---V- succ_bb -
| | | |
| |
| | /-----------------
-------------------------
| \ _ /
Furthermore, at the moment of loop generation we don't have the
induction variable, which is need for generation of a loop condition
in case of the option –no-isl-ast-build-atomic-upper-bound is unset.
The induction variable is returned by create_empty_loop_on_edge. Could
you please advise me another function to generate them?
>Please explain why we do not just generate a loop that has the loop
>bound at the top, but instead create a structure of the form
>
>if (lb > ub)
> do {
>
> } while (lb ..)
>
>(Such a figure, but completed might help).
>
>(I think the original motivation was that later we may be able to prove
>that a loop is executed at least once which allows us to remove the if
>which again enables better loop invariant code motion)
I didn't have special intentions for this. As was mentioned above, I
haven't found another way for generation of loops.
I see. The way you generate loops seems to be exactly right (at least
for now). I only was asking you to add comments explaining why this was
done this way. It seems you just went the way of least resistance, but
maybe you can use my previous comments to motivate this a little bit
more in the source code (I will then go through the comments and add the
missing pieces of motivation).
+/* We always use signed 128, until the isl is able to give information about
+types */
until isl is able
+
+static tree *graphite_temporary_tree_type = &int128_integer_type_node;
What about "graphite_expression_size_type". That seems more descriptive
to me.
[Skipped the hash table implementation]
I wait here for the std::map feedback.
void
@@ -65,9 +659,33 @@
isl_printer_free (prn);
}
+/* Add tree representations and names of parameters to ivs_params */
+
+static void
+add_parameters_to_ivs_params (scop_p scop, ivs_params_p ip)
+{
+ sese region = SCOP_REGION (scop);
+ int i;
+ int nb_parameters = SESE_PARAMS (region).length ();
+ gcc_assert (nb_parameters == (short) isl_set_dim (scop->context,
+ isl_dim_param));
+
+ for (i = 0; i < nb_parameters; i++)
+ {
+ tree param = SESE_PARAMS (region)[i];
+ const char *name = get_name (param);
+
+ if (!name)
+ name = "T";
+ save_isl_id_name_index (ip->ivs_params_index, name, i);
+ (ip->ivs_params_vec).safe_push (param);
Nice.
+ }
+}
+
+
/* Generates a build, which specifies the constraints on the parameters. */
-static isl_ast_build *
+static __isl_give isl_ast_build *
generate_isl_context (scop_p scop)
{
isl_set *context_isl = isl_set_params (isl_set_copy (scop->context));
@@ -77,7 +695,7 @@
/* Generates a schedule, which specifies an order used to
visit elements in a domain. */
-static isl_union_map *
+static __isl_give isl_union_map *
Such changes are independent and trivial changes. Please separate them
out and ask for separate review. We will then commit them immediately.
generate_isl_schedule (scop_p scop)
{
int i;
@@ -102,9 +720,16 @@
return schedule_isl;
}
-static isl_ast_node *
-scop_to_isl_ast (scop_p scop)
+static __isl_give isl_ast_node *
+scop_to_isl_ast (scop_p scop, ivs_params_p ip)
{
Such changes are independent and trivial changes. Please separate them
out and ask for separate review. We will then commit them immediately.
+ /* Generate loop upper bounds that consist of the current loop iterator,
+ an operator (< or <=) and an expression not involving the iterator.
+ If this option is not set, then the current loop iterator may appear several
+ times in the upper bound. See the isl manual for more details. */
+ isl_options_set_ast_build_atomic_upper_bound (scop->ctx, true);
Nice comment.
+ 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);
isl_ast_node *ast_isl = isl_ast_build_ast_from_schedule (context_isl,
@@ -117,21 +742,68 @@
the given SCOP. Return true if code generation succeeded.
FIXME: This is not yet a full implementation of the code generator
- with ISL ASTs. Generation of GIMPLE code is have to be added. */
+ with ISL ASTs. Generation of GIMPLE code has to be completed. */
Such changes are independent and trivial changes. Please separate them
out and ask for separate review. We will then commit them immediately.
I am still waiting for feedback on the std::map request, but generally
the patch looks very good already.
Cheers,
Tobias
