On 07/23/2011 12:01 AM, Sebastian Pop wrote:
The CLAST produced by CLooG-ISL contains an assignment and GCC chokes
on it.  The exact CLAST contains an assignment followed by an if:

scat_1 = max(0,ceild(T_4-7,8));
if (scat_1<= min(1,floord(T_4-1,8))) {
   S7(scat_1);
}

This is equivalent to a loop that iterates only once, and so CLooG
generates an assignment followed by an if instead of a loop.  This is
an important optimization that was improved in ISL, that allows
if-conversion: imagine GCC having to figure out that a loop like the
following actually iterates only once, and can be converted to an if:

for (scat_1 = max(0,ceild(T_4-7,8)); scat_1<= min(1,floord(T_4-1,8)); scat_1++)
   S7(scat_1);

This patch implements the translation of CLAST assignments.
Bootstrapped and tested on amd64-linux.

Hi Sebastian,

thanks for adding this to graphite. One comment inline.


Sebastian

2011-07-22  Sebastian Pop<sebastian....@amd.com>

        PR middle-end/48648
        * graphite-clast-to-gimple.c (clast_get_body_of_loop): Handle
        CLAST assignments.
        (translate_clast): Same.
        (translate_clast_assignment): New.

        * gcc.dg/graphite/id-pr48648.c: New.
---
  gcc/ChangeLog                              |    8 ++++
  gcc/graphite-clast-to-gimple.c             |   49 ++++++++++++++++++++++++++++
  gcc/testsuite/ChangeLog                    |    5 +++
  gcc/testsuite/gcc.dg/graphite/id-pr48648.c |   21 ++++++++++++
  4 files changed, 83 insertions(+), 0 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/graphite/id-pr48648.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9cfa21b..303c9c9 100644
+/* Translates a clast assignment STMT to gimple.
+
+   - NEXT_E is the edge where new generated code should be attached.
+   - BB_PBB_MAPPING is is a basic_block and it's related poly_bb_p mapping.  */
+
+static edge
+translate_clast_assignment (struct clast_assignment *stmt, edge next_e,
+                           int level, ivs_params_p ip)
+{
+  gimple_seq stmts;
+  mpz_t v1, v2;
+  tree type, new_name, var;
+  edge res = single_succ_edge (split_edge (next_e));
+  struct clast_expr *expr = (struct clast_expr *) stmt->RHS;
+  struct clast_user_stmt *body
+    = clast_get_body_of_loop ((struct clast_stmt *) stmt);

I am not a big fan of using clast_get_body_of_loop as it is buggy. Introducing new uses of it, is nothing what I would support. Do we really need this?

+  poly_bb_p pbb = (poly_bb_p) cloog_statement_usr (body->statement);
+
+  mpz_init (v1);
+  mpz_init (v2);

What about some more meaningful names like bound_one, bound_two?

+  type = type_for_clast_expr (expr, ip, v1, v2);
+  var = create_tmp_var (type, "graphite_var");
+  new_name = force_gimple_operand (clast_to_gcc_expression (type, expr, ip),
+                               &stmts, true, var);
+  add_referenced_var (var);
+  if (stmts)
+    {
+      gsi_insert_seq_on_edge (next_e, stmts);
+      gsi_commit_edge_inserts ();
+    }
+
+  compute_bounds_for_level (pbb, level, v1, v2);

Mh. I do not completely understand all the code. But can't we get v1 and v2 set without the need for the compute_bounds_for_level function. Is the type_for_clast_expression not setting them.

Cheers
Tobi

Reply via email to