diff mbox

[GSoC] generation of Gimple code from isl_ast_node_if

Message ID CABGF_gfK8dqg1WnWODOf0TjmDGpk4_609oKiiBhSq5giE=c-kw@mail.gmail.com
State New
Headers show

Commit Message

Roman Gareev July 24, 2014, 10:09 a.m. UTC
I've attached the patch, which contains generation of Gimple code from
isl_ast_node_if.

However, I've found out a problem. When I'm trying to generate Gimple
code from, for example, the following ISL AST:

{
  for (int c1 = 0; c1 <= 49; c1 += 1) {
    S_6(c1);
    if (c1 <= 48) {
      S_3(c1);
      if (c1 >= 24)
        S_4(c1);
      S_5(c1);
    }
  }
  S_7();
}

the pointer to Gimple basic block of S_3's poly basic block is NULL.

Could you please advise me possible reasons of this issue?

The source code of the example:

int
foo ()
{
  int i, res = 0;

  for (i = 0; i < 50; i++)
    {
      if (i >= 25)
        res += i;
    }

  return res;
}

extern void abort ();

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

  if (res != 1225)
    abort ();

  return 0;
}

--
                                   Cheers, Roman Gareev.

Comments

Tobias Grosser July 24, 2014, 10:39 a.m. UTC | #1
On 24/07/2014 12:09, Roman Gareev wrote:
> I've attached the patch, which contains generation of Gimple code from
> isl_ast_node_if.

Nice.

> However, I've found out a problem. When I'm trying to generate Gimple
> code from, for example, the following ISL AST:
>
> {
>    for (int c1 = 0; c1 <= 49; c1 += 1) {
>      S_6(c1);
>      if (c1 <= 48) {
>        S_3(c1);
>        if (c1 >= 24)
>          S_4(c1);
>        S_5(c1);
>      }
>    }
>    S_7();
> }
>
> the pointer to Gimple basic block of S_3's poly basic block is NULL.
>
> Could you please advise me possible reasons of this issue?

I have no idea. Is the Gimple basic block of S_3 never set, or is it set 
and deleted on the way? What code does S_3 correspond to?

The code itself looks good. Let's get back to this after we understood 
this bug.

Cheers,
Tobias
Roman Gareev July 25, 2014, 11:20 a.m. UTC | #2
> I have no idea. Is the Gimple basic block of S_3 never set, or is it set and
> deleted on the way?

I think, that it is deleted on the way. I've found out, that the
Gimple basic block will be set, if we add the following code to the
generate_isl_schedule:

bb_schedule = isl_map_set_tuple_id (bb_schedule, isl_dim_in,
isl_id_for_pbb (scop, pbb));

where isl_id_for_pbb is

static isl_id *
isl_id_for_pbb (scop_p s, poly_bb_p pbb)
{
  char name[50];
  snprintf (name, sizeof (name), "S_%d", pbb_index (pbb));
  return isl_id_alloc (s->ctx, name, pbb);
}

> What code does S_3 correspond to?

If I'm not mistaken, it is corresponds to:

res_18 = Cross_BB_scalar_dependence.7[0];
phi_out_of_ssa.4[0] = res_18;

I used the following code from
https://gcc.gnu.org/onlinedocs/gccint/Basic-Blocks.html to dump basic
block of the Gimple basic block:

gimple_stmt_iterator si;

for (si = gsi_start_phis (bb); !gsi_end_p (si); gsi_next (&si))
  {
    gimple phi = gsi_stmt (si);
    print_gimple_stmt (dump_file, phi, 0, TDF_SLIM);
  }
for (si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
  {
    gimple stmt = gsi_stmt (si);
    print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
  }

Could you please advise me possible functions from
graphite-sese-to-poly.c, which can delete this?

P.S.: Only S_3 has this problem in the example.

--
                                   Cheers, Roman Gareev.
Tobias Grosser July 25, 2014, 11:30 a.m. UTC | #3
On 25/07/2014 13:20, Roman Gareev wrote:
>> I have no idea. Is the Gimple basic block of S_3 never set, or is it set and
>> deleted on the way?
>
> I think, that it is deleted on the way. I've found out, that the
> Gimple basic block will be set, if we add the following code to the
> generate_isl_schedule:
>
> bb_schedule = isl_map_set_tuple_id (bb_schedule, isl_dim_in,
> isl_id_for_pbb (scop, pbb));

Is at this point the pbb of S_3 still valid? Does it point to valid data?

> where isl_id_for_pbb is
>
> static isl_id *
> isl_id_for_pbb (scop_p s, poly_bb_p pbb)
> {
>    char name[50];
>    snprintf (name, sizeof (name), "S_%d", pbb_index (pbb));
>    return isl_id_alloc (s->ctx, name, pbb);
> }

This is surprising. You previously said the pbb pointer is still valid, 
but only the pointer that within the pbb that points to the gimple bb is 
invalid. I don't really see why setting the isl_id again (pointing to 
the very same pbb) helps in preserving the data structures in pbb.

>> What code does S_3 correspond to?
>
> If I'm not mistaken, it is corresponds to:
>
> res_18 = Cross_BB_scalar_dependence.7[0];
> phi_out_of_ssa.4[0] = res_18;
>
> I used the following code from
> https://gcc.gnu.org/onlinedocs/gccint/Basic-Blocks.html to dump basic
> block of the Gimple basic block:
>
> gimple_stmt_iterator si;
>
> for (si = gsi_start_phis (bb); !gsi_end_p (si); gsi_next (&si))
>    {
>      gimple phi = gsi_stmt (si);
>      print_gimple_stmt (dump_file, phi, 0, TDF_SLIM);
>    }
> for (si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
>    {
>      gimple stmt = gsi_stmt (si);
>      print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
>    }
>
> Could you please advise me possible functions from
> graphite-sese-to-poly.c, which can delete this?

This is a hard guess. I would propose to debug this in gdb. Add a 
breakpoint at a location where pbb/isl_id are set, verify that they are 
correctly set and add a watchpoint on the pointer that is set to ZERO. 
This should make gdb stop exactly at the point where the information is 
lost.

Alternatively, you could also try to run this in valgrind.

Cheers,
Tobias
Tobias Grosser July 26, 2014, 9:26 a.m. UTC | #4
On 24/07/2014 12:09, Roman Gareev wrote:
> I've attached the patch, which contains generation of Gimple code from
> isl_ast_node_if.
>
> However, I've found out a problem. When I'm trying to generate Gimple
> code from, for example, the following ISL AST:
>
> {
>    for (int c1 = 0; c1 <= 49; c1 += 1) {
>      S_6(c1);
>      if (c1 <= 48) {
>        S_3(c1);
>        if (c1 >= 24)
>          S_4(c1);
>        S_5(c1);
>      }
>    }
>    S_7();
> }
>
> the pointer to Gimple basic block of S_3's poly basic block is NULL.
>
> Could you please advise me possible reasons of this issue?
>
> The source code of the example:
>
> int
> foo ()
> {
>    int i, res = 0;
>
>    for (i = 0; i < 50; i++)
>      {
>        if (i >= 25)
>          res += i;
>      }
>
>    return res;
> }
>
> extern void abort ();
>
> int
> main (void)
> {
>    int res = foo ();
>
>    if (res != 1225)
>      abort ();
>
>    return 0;
> }

This patch looks also good. Can you quickly repost with the two test 
cases as well as the appropriate ChangeLog, before I give the final OK.

Cheers,
Tobias
diff mbox

Patch

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
--- gcc/graphite-isl-ast-to-gimple.c	(revision 212922)
+++ gcc/graphite-isl-ast-to-gimple.c	(working copy)
@@ -636,6 +636,42 @@ 
   return next_e;
 }
 
+/* Creates a new if region corresponding to ISL's cond.  */
+
+static edge
+graphite_create_new_guard (edge entry_edge, __isl_take isl_ast_expr *if_cond,
+			   ivs_params &ip)
+{
+  tree type =
+    build_nonstandard_integer_type (graphite_expression_type_precision, 0);
+  tree cond_expr = gcc_expression_from_isl_expression (type, if_cond, ip);
+  edge exit_edge = create_empty_if_region_on_edge (entry_edge, cond_expr);
+  return exit_edge;
+}
+
+/* Translates an isl_ast_node_if to Gimple.  */
+
+static edge
+translate_isl_ast_node_if (loop_p context_loop,
+			   __isl_keep isl_ast_node *node,
+			   edge next_e, ivs_params &ip)
+{
+  gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_if);
+  isl_ast_expr *if_cond = isl_ast_node_if_get_cond (node);
+  edge last_e = graphite_create_new_guard (next_e, if_cond, ip);
+
+  edge true_e = get_true_edge_from_guard_bb (next_e->dest);
+  isl_ast_node *then_node = isl_ast_node_if_get_then (node);
+  translate_isl_ast (context_loop, then_node, true_e, ip);
+  isl_ast_node_free (then_node);
+
+  edge false_e = get_false_edge_from_guard_bb (next_e->dest);
+  isl_ast_node *else_node = isl_ast_node_if_get_else (node);
+  translate_isl_ast (context_loop, else_node, false_e, ip);
+  isl_ast_node_free (else_node);
+  return last_e;
+}
+
 /* Translates an ISL AST node NODE to GCC representation in the
    context of a SESE.  */
 
@@ -653,7 +689,8 @@ 
 					 next_e, ip);
 
     case isl_ast_node_if:
-      return next_e;
+      return translate_isl_ast_node_if (context_loop, node,
+					next_e, ip);
 
     case isl_ast_node_user:
       return translate_isl_ast_node_user (node, next_e, ip);