public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [GSoC] generation of Gimple code from isl_ast_node_user
@ 2014-07-12 12:18 Roman Gareev
  2014-07-13 12:27 ` Tobias Grosser
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-12 12:18 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

I've attached the patch, which contains generation of Gimple code from
isl_ast_node_user.

I think that it would be better to add motivation for the following
line from the original source:

if (GBB_BB (gbb) == ENTRY_BLOCK_PTR_FOR_FN (cfun))
  {
    isl_ast_expr_free (user_expr);
    return next_e;
  }

Do you know anything about this?

--
                                   Cheers, Roman Gareev.

[-- Attachment #2: ChangeLog_entry.txt --]
[-- Type: text/plain, Size: 1288 bytes --]

2014-07-12  Roman Gareev  <gareevroman@gmail.com>

	gcc/
	* graphite-isl-ast-to-gimple.c:
	Add inclusion of gimple-ssa.h, tree-into-ssa.h.
	(ivs_params_clear):
	Pass ivs_params_p ip instead of ivs_params &ip.
	(gcc_expression_from_isl_expression): Likewise.
	(gcc_expression_from_isl_ast_expr_id): Likewise.
	(binary_op_to_tree): Likewise.
	(ternary_op_to_tree): Likewise.
	(unary_op_to_tree): Likewise.
	(nary_op_to_tree): Likewise.
	(gcc_expression_from_isl_expr_op): Likewise.
	(graphite_create_new_loop): Likewise.
	(translate_isl_ast_for_loop): Pass ivs_params_p ip instead of
	ivs_params &ip. Add passing of bb_pbb_htab_type *bb_pbb_mapping.
	(translate_isl_ast_node_for): Likewise.
	(build_iv_mapping): New function.
	(mark_bb_with_pbb): Likewise.
	(translate_isl_ast_node_user): Likewise.
	(translate_isl_ast): Pass ivs_params_p ip instead of ivs_params &ip.
	Add passing of bb_pbb_htab_type *bb_pbb_mapping.
	(add_parameters_to_ivs_params): Pass ivs_params_p ip instead of
	ivs_params &ip.
	(scop_to_isl_ast): Likewise.
	(graphite_regenerate_ast_isl): Add passing of
	bb_pbb_htab_type *bb_pbb_mapping.
	* graphite-isl-ast-to-gimple.h: Add inclusion of the graphite-htab.h.
	* graphite.c (graphite_transform_loops): Add passing of &bb_pbb_mapping
	to graphite_regenerate_ast_isl.

[-- Attachment #3: patch.txt --]
[-- Type: text/plain, Size: 13567 bytes --]

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
--- gcc/graphite-isl-ast-to-gimple.c	(revision 212455)
+++ gcc/graphite-isl-ast-to-gimple.c	(working copy)
@@ -51,11 +51,14 @@
 #include "sese.h"
 #include "tree-ssa-loop-manip.h"
 #include "tree-scalar-evolution.h"
+#include "gimple-ssa.h"
+#include "tree-into-ssa.h"
 #include <map>
 
 #ifdef HAVE_cloog
 #include "graphite-poly.h"
 #include "graphite-isl-ast-to-gimple.h"
+#include "graphite-htab.h"
 
 /* This flag is set when an error occurred during the translation of
    ISL AST to Gimple.  */
@@ -94,18 +97,21 @@
 #endif
 }
 
-/* IVS_PARAMS maps ISL's scattering and parameter identifiers
+/* TREE_FROM_ISL_ID maps ISL's scattering and parameter identifiers
    to corresponding trees.  */
 
-typedef std::map<isl_id *, tree> ivs_params;
+typedef struct ivs_params {
+  std::map<isl_id *, tree> tree_from_isl_id;
+  sese region;
+} *ivs_params_p;
 
 /* Free all memory allocated for ISL's identifiers.  */
 
-void ivs_params_clear (ivs_params &ip)
+void ivs_params_clear (ivs_params_p ip)
 {
   std::map<isl_id *, tree>::iterator it;
-  for (it = ip.begin ();
-       it != ip.end (); it++)
+  for (it = ip->tree_from_isl_id.begin ();
+       it != ip->tree_from_isl_id.end (); it++)
     {
       isl_id_free (it->first);
     }
@@ -113,21 +119,21 @@
 
 static tree
 gcc_expression_from_isl_expression (tree type, __isl_take isl_ast_expr *,
-				    ivs_params &ip);
+				    ivs_params_p ip);
 
 /* Return the tree variable that corresponds to the given isl ast identifier
  expression (an isl_ast_expr of type isl_ast_expr_id).  */
 
 static tree
 gcc_expression_from_isl_ast_expr_id (__isl_keep isl_ast_expr *expr_id,
-				     ivs_params &ip)
+				     ivs_params_p ip)
 {
   gcc_assert (isl_ast_expr_get_type (expr_id) == isl_ast_expr_id);
   isl_id *tmp_isl_id = isl_ast_expr_get_id (expr_id);
   std::map<isl_id *, tree>::iterator res;
-  res = ip.find (tmp_isl_id);
+  res = ip->tree_from_isl_id.find (tmp_isl_id);
   isl_id_free (tmp_isl_id);
-  gcc_assert (res != ip.end () &&
+  gcc_assert (res != ip->tree_from_isl_id.end () &&
               "Could not map isl_id to tree expression");
   isl_ast_expr_free (expr_id);
   return res->second;
@@ -158,7 +164,7 @@
    type TYPE.  */
 
 static tree
-binary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params &ip)
+binary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params_p ip)
 {
   isl_ast_expr *arg_expr = isl_ast_expr_get_op_arg (expr, 0);
   tree tree_lhs_expr = gcc_expression_from_isl_expression (type, arg_expr, ip);
@@ -214,7 +220,7 @@
    type TYPE.  */
 
 static tree
-ternary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params &ip)
+ternary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params_p ip)
 {
   gcc_assert (isl_ast_expr_get_op_type (expr) == isl_ast_op_minus);
   isl_ast_expr *arg_expr = isl_ast_expr_get_op_arg (expr, 0);
@@ -235,7 +241,7 @@
    type TYPE.  */
 
 static tree
-unary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params &ip)
+unary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params_p ip)
 {
   gcc_assert (isl_ast_expr_get_op_type (expr) == isl_ast_op_minus);
   isl_ast_expr *arg_expr = isl_ast_expr_get_op_arg (expr, 0);
@@ -248,7 +254,7 @@
    to a GCC expression tree of type TYPE.  */
 
 static tree
-nary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params &ip)
+nary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params_p ip)
 {
   enum tree_code op_code;
   switch (isl_ast_expr_get_op_type (expr))
@@ -283,7 +289,7 @@
 
 static tree
 gcc_expression_from_isl_expr_op (tree type, __isl_take isl_ast_expr *expr,
-				 ivs_params &ip)
+				 ivs_params_p ip)
 {
   gcc_assert (isl_ast_expr_get_type (expr) == isl_ast_expr_op);
   switch (isl_ast_expr_get_op_type (expr))
@@ -334,7 +340,7 @@
 
 static tree
 gcc_expression_from_isl_expression (tree type, __isl_take isl_ast_expr *expr,
-				    ivs_params &ip)
+				    ivs_params_p ip)
 {
   switch (isl_ast_expr_get_type (expr))
     {
@@ -365,7 +371,7 @@
 static struct loop *
 graphite_create_new_loop (edge entry_edge, __isl_keep isl_ast_node *node_for,
 			  loop_p outer, tree type, tree lb, tree ub,
-			  ivs_params &ip)
+			  ivs_params_p ip)
 {
   isl_ast_expr *for_inc = isl_ast_node_for_get_inc (node_for);
   tree stride = gcc_expression_from_isl_expression (type, for_inc, ip);
@@ -377,14 +383,15 @@
 
   isl_ast_expr *for_iterator = isl_ast_node_for_get_iterator (node_for);
   isl_id *id = isl_ast_expr_get_id (for_iterator);
-  ip[id] = iv;
+  ip->tree_from_isl_id[id] = iv;
   isl_ast_expr_free (for_iterator);
   return loop;
 }
 
 static edge
 translate_isl_ast (loop_p context_loop, __isl_keep isl_ast_node *node,
-		   edge next_e, ivs_params &ip);
+		   edge next_e,  bb_pbb_htab_type *bb_pbb_mapping,
+		   ivs_params_p ip);
 
 /* Create the loop for a isl_ast_node_for.
 
@@ -393,8 +400,8 @@
 static edge
 translate_isl_ast_for_loop (loop_p context_loop,
 			    __isl_keep isl_ast_node *node_for, edge next_e,
-			    tree type, tree lb, tree ub,
-			    ivs_params &ip)
+			    bb_pbb_htab_type *bb_pbb_mapping, tree type,
+			    tree lb, tree ub, ivs_params_p ip)
 {
   gcc_assert (isl_ast_node_get_type (node_for) == isl_ast_node_for);
   struct loop *loop = graphite_create_new_loop (next_e, node_for, context_loop,
@@ -408,7 +415,7 @@
 
   /* Translate the body of the loop.  */
   isl_ast_node *for_body = isl_ast_node_for_get_body (node_for);
-  next_e = translate_isl_ast (loop, for_body, to_body, ip);
+  next_e = translate_isl_ast (loop, for_body, to_body, bb_pbb_mapping, ip);
   isl_ast_node_free (for_body);
   redirect_edge_succ_nodup (next_e, after);
   set_immediate_dominator (CDI_DOMINATORS, next_e->dest, next_e->src);
@@ -460,7 +467,8 @@
     case isl_ast_op_lt:
       {
         // (iterator < ub) => (iterator <= ub - 1)
-        isl_val *one = isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
+        isl_val *one =
+          isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
         isl_ast_expr *ub = isl_ast_expr_get_op_arg (for_cond, 1);
         res = isl_ast_expr_sub (ub, isl_ast_expr_from_val (one));
         break;
@@ -488,7 +496,7 @@
 static edge
 graphite_create_new_loop_guard (edge entry_edge,
 				__isl_keep isl_ast_node *node_for, tree *type,
-				tree *lb, tree *ub, ivs_params &ip)
+				tree *lb, tree *ub, ivs_params_p ip)
 {
   gcc_assert (isl_ast_node_get_type (node_for) == isl_ast_node_for);
   tree cond_expr;
@@ -528,7 +536,8 @@
 
 static edge
 translate_isl_ast_node_for (loop_p context_loop, __isl_keep isl_ast_node *node,
-			    edge next_e, ivs_params &ip)
+			    edge next_e, bb_pbb_htab_type *bb_pbb_mapping,
+			    ivs_params_p ip)
 {
   gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_for);
   tree type, lb, ub;
@@ -537,16 +546,96 @@
   edge true_e = get_true_edge_from_guard_bb (next_e->dest);
 
   translate_isl_ast_for_loop (context_loop, node, true_e,
-			      type, lb, ub, ip);
+			      bb_pbb_mapping, type, lb, ub, ip);
   return last_e;
 }
 
+/* Inserts in iv_map a tuple (OLD_LOOP->num, NEW_NAME) for the
+   induction variables of the loops around GBB in SESE.  */
+
+static void
+build_iv_mapping (vec<tree> iv_map, gimple_bb_p gbb,
+		  __isl_keep isl_ast_expr *user_expr, ivs_params_p ip)
+{
+  gcc_assert (isl_ast_expr_get_type (user_expr) == isl_ast_expr_op &&
+              isl_ast_expr_get_op_type (user_expr) == isl_ast_op_call);
+  int i;
+  isl_ast_expr *arg_expr;
+  for (i = 1; i < isl_ast_expr_get_op_n_arg (user_expr); i++)
+    {
+      arg_expr = isl_ast_expr_get_op_arg (user_expr, i);
+      tree type = *graphite_expression_size_type;
+      tree t = gcc_expression_from_isl_expression (type, arg_expr, ip);
+      loop_p old_loop = gbb_loop_at_index (gbb, ip->region, i - 1);
+      iv_map[old_loop->num] = t;
+    }
+
+}
+
+/* Mark BB with it's relevant PBB via hashing table BB_PBB_MAPPING.  */
+
+static void
+mark_bb_with_pbb (poly_bb_p pbb, basic_block bb,
+		  bb_pbb_htab_type *bb_pbb_mapping)
+{
+  bool existed;
+  poly_bb_p &e = bb_pbb_mapping->get_or_insert (bb, &existed);
+  if (!existed)
+    e = pbb;
+}
+
+/* Translates an isl_ast_node_user to Gimple. */
+
+static edge
+translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
+			     edge next_e, bb_pbb_htab_type *bb_pbb_mapping,
+			     ivs_params_p ip)
+{
+  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_user);
+  int i, nb_loops;
+  basic_block new_bb;
+  isl_ast_expr *user_expr = isl_ast_node_user_get_expr (node);
+  isl_ast_expr *name_expr = isl_ast_expr_get_op_arg (user_expr, 0);
+  gcc_assert (isl_ast_expr_get_type (name_expr) == isl_ast_expr_id);
+  isl_id *name_id = isl_ast_expr_get_id (name_expr);
+  poly_bb_p pbb = (poly_bb_p) isl_id_get_user (name_id);
+  gcc_assert (pbb);
+  gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
+  vec<tree> iv_map;
+  isl_ast_expr_free (name_expr);
+  isl_id_free (name_id);
+
+  if (GBB_BB (gbb) == ENTRY_BLOCK_PTR_FOR_FN (cfun))
+    {
+      isl_ast_expr_free (user_expr);
+      return next_e;
+    }
+
+  nb_loops = number_of_loops (cfun);
+  iv_map.create (nb_loops);
+  for (i = 0; i < nb_loops; i++)
+    iv_map.quick_push (NULL_TREE);
+
+  build_iv_mapping (iv_map, gbb, user_expr, ip);
+  isl_ast_expr_free (user_expr);
+  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb), ip->region,
+					   next_e, iv_map,
+					   &graphite_regenerate_error);
+  iv_map.release ();
+  new_bb = next_e->src;
+  mark_bb_with_pbb (pbb, new_bb, bb_pbb_mapping);
+  mark_virtual_operands_for_renaming (cfun);
+  update_ssa (TODO_update_ssa);
+  return next_e;
+}
+
 /* Translates an ISL AST node NODE to GCC representation in the
    context of a SESE.  */
 
 static edge
 translate_isl_ast (loop_p context_loop, __isl_keep isl_ast_node *node,
-		   edge next_e, ivs_params &ip)
+		   edge next_e, bb_pbb_htab_type *bb_pbb_mapping,
+		   ivs_params_p ip)
 {
   switch (isl_ast_node_get_type (node))
     {
@@ -555,13 +644,13 @@
 
     case isl_ast_node_for:
       return translate_isl_ast_node_for (context_loop, node,
-					 next_e, ip);
+					 next_e, bb_pbb_mapping, ip);
 
     case isl_ast_node_if:
       return next_e;
 
     case isl_ast_node_user:
-      return next_e;
+      return translate_isl_ast_node_user (node, next_e, bb_pbb_mapping, ip);
 
     case isl_ast_node_block:
       return next_e;
@@ -587,7 +676,7 @@
 /* Add ISL's parameter identifiers and corresponding.trees to ivs_params  */
 
 static void
-add_parameters_to_ivs_params (scop_p scop, ivs_params &ip)
+add_parameters_to_ivs_params (scop_p scop, ivs_params_p ip)
 {
   sese region = SCOP_REGION (scop);
   unsigned nb_parameters = isl_set_dim (scop->context, isl_dim_param);
@@ -596,7 +685,7 @@
   for (i = 0; i < nb_parameters; i++)
     {
       isl_id *tmp_id = isl_set_get_dim_id (scop->context, isl_dim_param, i);
-      ip[tmp_id] = SESE_PARAMS (region)[i];
+      ip->tree_from_isl_id[tmp_id] = SESE_PARAMS (region)[i];
     }
 }
 
@@ -639,7 +728,7 @@
 }
 
 static __isl_give isl_ast_node *
-scop_to_isl_ast (scop_p scop, ivs_params &ip)
+scop_to_isl_ast (scop_p scop, ivs_params_p ip)
 {
   /* Generate loop upper bounds that consist of the current loop iterator,
   an operator (< or <=) and an expression not involving the iterator.
@@ -663,17 +752,18 @@
           with ISL ASTs. Generation of GIMPLE code has to be completed.  */
 
 bool
-graphite_regenerate_ast_isl (scop_p scop)
+graphite_regenerate_ast_isl (scop_p scop, bb_pbb_htab_type *bb_pbb_mapping)
 {
   loop_p context_loop;
   sese region = SCOP_REGION (scop);
   ifsese if_region = NULL;
   isl_ast_node *root_node;
-  ivs_params ip;
+  struct ivs_params ip;
+  ip.region = region;
 
   timevar_push (TV_GRAPHITE_CODE_GEN);
   graphite_regenerate_error = false;
-  root_node = scop_to_isl_ast (scop, ip);
+  root_node = scop_to_isl_ast (scop, &ip);
 
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
@@ -696,7 +786,7 @@
   context_loop = SESE_ENTRY (region)->src->loop_father;
 
   translate_isl_ast (context_loop, root_node, if_region->true_region->entry,
-		     ip);
+		     bb_pbb_mapping, &ip);
   graphite_verify ();
   scev_reset ();
   recompute_all_dominators ();
@@ -709,7 +799,7 @@
   free (if_region->region);
   free (if_region);
 
-  ivs_params_clear (ip);
+  ivs_params_clear (&ip);
   isl_ast_node_free (root_node);
   timevar_pop (TV_GRAPHITE_CODE_GEN);
   /* TODO: Add dump  */
Index: gcc/graphite-isl-ast-to-gimple.h
===================================================================
--- gcc/graphite-isl-ast-to-gimple.h	(revision 212454)
+++ gcc/graphite-isl-ast-to-gimple.h	(working copy)
@@ -21,6 +21,8 @@
 #ifndef GCC_GRAPHITE_ISL_AST_TO_GIMPLE_H
 #define GCC_GRAPHITE_ISL_AST_TO_GIMPLE_H
 
-extern bool graphite_regenerate_ast_isl (scop_p);
+#include "graphite-htab.h"
 
+extern bool graphite_regenerate_ast_isl (scop_p, bb_pbb_htab_type *);
+
 #endif
Index: gcc/graphite.c
===================================================================
--- gcc/graphite.c	(revision 212454)
+++ gcc/graphite.c	(working copy)
@@ -301,7 +301,7 @@
 	if (POLY_SCOP_P (scop)
 	    && apply_poly_transforms (scop)
 	    && (((flag_graphite_code_gen == FGRAPHITE_CODE_GEN_ISL)
-	    && graphite_regenerate_ast_isl (scop))
+	    && graphite_regenerate_ast_isl (scop, &bb_pbb_mapping))
 	    || ((flag_graphite_code_gen == FGRAPHITE_CODE_GEN_CLOOG)
 	    && graphite_regenerate_ast_cloog (scop, &bb_pbb_mapping))))
 	  need_cfg_cleanup_p = true;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-12 12:18 [GSoC] generation of Gimple code from isl_ast_node_user Roman Gareev
@ 2014-07-13 12:27 ` Tobias Grosser
  2014-07-15 15:02   ` Roman Gareev
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Grosser @ 2014-07-13 12:27 UTC (permalink / raw)
  To: Roman Gareev; +Cc: Mircea Namolaru, gcc-patches

On 12/07/2014 14:18, Roman Gareev wrote:
> I've attached the patch, which contains generation of Gimple code from
> isl_ast_node_user.
>
> I think that it would be better to add motivation for the following
> line from the original source:
>
> if (GBB_BB (gbb) == ENTRY_BLOCK_PTR_FOR_FN (cfun))
>    {
>      isl_ast_expr_free (user_expr);
>      return next_e;
>    }

> Do you know anything about this?

No, no idea. To my understanding the entry block should not even appear
within a scop (see build_scops, where we only start detecting scops at
the successor of the entry_block). Maybe we replace this with an assert
to get a good error message in case I have missed something.

> -/* IVS_PARAMS maps ISL's scattering and parameter identifiers

> +/* TREE_FROM_ISL_ID maps ISL's scattering and parameter identifiers
>      to corresponding trees.  */

>
> -typedef std::map<isl_id *, tree> ivs_params;
> +typedef struct ivs_params {
> +  std::map<isl_id *, tree> tree_from_isl_id;
> +  sese region;

I think this region is actually not needed. At the place where you need
it, you have a pbb available, from which you can obtain a pointer to the
surrounding scop and from this you can obtain this region itself.

> +} *ivs_params_p;


>       case isl_ast_op_lt:
>         {
>           // (iterator < ub) => (iterator <= ub - 1)
> -        isl_val *one = isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
> +        isl_val *one =
> +          isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);

This is a pure style change which seems unrelated. Also, is the original
line really too long? I may have miscounted, but it seems to fit
exactly.

> +/* Inserts in iv_map a tuple (OLD_LOOP->num, NEW_NAME) for the
> +   induction variables of the loops around GBB in SESE.  */

In polly we just create an iv_map from [0, max-loop-depth-within-scop], so
it contains at most as many elements as the maximal loop depth. Your map
is unnecessarily large, as it contains all loops in the function.

If we can avoid this immediately, e.g. by indexing according to the
loop-depths that would be great. On the other side, if the existing
functions require this, I propose to not touch this area, but to add a
fixme that documents this issue. We can solve it after having removed
the CLooG codegen.

> +/* Mark BB with it's relevant PBB via hashing table BB_PBB_MAPPING.  */
> +
> +static void
> +mark_bb_with_pbb (poly_bb_p pbb, basic_block bb,
> +		  bb_pbb_htab_type *bb_pbb_mapping)
> +{
> +  bool existed;
> +  poly_bb_p &e = bb_pbb_mapping->get_or_insert (bb, &existed);
> +  if (!existed)
> +    e = pbb;
> +}

I was just briefly looking the code to remind me what this
bb_pbb_mapping hash table is about, but could not find the reason it is
needed. Do you know why it is needed?

Is it necessary for this patch? Or did you just copy it from the
previous code generation?

> +/* Translates an isl_ast_node_user to Gimple. */
> +
> +static edge
> +translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
> +			     edge next_e, bb_pbb_htab_type *bb_pbb_mapping,
> +			     ivs_params_p ip)
> +{
> +  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_user);
> +  int i, nb_loops;
> +  basic_block new_bb;
> +  isl_ast_expr *user_expr = isl_ast_node_user_get_expr (node);
> +  isl_ast_expr *name_expr = isl_ast_expr_get_op_arg (user_expr, 0);
> +  gcc_assert (isl_ast_expr_get_type (name_expr) == isl_ast_expr_id);
> +  isl_id *name_id = isl_ast_expr_get_id (name_expr);
> +  poly_bb_p pbb = (poly_bb_p) isl_id_get_user (name_id);
> +  gcc_assert (pbb);
> +  gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
> +  vec<tree> iv_map;
> +  isl_ast_expr_free (name_expr);
> +  isl_id_free (name_id);
> +
> +  if (GBB_BB (gbb) == ENTRY_BLOCK_PTR_FOR_FN (cfun))
> +    {
> +      isl_ast_expr_free (user_expr);
> +      return next_e;
> +    }
> +
> +  nb_loops = number_of_loops (cfun);
> +  iv_map.create (nb_loops);
> +  for (i = 0; i < nb_loops; i++)
> +    iv_map.quick_push (NULL_TREE);

Why do you need to first push NULL_TREEs into this vec, which will be
overwritten right after?

> +  build_iv_mapping (iv_map, gbb, user_expr, ip);
> +  isl_ast_expr_free (user_expr);
> +  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb), ip->region,
> +					   next_e, iv_map,
> +					   &graphite_regenerate_error);
> +  iv_map.release ();
> +  new_bb = next_e->src;
> +  mark_bb_with_pbb (pbb, new_bb, bb_pbb_mapping);
> +  mark_virtual_operands_for_renaming (cfun);
> +  update_ssa (TODO_update_ssa);
> +  return next_e;
> +}
> +

It is unclear how this patches have been tested. Can you elaborate?

Also, we need to find a way to test this in gcc itself, possibly by
running test cases that already work with both the cloog and the isl ast
generator.

Cheers,
Tobias

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-13 12:27 ` Tobias Grosser
@ 2014-07-15 15:02   ` Roman Gareev
  2014-07-15 16:15     ` Tobias Grosser
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-15 15:02 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4187 bytes --]

> No, no idea. To my understanding the entry block should not even appear
> within a scop (see build_scops, where we only start detecting scops at
> the successor of the entry_block). Maybe we replace this with an assert
> to get a good error message in case I have missed something.

Yes, I think this would be a good solution at this moment.

> I think this region is actually not needed. At the place where you need
> it, you have a pbb available, from which you can obtain a pointer to the
> surrounding scop and from this you can obtain this region itself.

Yes, this is redundant.

> This is a pure style change which seems unrelated. Also, is the original
> line really too long? I may have miscounted, but it seems to fit
> exactly.

If I am not mistaken, lines should be limited to 80 characters,
according to conventions, which are mentioned here
https://gcc.gnu.org/wiki/CppConventions. This line contains 81
characters.

> In polly we just create an iv_map from [0, max-loop-depth-within-scop], so
> it contains at most as many elements as the maximal loop depth. Your map
> is unnecessarily large, as it contains all loops in the function.
>
> If we can avoid this immediately, e.g. by indexing according to the
> loop-depths that would be great. On the other side, if the existing
> functions require this, I propose to not touch this area, but to add a
> fixme that documents this issue. We can solve it after having removed
> the CLooG codegen.

If I am not mistaken, the existing function doesn't require that
iv_map contains at most as many elements as the maximal loop depth.
(If we consider chrec_apply_map (I think that it is the only function,
which works with iv_map), we'll see that it calls chrec_apply when the
expression is not NULL. In our case, we push NULL_TREEs into iv_map to
extend its size to the maximal loop depth. They aren't considered,
because tree NULL_TREE is equivalent to NULL, according to tree.h)

Maybe we could use a number of the innermost loop that contains the
basic block gbb. If I am not mistaken, outer loops considered in
build_iv_mapping have smaller numbers than it. What do you think about
this?

> I was just briefly looking the code to remind me what this
> bb_pbb_mapping hash table is about, but could not find the reason it is
> needed. Do you know why it is needed?
>
> Is it necessary for this patch? Or did you just copy it from the
> previous code generation?

Yes, I've forgotten to remove this. If I am not mistaken,
bb_pbb_mapping is used for checking for the loop parallelism in
graphite-clast-to-gimple.c.

> Why do you need to first push NULL_TREEs into this vec, which will be
> overwritten right after?

In the current implementation, we'll get the error “internal compiler
error: in operator[], at vec.h:736”, if we try to assign a value to
iv_map via [] and remove initial pushing of NULL_TREEs. We could push
new values into iv_map in build_iv_mapping, but I am not sure if there
are cases, which require special processing (for example, NULL_TREEs
should be pushed into iv_map before the next old_loop->num. Possibly,
there are no such cases.).

> It is unclear how this patches have been tested. Can you elaborate?

I've compared the result of Gimple code generation (I've attached them
to the letter) for the following examples:

int
main (int n, int *a)
{
  int i;

  for (i = n; i < 100; i++)
    a[i] = i;

 return 0;
}


int
main (int 0, int *a)
{
  int i;

  for (i = n; i < 100; i++)
    a[i] = i;

 return 0;
}

> Also, we need to find a way to test this in gcc itself, possibly by
> running test cases that already work with both the cloog and the isl ast
> generator.

Maybe we could choose the strategy, which was used in
/gcc/testsuite/gcc.dg/graphite/interchange-1.c,
/gcc/testsuite/gcc.dg/graphite/interchange-mvt.c,
/gcc/testsuite/gcc.dg/graphite/run-id-5.c?
(The result of the transformed function is compared to the assumed
value and the abort function is called in case of inequality.) What do
you think about this?

--
                                   Cheers, Roman Gareev.

[-- Attachment #2: ChangeLog_entry.txt --]
[-- Type: text/plain, Size: 303 bytes --]

2014-07-12  Roman Gareev  <gareevroman@gmail.com>

	gcc/
	* graphite-isl-ast-to-gimple.c:
	Add inclusion of gimple-ssa.h, tree-into-ssa.h.
	(ivs_params_clear):
	(build_iv_mapping): New function.
	(translate_isl_ast_node_user): Likewise.
	(translate_isl_ast): Add calling of translate_isl_ast_node_user.

[-- Attachment #3: cloog-1.txt --]
[-- Type: text/plain, Size: 871 bytes --]

loop_0 (header = 0, latch = 1, niter = )
{
  bb_2 (preds = {bb_0 }, succs = {bb_3 })
  {
    <bb 2>:

  }
  bb_5 (preds = {bb_3 }, succs = {bb_1 })
  {
    <bb 5>:
    # .MEM_18 = PHI <.MEM_11(3)>
    # VUSE <.MEM_18>
    return 0;

  }
  loop_2 (header = 3, latch = 4, niter = )
  {
    bb_3 (preds = {bb_2 bb_4 }, succs = {bb_4 bb_5 })
    {
      <bb 3>:
      # graphite_IV.3_1 = PHI <0(2), graphite_IV.3_14(4)>
      # .MEM_19 = PHI <.MEM_3(D)(2), .MEM_11(4)>
      _2 = (sizetype) graphite_IV.3_1;
      _15 = _2 * 4;
      _16 = a_6(D) + _15;
      _17 = (int) graphite_IV.3_1;
      # .MEM_11 = VDEF <.MEM_19>
      *_16 = _17;
      graphite_IV.3_14 = graphite_IV.3_1 + 1;
      if (graphite_IV.3_1 < 99)
        goto <bb 4>;
      else
        goto <bb 5>;

    }
    bb_4 (preds = {bb_3 }, succs = {bb_3 })
    {
      <bb 4>:
      goto <bb 3>;

    }
  }
}


[-- Attachment #4: cloog-2.txt --]
[-- Type: text/plain, Size: 1456 bytes --]

loop_0 (header = 0, latch = 1, niter = )
{
  bb_2 (preds = {bb_0 }, succs = {bb_4 bb_3 })
  {
    <bb 2>:
    if (n_3(D) <= 99)
      goto <bb 4>;
    else
      goto <bb 3>;

  }
  bb_3 (preds = {bb_2 bb_8 }, succs = {bb_1 })
  {
    <bb 3>:
    # .MEM_12 = PHI <.MEM_4(D)(2), .MEM_25(8)>
    # VUSE <.MEM_12>
    return 0;

  }
  bb_4 (preds = {bb_2 }, succs = {bb_5 bb_8 })
  {
    <bb 4>:
    _2 = n_3(D) <= 99;
    if (_2 != 0)
      goto <bb 5>;
    else
      goto <bb 8>;

  }
  bb_5 (preds = {bb_4 }, succs = {bb_6 })
  {
    <bb 5>:
    _1 = 99 - n_3(D);

  }
  bb_8 (preds = {bb_6 bb_4 }, succs = {bb_3 })
  {
    <bb 8>:
    # .MEM_25 = PHI <.MEM_18(6), .MEM_4(D)(4)>
    goto <bb 3>;

  }
  loop_2 (header = 6, latch = 7, niter = (unsigned int) MAX_EXPR <_1, 0>, upper_bound = 2147483646)
  {
    bb_6 (preds = {bb_5 bb_7 }, succs = {bb_7 bb_8 })
    {
      <bb 6>:
      # graphite_IV.3_16 = PHI <0(5), graphite_IV.3_17(7)>
      # .MEM_26 = PHI <.MEM_4(D)(5), .MEM_18(7)>
      _19 = (sizetype) n_3(D);
      _20 = (sizetype) graphite_IV.3_16;
      _21 = _19 + _20;
      _22 = _21 * 4;
      _23 = a_7(D) + _22;
      _24 = n_3(D) + graphite_IV.3_16;
      # .MEM_18 = VDEF <.MEM_26>
      *_23 = _24;
      graphite_IV.3_17 = graphite_IV.3_16 + 1;
      if (graphite_IV.3_16 < _1)
        goto <bb 7>;
      else
        goto <bb 8>;

    }
    bb_7 (preds = {bb_6 }, succs = {bb_6 })
    {
      <bb 7>:
      goto <bb 6>;

    }
  }
}

[-- Attachment #5: isl-1.txt --]
[-- Type: text/plain, Size: 870 bytes --]

loop_0 (header = 0, latch = 1, niter = )
{
  bb_2 (preds = {bb_0 }, succs = {bb_3 })
  {
    <bb 2>:

  }
  bb_5 (preds = {bb_3 }, succs = {bb_1 })
  {
    <bb 5>:
    # .MEM_18 = PHI <.MEM_11(3)>
    # VUSE <.MEM_18>
    return 0;

  }
  loop_2 (header = 3, latch = 4, niter = )
  {
    bb_3 (preds = {bb_2 bb_4 }, succs = {bb_4 bb_5 })
    {
      <bb 3>:
      # graphite_IV.3_1 = PHI <0(2), graphite_IV.3_14(4)>
      # .MEM_19 = PHI <.MEM_3(D)(2), .MEM_11(4)>
      _2 = (sizetype) graphite_IV.3_1;
      _15 = _2 * 4;
      _16 = a_6(D) + _15;
      _17 = (int) graphite_IV.3_1;
      # .MEM_11 = VDEF <.MEM_19>
      *_16 = _17;
      graphite_IV.3_14 = graphite_IV.3_1 + 1;
      if (graphite_IV.3_1 < 99)
        goto <bb 4>;
      else
        goto <bb 5>;

    }
    bb_4 (preds = {bb_3 }, succs = {bb_3 })
    {
      <bb 4>:
      goto <bb 3>;

    }
  }
}

[-- Attachment #6: isl-2.txt --]
[-- Type: text/plain, Size: 1523 bytes --]

loop_0 (header = 0, latch = 1, niter = )
{
  bb_2 (preds = {bb_0 }, succs = {bb_4 bb_3 })
  {
    <bb 2>:
    if (n_3(D) <= 99)
      goto <bb 4>;
    else
      goto <bb 3>;

  }
  bb_3 (preds = {bb_2 bb_8 }, succs = {bb_1 })
  {
    <bb 3>:
    # .MEM_12 = PHI <.MEM_4(D)(2), .MEM_27(8)>
    # VUSE <.MEM_12>
    return 0;

  }
  bb_4 (preds = {bb_2 }, succs = {bb_5 bb_8 })
  {
    <bb 4>:
    _2 = n_3(D) <= 99;
    if (_2 != 0)
      goto <bb 5>;
    else
      goto <bb 8>;

  }
  bb_5 (preds = {bb_4 }, succs = {bb_6 })
  {
    <bb 5>:
    _1 = (long long int) n_3(D);
    _16 = 99 - _1;

  }
  bb_8 (preds = {bb_6 bb_4 }, succs = {bb_3 })
  {
    <bb 8>:
    # .MEM_27 = PHI <.MEM_19(6), .MEM_4(D)(4)>
    goto <bb 3>;

  }
  loop_2 (header = 6, latch = 7, niter = (unsigned long) MAX_EXPR <_16, 0>, upper_bound = 4611686018427387902)
  {
    bb_6 (preds = {bb_5 bb_7 }, succs = {bb_7 bb_8 })
    {
      <bb 6>:
      # graphite_IV.3_17 = PHI <0(5), graphite_IV.3_18(7)>
      # .MEM_28 = PHI <.MEM_4(D)(5), .MEM_19(7)>
      _20 = (sizetype) n_3(D);
      _21 = (sizetype) graphite_IV.3_17;
      _22 = _20 + _21;
      _23 = _22 * 4;
      _24 = a_7(D) + _23;
      _25 = (int) graphite_IV.3_17;
      _26 = n_3(D) + _25;
      # .MEM_19 = VDEF <.MEM_28>
      *_24 = _26;
      graphite_IV.3_18 = graphite_IV.3_17 + 1;
      if (graphite_IV.3_17 < _16)
        goto <bb 7>;
      else
        goto <bb 8>;

    }
    bb_7 (preds = {bb_6 }, succs = {bb_6 })
    {
      <bb 7>:
      goto <bb 6>;

    }
  }
}



[-- Attachment #7: patch.txt --]
[-- Type: text/plain, Size: 3775 bytes --]

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
--- gcc/graphite-isl-ast-to-gimple.c	(revision 212491)
+++ gcc/graphite-isl-ast-to-gimple.c	(working copy)
@@ -51,6 +51,8 @@
 #include "sese.h"
 #include "tree-ssa-loop-manip.h"
 #include "tree-scalar-evolution.h"
+#include "gimple-ssa.h"
+#include "tree-into-ssa.h"
 #include <map>
 
 #ifdef HAVE_cloog
@@ -65,7 +67,9 @@
 /* We always use signed 128, until isl is able to give information about
 types  */
 
-static tree *graphite_expression_size_type = &int128_integer_type_node;
+static tree *graphite_expression_size_type = int128_integer_type_node ?
+					     &int128_integer_type_node :
+					     &long_long_integer_type_node;
 
 /* Converts a GMP constant VAL to a tree and returns it.  */
 
@@ -460,7 +464,8 @@
     case isl_ast_op_lt:
       {
         // (iterator < ub) => (iterator <= ub - 1)
-        isl_val *one = isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
+        isl_val *one =
+          isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
         isl_ast_expr *ub = isl_ast_expr_get_op_arg (for_cond, 1);
         res = isl_ast_expr_sub (ub, isl_ast_expr_from_val (one));
         break;
@@ -541,6 +546,68 @@
   return last_e;
 }
 
+/* Inserts in iv_map a tuple (OLD_LOOP->num, NEW_NAME) for the
+   induction variables of the loops around GBB in SESE.  */
+
+static void
+build_iv_mapping (vec<tree> iv_map, gimple_bb_p gbb,
+		  __isl_keep isl_ast_expr *user_expr, ivs_params &ip,
+		  sese region)
+{
+  gcc_assert (isl_ast_expr_get_type (user_expr) == isl_ast_expr_op &&
+              isl_ast_expr_get_op_type (user_expr) == isl_ast_op_call);
+  int i;
+  isl_ast_expr *arg_expr;
+  for (i = 1; i < isl_ast_expr_get_op_n_arg (user_expr); i++)
+    {
+      arg_expr = isl_ast_expr_get_op_arg (user_expr, i);
+      tree type = *graphite_expression_size_type;
+      tree t = gcc_expression_from_isl_expression (type, arg_expr, ip);
+      loop_p old_loop = gbb_loop_at_index (gbb, region, i - 1);
+      iv_map[old_loop->num] = t;
+    }
+
+}
+
+/* Translates an isl_ast_node_user to Gimple. */
+
+static edge
+translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
+			     edge next_e, ivs_params &ip)
+{
+  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_user);
+  isl_ast_expr *user_expr = isl_ast_node_user_get_expr (node);
+  isl_ast_expr *name_expr = isl_ast_expr_get_op_arg (user_expr, 0);
+  gcc_assert (isl_ast_expr_get_type (name_expr) == isl_ast_expr_id);
+  isl_id *name_id = isl_ast_expr_get_id (name_expr);
+  poly_bb_p pbb = (poly_bb_p) isl_id_get_user (name_id);
+  gcc_assert (pbb);
+  gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
+  vec<tree> iv_map;
+  isl_ast_expr_free (name_expr);
+  isl_id_free (name_id);
+
+  gcc_assert (GBB_BB (gbb) != ENTRY_BLOCK_PTR_FOR_FN (cfun) &&
+	      "The entry block should not even appear within a scop");
+
+  loop_p loop = gbb_loop (gbb);
+  iv_map.create (loop->num + 1);
+  int i;
+  for (i = 0; i < loop->num + 1; i++)
+    iv_map.quick_push (NULL_TREE);
+
+  build_iv_mapping (iv_map, gbb, user_expr, ip, SCOP_REGION (pbb->scop));
+  isl_ast_expr_free (user_expr);
+  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb),
+					   SCOP_REGION (pbb->scop), next_e,
+					   iv_map,
+					   &graphite_regenerate_error);
+  iv_map.release ();
+  mark_virtual_operands_for_renaming (cfun);
+  update_ssa (TODO_update_ssa);
+  return next_e;
+}
+
 /* Translates an ISL AST node NODE to GCC representation in the
    context of a SESE.  */
 
@@ -561,7 +628,7 @@
       return next_e;
 
     case isl_ast_node_user:
-      return next_e;
+      return translate_isl_ast_node_user (node, next_e, ip);
 
     case isl_ast_node_block:
       return next_e;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-15 15:02   ` Roman Gareev
@ 2014-07-15 16:15     ` Tobias Grosser
  2014-07-17 14:11       ` Roman Gareev
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Grosser @ 2014-07-15 16:15 UTC (permalink / raw)
  To: Roman Gareev; +Cc: Mircea Namolaru, gcc-patches

On 15/07/2014 16:59, Roman Gareev wrote:
 >> >This is a pure style change which seems unrelated. Also, is the 
original
>> >line really too long? I may have miscounted, but it seems to fit
>> >exactly.
> If I am not mistaken, lines should be limited to 80 characters,
> according to conventions, which are mentioned here
> https://gcc.gnu.org/wiki/CppConventions. This line contains 81
> characters.

OK. Then I miscounted. Please commit this as a separate (obvious) change
and post the ChangeLog to gcc-patches.

We should keep unrelated cleanup patches out of patch reviews.

>> >In polly we just create an iv_map from [0, max-loop-depth-within-scop], so
>> >it contains at most as many elements as the maximal loop depth. Your map
>> >is unnecessarily large, as it contains all loops in the function.
>> >
>> >If we can avoid this immediately, e.g. by indexing according to the
>> >loop-depths that would be great. On the other side, if the existing
>> >functions require this, I propose to not touch this area, but to add a
>> >fixme that documents this issue. We can solve it after having removed
>> >the CLooG codegen.
> If I am not mistaken, the existing function doesn't require that
> iv_map contains at most as many elements as the maximal loop depth.
> (If we consider chrec_apply_map (I think that it is the only function,
> which works with iv_map), we'll see that it calls chrec_apply when the
> expression is not NULL. In our case, we push NULL_TREEs into iv_map to
> extend its size to the maximal loop depth. They aren't considered,
> because tree NULL_TREE is equivalent to NULL, according to tree.h)
>
> Maybe we could use a number of the innermost loop that contains the
> basic block gbb. If I am not mistaken, outer loops considered in
> build_iv_mapping have smaller numbers than it. What do you think about
> this?

I just looked again into chrec_apply_map and it requires a vector that
maps from the loop id to a tree. So the existing interface requires this
kind of input. I think the interface is not nice, but don't think we
should worry about this at the moment.

Maybe add a FIXME that says:

FIXME: Instead of using a vec<tree> that maps each loop id to a possible
chrec, we could consider using a map<int, tree> that maps loop ids
to the corresponding tree expressions.

>> >Why do you need to first push NULL_TREEs into this vec, which will be
>> >overwritten right after?
> In the current implementation, we'll get the error “internal compiler
> error: in operator[], at vec.h:736”, if we try to assign a value to
> iv_map via [] and remove initial pushing of NULL_TREEs. We could push
> new values into iv_map in build_iv_mapping, but I am not sure if there
> are cases, which require special processing (for example, NULL_TREEs
> should be pushed into iv_map before the next old_loop->num. Possibly,
> there are no such cases.).

I see. Could you use vec_safe_grow_cleared(iv_map, loop_num) instead?
This shows probably better that you zero initialize the vector.

>> >It is unclear how this patches have been tested. Can you elaborate?
> I've compared the result of Gimple code generation (I've attached them
> to the letter) for the following examples:
>
> int
> main (int n, int *a)
> {
>    int i;
>
>    for (i = n; i < 100; i++)
>      a[i] = i;
>
>   return 0;
> }
>
>
> int
> main (int 0, int *a)
> {
>    int i;
>
>    for (i = n; i < 100; i++)
>      a[i] = i;
>
>   return 0;
> }
>
>> >Also, we need to find a way to test this in gcc itself, possibly by
>> >running test cases that already work with both the cloog and the isl ast
>> >generator.
> Maybe we could choose the strategy, which was used in
> /gcc/testsuite/gcc.dg/graphite/interchange-1.c,
> /gcc/testsuite/gcc.dg/graphite/interchange-mvt.c,
> /gcc/testsuite/gcc.dg/graphite/run-id-5.c?
> (The result of the transformed function is compared to the assumed
> value and the abort function is called in case of inequality.) What do
> you think about this?

Right. You can just start by adjusting your test case, naming it
isl-ast-gen-single-loop.c

and adding the following at the beginnign of the file:

/* { dg-do run } */
/* { dg-options "-O2 -fgraphite-identity" } */

Some more comments:

>   /* We always use signed 128, until isl is able to give information about
>   types  */
>
> -static tree *graphite_expression_size_type = &int128_integer_type_node;
> +static tree *graphite_expression_size_type = int128_integer_type_node ?
> +					     &int128_integer_type_node :
> +					     &long_long_integer_type_node;

Please keep unrelated changes out of the patch review.

>   /* Converts a GMP constant VAL to a tree and returns it.  */
>
> @@ -460,7 +464,8 @@
>       case isl_ast_op_lt:
>         {
>           // (iterator < ub) => (iterator <= ub - 1)
> -        isl_val *one = isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
> +        isl_val *one =
> +          isl_val_int_from_si (isl_ast_expr_get_ctx (for_cond), 1);
Please keep unrelated changes out of the patch review.

Also, I think the patch now got nice and clean.

Tobias

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-15 16:15     ` Tobias Grosser
@ 2014-07-17 14:11       ` Roman Gareev
  2014-07-17 14:39         ` Tobias Grosser
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-17 14:11 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

> I see. Could you use vec_safe_grow_cleared(iv_map, loop_num) instead?
> This shows probably better that you zero initialize the vector.

If I am not mistaken, vec_safe_grow_cleared has the following declaration:

vec_safe_grow_cleared (vec<T, A, vl_embed> *&v, unsigned len CXX_MEM_STAT_INFO)

Should we rewrite all the functions, which interact with iv_map?

I've added test cases, which produce the following ISL code:

isl-ast-gen-single-loop-1.c

for (int c1 = 0; c1 <= 49; c1 += 1)
  S_3(c1);

isl-ast-gen-single-loop-2.c
for (int c1 = 0; c1 <= -n.0 + 69; c1 += 1)
  S_5(c1);

isl-ast-gen-single-loop-3.c
for (int c1 = 0; c1 < n.0; c1 += 1)
  S_5(c1);

The second and the third one use arrays. I wanted to make them similar
to the first one, but inability to handle blocks prevented this. For
example,

/* { dg-do run } */
/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */

int n = 25;

int
foo ()
{
  int i, res;

  for (i = n, res = 0; i < 50; i++)
      res += i;

  return res;
}

extern void abort ();

int
main (void)
{
  int res = foo ();

  if (res != 1225)
    abort ();

  return 0;
}

produces the following code:

{
  S_6();
  for (int c1 = 0; c1 <= -i + 49; c1 += 1)
    S_4(c1);
}

[-- Attachment #2: ChangeLog_entry.txt --]
[-- Type: text/plain, Size: 474 bytes --]

2014-07-12  Roman Gareev  <gareevroman@gmail.com>

	gcc/
	* graphite-isl-ast-to-gimple.c:
	Add inclusion of gimple-ssa.h, tree-into-ssa.h.
	(ivs_params_clear):
	(build_iv_mapping): New function.
	(translate_isl_ast_node_user): Likewise.
	(translate_isl_ast): Add calling of translate_isl_ast_node_user.

	gcc/testsuite/gcc.dg/graphite/
	* isl-ast-gen-single-loop-1.c: New testcase.
	* isl-ast-gen-single-loop-2.c: New testcase.
	* isl-ast-gen-single-loop-3.c: New testcase.

[-- Attachment #3: patch.txt --]
[-- Type: text/plain, Size: 5338 bytes --]

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
--- gcc/graphite-isl-ast-to-gimple.c	(revision 212756)
+++ gcc/graphite-isl-ast-to-gimple.c	(working copy)
@@ -51,6 +51,8 @@
 #include "sese.h"
 #include "tree-ssa-loop-manip.h"
 #include "tree-scalar-evolution.h"
+#include "gimple-ssa.h"
+#include "tree-into-ssa.h"
 #include <map>
 
 #ifdef HAVE_cloog
@@ -541,6 +543,72 @@
   return last_e;
 }
 
+/* Inserts in iv_map a tuple (OLD_LOOP->num, NEW_NAME) for the induction
+   variables of the loops around GBB in SESE.
+ 
+   FIXME: Instead of using a vec<tree> that maps each loop id to a possible
+   chrec, we could consider using a map<int, tree> that maps loop ids to the
+   corresponding tree expressions.  */
+
+static void
+build_iv_mapping (vec<tree> iv_map, gimple_bb_p gbb,
+		  __isl_keep isl_ast_expr *user_expr, ivs_params &ip,
+		  sese region)
+{
+  gcc_assert (isl_ast_expr_get_type (user_expr) == isl_ast_expr_op &&
+              isl_ast_expr_get_op_type (user_expr) == isl_ast_op_call);
+  int i;
+  isl_ast_expr *arg_expr;
+  for (i = 1; i < isl_ast_expr_get_op_n_arg (user_expr); i++)
+    {
+      arg_expr = isl_ast_expr_get_op_arg (user_expr, i);
+      tree type = *graphite_expression_size_type;
+      tree t = gcc_expression_from_isl_expression (type, arg_expr, ip);
+      loop_p old_loop = gbb_loop_at_index (gbb, region, i - 1);
+      iv_map[old_loop->num] = t;
+    }
+
+}
+
+/* Translates an isl_ast_node_user to Gimple. */
+
+static edge
+translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
+			     edge next_e, ivs_params &ip)
+{
+  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_user);
+  isl_ast_expr *user_expr = isl_ast_node_user_get_expr (node);
+  isl_ast_expr *name_expr = isl_ast_expr_get_op_arg (user_expr, 0);
+  gcc_assert (isl_ast_expr_get_type (name_expr) == isl_ast_expr_id);
+  isl_id *name_id = isl_ast_expr_get_id (name_expr);
+  poly_bb_p pbb = (poly_bb_p) isl_id_get_user (name_id);
+  gcc_assert (pbb);
+  gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
+  vec<tree> iv_map;
+  isl_ast_expr_free (name_expr);
+  isl_id_free (name_id);
+
+  gcc_assert (GBB_BB (gbb) != ENTRY_BLOCK_PTR_FOR_FN (cfun) &&
+	      "The entry block should not even appear within a scop");
+
+  loop_p loop = gbb_loop (gbb);
+  iv_map.create (loop->num + 1);
+  int i;
+  for (i = 0; i < loop->num + 1; i++)
+    iv_map.quick_push (NULL_TREE);
+
+  build_iv_mapping (iv_map, gbb, user_expr, ip, SCOP_REGION (pbb->scop));
+  isl_ast_expr_free (user_expr);
+  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb),
+					   SCOP_REGION (pbb->scop), next_e,
+					   iv_map,
+					   &graphite_regenerate_error);
+  iv_map.release ();
+  mark_virtual_operands_for_renaming (cfun);
+  update_ssa (TODO_update_ssa);
+  return next_e;
+}
+
 /* Translates an ISL AST node NODE to GCC representation in the
    context of a SESE.  */
 
@@ -561,7 +629,7 @@
       return next_e;
 
     case isl_ast_node_user:
-      return next_e;
+      return translate_isl_ast_node_user (node, next_e, ip);
 
     case isl_ast_node_block:
       return next_e;
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c	(working copy)
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+
+int n = 25;
+
+int
+foo ()
+{
+  int i, res;
+
+  for (i = n, res = 0; i < 50; i++)
+      res += i;
+
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{ 
+  int res = foo ();
+
+  if (res != 1225)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c	(working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+int n = 50;
+
+void
+foo (int a[])
+{
+  int i;
+  for (i = n - 20; i < 50; i++)
+    a[i] = i;
+}
+
+int
+array_sum (int a[])
+{
+  int i;
+  int res = 0;
+  for(i = n - 20; i < n; i *= 2)
+    res += a[i];
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{
+  int a[50];
+  foo (a);
+  int res = array_sum (a);
+  if (res != 30)
+    abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c	(working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+int n = 50;
+
+void
+foo (int a[])
+{
+  int i;
+  for (i = 0; i < n; i++)
+    a[i] = i;
+}
+
+int
+array_sum (int a[])
+{
+  int i;
+  int res = 0;
+  for(i = 1; i < n; i *= 2)
+    res += a[i];
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{
+  int a[50];
+  foo (a);
+  int res = array_sum (a);
+  if (res != 63)
+    abort ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-17 14:11       ` Roman Gareev
@ 2014-07-17 14:39         ` Tobias Grosser
  2014-07-18 10:31           ` Roman Gareev
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Grosser @ 2014-07-17 14:39 UTC (permalink / raw)
  To: Roman Gareev; +Cc: Mircea Namolaru, gcc-patches

On 17/07/2014 16:08, Roman Gareev wrote:
>> I see. Could you use vec_safe_grow_cleared(iv_map, loop_num) instead?
>> >This shows probably better that you zero initialize the vector.
> If I am not mistaken, vec_safe_grow_cleared has the following declaration:
>
> vec_safe_grow_cleared (vec<T, A, vl_embed> *&v, unsigned len CXX_MEM_STAT_INFO)
>
> Should we rewrite all the functions, which interact with iv_map?

Can you explain why all functions would need to be rewritten? I proposed
this function as an easier way to NULL initialize the vector and did not 
expect any rewrite to be necessary.

If there is no such thing, please just add a comment that your loop NULL 
initializes the vector. We can later improve this.

> I've added test cases, which produce the following ISL code:
>
> isl-ast-gen-single-loop-1.c
>
> for (int c1 = 0; c1 <= 49; c1 += 1)
>    S_3(c1);
>
> isl-ast-gen-single-loop-2.c
> for (int c1 = 0; c1 <= -n.0 + 69; c1 += 1)
>    S_5(c1);
>
> isl-ast-gen-single-loop-3.c
> for (int c1 = 0; c1 < n.0; c1 += 1)
>    S_5(c1);
>
> The second and the third one use arrays. I wanted to make them similar
> to the first one, but inability to handle blocks prevented this. For
> example,
>

OK. The tests look good.

Cheers,
Tobias

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-17 14:39         ` Tobias Grosser
@ 2014-07-18 10:31           ` Roman Gareev
  2014-07-18 10:34             ` Tobias Grosser
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-18 10:31 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

> Can you explain why all functions would need to be rewritten? I proposed
> this function as an easier way to NULL initialize the vector and did not
> expect any rewrite to be necessary.
>
> If there is no such thing, please just add a comment that your loop NULL
> initializes the vector. We can later improve this.

Sorry, I, probably, mixed something up. This function was used in the
attached patch without rewriting of other functions.

--
                                   Cheers, Roman Gareev.

[-- Attachment #2: ChangeLog_entry.txt --]
[-- Type: text/plain, Size: 474 bytes --]

2014-07-12  Roman Gareev  <gareevroman@gmail.com>

	gcc/
	* graphite-isl-ast-to-gimple.c:
	Add inclusion of gimple-ssa.h, tree-into-ssa.h.
	(ivs_params_clear):
	(build_iv_mapping): New function.
	(translate_isl_ast_node_user): Likewise.
	(translate_isl_ast): Add calling of translate_isl_ast_node_user.

	gcc/testsuite/gcc.dg/graphite/
	* isl-ast-gen-single-loop-1.c: New testcase.
	* isl-ast-gen-single-loop-2.c: New testcase.
	* isl-ast-gen-single-loop-3.c: New testcase.

[-- Attachment #3: patch.txt --]
[-- Type: text/plain, Size: 5298 bytes --]

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
--- gcc/graphite-isl-ast-to-gimple.c	(revision 212807)
+++ gcc/graphite-isl-ast-to-gimple.c	(working copy)
@@ -51,6 +51,8 @@
 #include "sese.h"
 #include "tree-ssa-loop-manip.h"
 #include "tree-scalar-evolution.h"
+#include "gimple-ssa.h"
+#include "tree-into-ssa.h"
 #include <map>
 
 #ifdef HAVE_cloog
@@ -541,6 +543,70 @@
   return last_e;
 }
 
+/* Inserts in iv_map a tuple (OLD_LOOP->num, NEW_NAME) for the induction
+   variables of the loops around GBB in SESE.
+ 
+   FIXME: Instead of using a vec<tree> that maps each loop id to a possible
+   chrec, we could consider using a map<int, tree> that maps loop ids to the
+   corresponding tree expressions.  */
+
+static void
+build_iv_mapping (vec<tree> iv_map, gimple_bb_p gbb,
+		  __isl_keep isl_ast_expr *user_expr, ivs_params &ip,
+		  sese region)
+{
+  gcc_assert (isl_ast_expr_get_type (user_expr) == isl_ast_expr_op &&
+              isl_ast_expr_get_op_type (user_expr) == isl_ast_op_call);
+  int i;
+  isl_ast_expr *arg_expr;
+  for (i = 1; i < isl_ast_expr_get_op_n_arg (user_expr); i++)
+    {
+      arg_expr = isl_ast_expr_get_op_arg (user_expr, i);
+      tree type = *graphite_expression_size_type;
+      tree t = gcc_expression_from_isl_expression (type, arg_expr, ip);
+      loop_p old_loop = gbb_loop_at_index (gbb, region, i - 1);
+      iv_map[old_loop->num] = t;
+    }
+
+}
+
+/* Translates an isl_ast_node_user to Gimple. */
+
+static edge
+translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
+			     edge next_e, ivs_params &ip)
+{
+  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_user);
+  isl_ast_expr *user_expr = isl_ast_node_user_get_expr (node);
+  isl_ast_expr *name_expr = isl_ast_expr_get_op_arg (user_expr, 0);
+  gcc_assert (isl_ast_expr_get_type (name_expr) == isl_ast_expr_id);
+  isl_id *name_id = isl_ast_expr_get_id (name_expr);
+  poly_bb_p pbb = (poly_bb_p) isl_id_get_user (name_id);
+  gcc_assert (pbb);
+  gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
+  vec<tree> iv_map;
+  isl_ast_expr_free (name_expr);
+  isl_id_free (name_id);
+
+  gcc_assert (GBB_BB (gbb) != ENTRY_BLOCK_PTR_FOR_FN (cfun) &&
+	      "The entry block should not even appear within a scop");
+
+  loop_p loop = gbb_loop (gbb);
+  iv_map.create (loop->num + 1);
+  iv_map.safe_grow_cleared (loop->num + 1);
+
+  build_iv_mapping (iv_map, gbb, user_expr, ip, SCOP_REGION (pbb->scop));
+  isl_ast_expr_free (user_expr);
+  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb),
+					   SCOP_REGION (pbb->scop), next_e,
+					   iv_map,
+					   &graphite_regenerate_error);
+  iv_map.release ();
+  mark_virtual_operands_for_renaming (cfun);
+  update_ssa (TODO_update_ssa);
+  return next_e;
+}
+
 /* Translates an ISL AST node NODE to GCC representation in the
    context of a SESE.  */
 
@@ -561,7 +627,7 @@
       return next_e;
 
     case isl_ast_node_user:
-      return next_e;
+      return translate_isl_ast_node_user (node, next_e, ip);
 
     case isl_ast_node_block:
       return next_e;
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c	(working copy)
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+
+int n = 25;
+
+int
+foo ()
+{
+  int i, res;
+
+  for (i = n, res = 0; i < 50; i++)
+      res += i;
+
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{ 
+  int res = foo ();
+
+  if (res != 1225)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c	(working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+int n = 50;
+
+void
+foo (int a[])
+{
+  int i;
+  for (i = n - 20; i < 50; i++)
+    a[i] = i;
+}
+
+int
+array_sum (int a[])
+{
+  int i;
+  int res = 0;
+  for(i = n - 20; i < n; i *= 2)
+    res += a[i];
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{
+  int a[50];
+  foo (a);
+  int res = array_sum (a);
+  if (res != 30)
+    abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c	(working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+int n = 50;
+
+void
+foo (int a[])
+{
+  int i;
+  for (i = 0; i < n; i++)
+    a[i] = i;
+}
+
+int
+array_sum (int a[])
+{
+  int i;
+  int res = 0;
+  for(i = 1; i < n; i *= 2)
+    res += a[i];
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{
+  int a[50];
+  foo (a);
+  int res = array_sum (a);
+  if (res != 63)
+    abort ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-18 10:31           ` Roman Gareev
@ 2014-07-18 10:34             ` Tobias Grosser
  2014-07-18 13:36               ` Roman Gareev
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Grosser @ 2014-07-18 10:34 UTC (permalink / raw)
  To: Roman Gareev; +Cc: Mircea Namolaru, gcc-patches

One last question:

On 18/07/2014 12:28, Roman Gareev wrote:
> +  iv_map.create (loop->num + 1);
> +  iv_map.safe_grow_cleared (loop->num + 1);

One of these two seems redundant.

Cheers,
Tobias

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-18 10:34             ` Tobias Grosser
@ 2014-07-18 13:36               ` Roman Gareev
  2014-07-21  8:32                 ` Roman Gareev
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-18 13:36 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

> One of these two seems redundant.

I get the following error without “iv_map.create (loop->num + 1);”:

"/home/roman/sec_trunk/gcc/gcc/vec.h:1184:39: error: ‘iv_map’ may be
used uninitialized in this function [-Werror=maybe-uninitialized] {
return m_vec ? m_vec->length () : 0; }"

>Can you explain why all functions would need to be rewritten? I proposed this function as an easier way to NULL initialize the vector and did not expect any rewrite to be necessary.

When I'm trying to pass iv_map to vec_safe_grow_cleared I get the
following error:

"/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:596:47:
note:   mismatched types ‘vec<T, A, vl_embed>*’ and ‘vec<tree_node*>’
vec_safe_grow_cleared (iv_map, loop->num + 1);"

In case of passing of iv_map*, I get the following error:

"/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:596:47:
error: no matching function for call to
‘vec_safe_grow_cleared(vec<tree_node*>*&, int)’
   vec_safe_grow_cleared (iv_map, loop->num + 1);
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:596:47:
note: candidate is:
In file included from /home/roman/sec_trunk/gcc/gcc/tree-core.h:27:0,
                 from /home/roman/sec_trunk/gcc/gcc/tree.h:23,
                 from
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:39:
/home/roman/sec_trunk/gcc/gcc/vec.h:627:1: note: template<class T,
class A> void vec_safe_grow_cleared(vec<T, A, vl_embed>*&, unsigned
int)
 vec_safe_grow_cleared (vec<T, A, vl_embed> *&v, unsigned len CXX_MEM_STAT_INFO)
/home/roman/sec_trunk/gcc/gcc/vec.h:627:1: note:   template argument
deduction/substitution failed:
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:596:47:
note:   mismatched types ‘vl_embed’ and ‘vl_ptr’
   vec_safe_grow_cleared (iv_map, loop->num + 1);
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:596:47:
note:   ‘vec<tree_node*>’ is not derived from ‘vec<T, A, vl_embed>’"

If we use vec<T, A, vl_embed> as a type for iv_map, we'll have to
rewrite declarations of the following functions, which are working
with vec<tree_node*>:copy_bb_and_scalar_dependences,
graphite_copy_stmts_from_block, rename_uses, chrec_apply_map. We'll
also have to rewrite declarations of iv_map in
graphite-clast-to-gimple.c in this case.

Please correct me, if I am wrong.

--
                                   Cheers, Roman Gareev.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-18 13:36               ` Roman Gareev
@ 2014-07-21  8:32                 ` Roman Gareev
  2014-07-21  8:57                   ` Tobias Grosser
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-21  8:32 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

Maybe we should  temporary postpone this and add a FIXME that says:

“We should remove iv_map.create (loop->num + 1), if it is possible.”

What do you think about this?

--
                                   Cheers, Roman Gareev.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-21  8:32                 ` Roman Gareev
@ 2014-07-21  8:57                   ` Tobias Grosser
  2014-07-21 10:34                     ` Roman Gareev
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Grosser @ 2014-07-21  8:57 UTC (permalink / raw)
  To: Roman Gareev; +Cc: Mircea Namolaru, gcc-patches

On 21/07/2014 10:25, Roman Gareev wrote:
> Maybe we should  temporary postpone this and add a FIXME that says:
>
> “We should remove iv_map.create (loop->num + 1), if it is possible.”
>
> What do you think about this?

Fine with me. Please post a question on gcc devel to see if someone can 
explain us the vec.h implementation.

Cheers,
Tobias

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-21  8:57                   ` Tobias Grosser
@ 2014-07-21 10:34                     ` Roman Gareev
  2014-07-21 15:30                       ` Tobias Grosser
  0 siblings, 1 reply; 13+ messages in thread
From: Roman Gareev @ 2014-07-21 10:34 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: Mircea Namolaru, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 135 bytes --]

I've asked the community about this.

The patch below contains the FIXME.

--
                                   Cheers, Roman Gareev.

[-- Attachment #2: ChangeLog_entry.txt --]
[-- Type: text/plain, Size: 474 bytes --]

2014-07-12  Roman Gareev  <gareevroman@gmail.com>

	gcc/
	* graphite-isl-ast-to-gimple.c:
	Add inclusion of gimple-ssa.h, tree-into-ssa.h.
	(ivs_params_clear):
	(build_iv_mapping): New function.
	(translate_isl_ast_node_user): Likewise.
	(translate_isl_ast): Add calling of translate_isl_ast_node_user.

	gcc/testsuite/gcc.dg/graphite/
	* isl-ast-gen-single-loop-1.c: New testcase.
	* isl-ast-gen-single-loop-2.c: New testcase.
	* isl-ast-gen-single-loop-3.c: New testcase.

[-- Attachment #3: patch.txt --]
[-- Type: text/plain, Size: 5413 bytes --]

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
--- gcc/graphite-isl-ast-to-gimple.c	(revision 212888)
+++ gcc/graphite-isl-ast-to-gimple.c	(working copy)
@@ -51,6 +51,8 @@
 #include "sese.h"
 #include "tree-ssa-loop-manip.h"
 #include "tree-scalar-evolution.h"
+#include "gimple-ssa.h"
+#include "tree-into-ssa.h"
 #include <map>
 
 #ifdef HAVE_cloog
@@ -547,6 +549,73 @@
   return last_e;
 }
 
+/* Inserts in iv_map a tuple (OLD_LOOP->num, NEW_NAME) for the induction
+   variables of the loops around GBB in SESE.
+ 
+   FIXME: Instead of using a vec<tree> that maps each loop id to a possible
+   chrec, we could consider using a map<int, tree> that maps loop ids to the
+   corresponding tree expressions.  */
+
+static void
+build_iv_mapping (vec<tree> iv_map, gimple_bb_p gbb,
+		  __isl_keep isl_ast_expr *user_expr, ivs_params &ip,
+		  sese region)
+{
+  gcc_assert (isl_ast_expr_get_type (user_expr) == isl_ast_expr_op &&
+              isl_ast_expr_get_op_type (user_expr) == isl_ast_op_call);
+  int i;
+  isl_ast_expr *arg_expr;
+  for (i = 1; i < isl_ast_expr_get_op_n_arg (user_expr); i++)
+    {
+      arg_expr = isl_ast_expr_get_op_arg (user_expr, i);
+      tree type =
+        build_nonstandard_integer_type (graphite_expression_type_precision, 0);
+      tree t = gcc_expression_from_isl_expression (type, arg_expr, ip);
+      loop_p old_loop = gbb_loop_at_index (gbb, region, i - 1);
+      iv_map[old_loop->num] = t;
+    }
+
+}
+
+/* Translates an isl_ast_node_user to Gimple.
+
+   FIXME: We should remove iv_map.create (loop->num + 1), if it is possible.  */
+
+static edge
+translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
+			     edge next_e, ivs_params &ip)
+{
+  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_user);
+  isl_ast_expr *user_expr = isl_ast_node_user_get_expr (node);
+  isl_ast_expr *name_expr = isl_ast_expr_get_op_arg (user_expr, 0);
+  gcc_assert (isl_ast_expr_get_type (name_expr) == isl_ast_expr_id);
+  isl_id *name_id = isl_ast_expr_get_id (name_expr);
+  poly_bb_p pbb = (poly_bb_p) isl_id_get_user (name_id);
+  gcc_assert (pbb);
+  gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
+  vec<tree> iv_map;
+  isl_ast_expr_free (name_expr);
+  isl_id_free (name_id);
+
+  gcc_assert (GBB_BB (gbb) != ENTRY_BLOCK_PTR_FOR_FN (cfun) &&
+	      "The entry block should not even appear within a scop");
+
+  loop_p loop = gbb_loop (gbb);
+  iv_map.create (loop->num + 1);
+  iv_map.safe_grow_cleared (loop->num + 1);
+
+  build_iv_mapping (iv_map, gbb, user_expr, ip, SCOP_REGION (pbb->scop));
+  isl_ast_expr_free (user_expr);
+  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb),
+					   SCOP_REGION (pbb->scop), next_e,
+					   iv_map,
+					   &graphite_regenerate_error);
+  iv_map.release ();
+  mark_virtual_operands_for_renaming (cfun);
+  update_ssa (TODO_update_ssa);
+  return next_e;
+}
+
 /* Translates an ISL AST node NODE to GCC representation in the
    context of a SESE.  */
 
@@ -567,7 +636,7 @@
       return next_e;
 
     case isl_ast_node_user:
-      return next_e;
+      return translate_isl_ast_node_user (node, next_e, ip);
 
     case isl_ast_node_block:
       return next_e;
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-1.c	(working copy)
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+
+int
+foo ()
+{
+  int i, res;
+
+  for (i = 0, res = 0; i < 50; i++)
+      res += i;
+
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{ 
+  int res = foo ();
+
+  if (res != 1225)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-2.c	(working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+int n = 50;
+
+void
+foo (int a[])
+{
+  int i;
+  for (i = n - 20; i < 50; i++)
+    a[i] = i;
+}
+
+int
+array_sum (int a[])
+{
+  int i;
+  int res = 0;
+  for(i = n - 20; i < n; i *= 2)
+    res += a[i];
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{
+  int a[50];
+  foo (a);
+  int res = array_sum (a);
+  if (res != 30)
+    abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/isl-ast-gen-single-loop-3.c	(working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fgraphite-code-generator=isl" } */
+int n = 50;
+
+void
+foo (int a[])
+{
+  int i;
+  for (i = 0; i < n; i++)
+    a[i] = i;
+}
+
+int
+array_sum (int a[])
+{
+  int i;
+  int res = 0;
+  for(i = 1; i < n; i *= 2)
+    res += a[i];
+  return res;
+}
+
+extern void abort ();
+
+int
+main (void)
+{
+  int a[50];
+  foo (a);
+  int res = array_sum (a);
+  if (res != 63)
+    abort ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [GSoC] generation of Gimple code from isl_ast_node_user
  2014-07-21 10:34                     ` Roman Gareev
@ 2014-07-21 15:30                       ` Tobias Grosser
  0 siblings, 0 replies; 13+ messages in thread
From: Tobias Grosser @ 2014-07-21 15:30 UTC (permalink / raw)
  To: Roman Gareev; +Cc: Mircea Namolaru, gcc-patches

On 21/07/2014 12:25, Roman Gareev wrote:
> I've asked the community about this.
>
> The patch below contains the FIXME.

LGTM. Feel free to commit.

Thanks,
Tobias

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-07-21 12:45 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-12 12:18 [GSoC] generation of Gimple code from isl_ast_node_user Roman Gareev
2014-07-13 12:27 ` Tobias Grosser
2014-07-15 15:02   ` Roman Gareev
2014-07-15 16:15     ` Tobias Grosser
2014-07-17 14:11       ` Roman Gareev
2014-07-17 14:39         ` Tobias Grosser
2014-07-18 10:31           ` Roman Gareev
2014-07-18 10:34             ` Tobias Grosser
2014-07-18 13:36               ` Roman Gareev
2014-07-21  8:32                 ` Roman Gareev
2014-07-21  8:57                   ` Tobias Grosser
2014-07-21 10:34                     ` Roman Gareev
2014-07-21 15:30                       ` Tobias Grosser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).