[PATCH] [Graphite] Using SESE regions as black-boxes

Tobias Grosser grosser@fim.uni-passau.de
Tue May 25 00:06:00 GMT 2010


On 05/23/10 21:28, Antoniu Pop wrote:
> Hi,
>
> Updated and re-tested patch. Changelog remains unchanged.
>
> Antoniu

Hi Antoniu,

I had a look at your patch. I like the idea and I think it will become 
very useful.
At the moment I agree with Sebastian that the code introduced is not 
tested without test cases. The way to do this is probably to improve our 
scop detection to automatically create black box regions or add some 
predecessor pass that can automatically detect black box regions.
Using this we can detect a large number of test cases to get good test 
coverage.

I also commented on the patch directly.

Thanks for your work

Tobi



 >diff --git a/gcc/graphite-clast-to-gimple.c 
b/gcc/graphite-clast-to-gimple.c
 >index 69cb400..d3db03f 100644
 >--- a/gcc/graphite-clast-to-gimple.c
 >+++ b/gcc/graphite-clast-to-gimple.c
 >@@ -995,13 +995,13 @@ translate_clast_user (sese region, struct 
clast_user_stmt *stmt, edge next_e,
 >   poly_bb_p pbb = (poly_bb_p) cloog_statement_usr (stmt->statement);
 >   gbb = PBB_BLACK_BOX (pbb);
 >
 >-  if (GBB_BB (gbb) == ENTRY_BLOCK_PTR)
 >+  if (GBB_ENTRY (gbb) == ENTRY_BLOCK_PTR)
 >     return next_e;
 >
 >   build_iv_mapping (rename_map, region, *newivs, newivs_index, stmt,
 > 		    params_index);
 >-  next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb), region,
 >-					   next_e, rename_map);
 >+  next_e = copy_gbb_and_scalar_dependences (gbb, region,
 >+					    next_e, rename_map);
 >   new_bb = next_e->src;
 >   mark_bb_with_pbb (pbb, new_bb, bb_pbb_mapping);
 >   update_ssa (TODO_update_ssa);
 >diff --git a/gcc/graphite-poly.c b/gcc/graphite-poly.c
 >index f35ad9e..eb5e64f 100644
 >--- a/gcc/graphite-poly.c
 >+++ b/gcc/graphite-poly.c
 >@@ -596,7 +596,7 @@ print_pbb_domain (FILE *file, poly_bb_p pbb, int 
verbosity)
 >
 >   if (verbosity > 0)
 >     {
 >-      fprintf (file, "# Iteration domain of bb_%d (\n", GBB_BB 
(gbb)->index);
 >+      fprintf (file, "# Iteration domain of bb_%d (\n", GBB_ENTRY 
(gbb)->index);
 >       fprintf (file, "#  eq");
Is a region completely defined by an entry? Otherwise I would prefer 
something like
"entry => exit".


 >       for (i = 0; i < pbb_dim_iter_domain (pbb); i++)
 >@@ -633,7 +633,7 @@ dump_gbb_cases (FILE *file, gimple_bb_p gbb)
 >   if (VEC_empty (gimple, cases))
 >     return;
 >
 >-  fprintf (file, "# cases bb_%d (\n", GBB_BB (gbb)->index);
 >+  fprintf (file, "# cases bb_%d (\n", GBB_ENTRY (gbb)->index);
Dito.

 >   for (i = 0; VEC_iterate (gimple, cases, i, stmt); i++)
 >     {
 >@@ -660,7 +660,7 @@ dump_gbb_conditions (FILE *file, gimple_bb_p gbb)
 >   if (VEC_empty (gimple, conditions))
 >     return;
 >
 >-  fprintf (file, "# conditions bb_%d (\n", GBB_BB (gbb)->index);
 >+  fprintf (file, "# conditions bb_%d (\n", GBB_ENTRY (gbb)->index);
Dito.


 >   for (i = 0; VEC_iterate (gimple, conditions, i, stmt); i++)
 >     {
 >@@ -1161,7 +1161,7 @@ loop_to_lst (loop_p loop, VEC (poly_bb_p, heap) 
*bbs, int *i)
 >   for (; VEC_iterate (poly_bb_p, bbs, *i, pbb); (*i)++)
 >     {
 >       lst_p stmt;
 >-      basic_block bb = GBB_BB (PBB_BLACK_BOX (pbb));
 >+      basic_block bb = GBB_ENTRY (PBB_BLACK_BOX (pbb));
It seems you are just checking the entry of the region. Is there anything
special about it?

I see two problems with this.

1. If there is a loop that starts at entry, but it is completely hidden 
in the
region. Do you want it to be part of the LST? Why?

2. Different basic blocks in the region, may be part of different loops. 
Which
of these loops should be part of the lst? Just the loop_father of the 
entry? Or
the first loop that is not completely part of the region?
 >
 >       if (bb->loop_father == loop)
 > 	stmt = new_lst_stmt (pbb);
 >@@ -1200,7 +1200,7 @@ scop_to_lst (scop_p scop)
 >   for (i = 0; i < n; i++)
 >     {
 >       poly_bb_p pbb = VEC_index (poly_bb_p, SCOP_BBS (scop), i);
 >-      loop_p loop = outermost_loop_in_sese (region, GBB_BB 
(PBB_BLACK_BOX (pbb)));
 >+      loop_p loop = outermost_loop_in_sese (region, GBB_ENTRY 
(PBB_BLACK_BOX (pbb)));
 >
 >       if (loop_in_sese_p (loop, region))
 > 	res = loop_to_lst (loop, SCOP_BBS (scop), &i);
 >diff --git a/gcc/graphite-poly.h b/gcc/graphite-poly.h
 >index 8ab24f9..db491a4 100644
 >--- a/gcc/graphite-poly.h
 >+++ b/gcc/graphite-poly.h
 >@@ -279,7 +279,8 @@ struct poly_scattering
 >
 > struct poly_bb
 > {
 >-  /* Pointer to a basic block or a statement in the compiler.  */
 >+  /* Pointer to a GIMPLE Black Box, which is a SESE region that will
 >+     be seen as a black box.  */
 >   void *black_box;
 >
 >   /* Pointer to the SCOP containing this PBB.  */
 >@@ -391,7 +392,7 @@ number_of_write_pdrs (poly_bb_p pbb)
 > static inline basic_block
 > pbb_bb (poly_bb_p pbb)
 > {
 >-  return GBB_BB (PBB_BLACK_BOX (pbb));
 >+  return GBB_ENTRY (PBB_BLACK_BOX (pbb));
There is not a specific basic block for the black box, but a region. 
What is the reason
to return the entry basic block for this? Does this function make any 
sense at all
after introducing real black boxes?

 > }
 >
 > /* The index of the PBB.  */
 >diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
 >index 3e2ad0f..add6640 100644
 >--- a/gcc/graphite-scop-detection.c
 >+++ b/gcc/graphite-scop-detection.c
 >@@ -60,7 +60,8 @@ typedef enum gbb_type {
 >   GBB_LOOP_EXIT,
 >   GBB_COND_HEADER,
 >   GBB_SIMPLE,
 >-  GBB_LAST
 >+  GBB_LAST,
 >+  GBB_BLACK_BOX
 > } gbb_type;
 >
 > /* Detect the type of BB.  Loop headers are only marked, if they are
 >@@ -75,6 +76,14 @@ get_bb_type (basic_block bb, struct loop *last_loop)
 >   int nb_dom, nb_suc;
 >   struct loop *loop = bb->loop_father;
 >
 >+  /* If the gimple_bb has already been built, then this should already
 >+     be seen as a black box.  */
 >+  if (gbb_from_bb (bb))
 >+    {
 >+      gcc_assert (GBB_IS_SESE (gbb_from_bb (bb)));
 >+      return GBB_BLACK_BOX;
 >+    }
In which cases is the gimple_bb already built?

 >+
 >   /* Check, if we entry into a new loop. */
 >   if (loop != last_loop)
 >     {
 >@@ -451,6 +460,22 @@ scopdet_basic_block_info (basic_block bb, loop_p 
outermost_loop,
 >
 >   switch (type)
 >     {
 >+    case GBB_BLACK_BOX:
 >+      {
 >+	gimple_bb_p gbb = gbb_from_bb (bb);
 >+
 >+	/* For now we will consider that only SESE regions (i.e., no
 >+	   single BB) can be considered black-boxes, though this may
 >+	   change.  */
 >+	gcc_assert (GBB_IS_SESE (gbb));
 >+
 >+	/* We skip the whole SESE of this WBB.  */
 >+	result.next = single_succ (GBB_EXIT (gbb));
 >+	result.exits = false;
 >+	result.exit = single_succ (GBB_EXIT (gbb));
 >+	result.difficult = false;
 >+	break;
 >+      }

How do you know that the black box does not contain any side effects? Is 
this
true for all black boxes or do you require that the blackbox detection
annotates the black boxes?  I have read that you are saving the data 
references
in the black box, however you are not checking them during SCoP 
detection, do
you?

 >     case GBB_LAST:
 >       result.next = NULL;
 >       result.exits = false;
 >diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
 >index 6f3f8aa..d4d568d 100644
 >--- a/gcc/graphite-sese-to-poly.c
 >+++ b/gcc/graphite-sese-to-poly.c
 >@@ -272,13 +272,23 @@ graphite_stmt_p (sese region, basic_block bb,
 > /* Store the GRAPHITE representation of BB.  */
 >
 > static gimple_bb_p
 >-new_gimple_bb (basic_block bb, VEC (data_reference_p, heap) *drs)
 >+new_gimple_bb (basic_block entry, basic_block exit,
 >+	       VEC (data_reference_p, heap) *drs)
How do you define the entry and exit of a SESE region? Are both contained in
the region. Probably adding a comment describing this would be useful. 
Furthermore
I would prefer, if we switch to support black boxes to only support 
regions in the scop.
So every polyhedral statement is a black box, a single basic block just 
a trivial one.
This would reduce special handling of black boxes.

Is this possible? This would also allow to get most of the code tested 
in daily use.

 > {
 >   struct gimple_bb *gbb;
 >
 >+  /* If this is a SESE black-box, we need to check that it is properly
 >+     isolated.  */
 >+  if (exit != NULL)
 >+    {
 >+      gcc_assert (single_pred_p (entry));
 >+      gcc_assert (single_succ_p (exit));
 >+    }
 >+
 >   gbb = XNEW (struct gimple_bb);
 >-  bb->aux = gbb;
 >-  GBB_BB (gbb) = bb;
 >+  entry->aux = gbb;
 >+  GBB_ENTRY (gbb) = entry;
 >+  GBB_EXIT (gbb) = exit;
 >   GBB_DATA_REFS (gbb) = drs;
 >   GBB_CONDITIONS (gbb) = NULL;
 >   GBB_CONDITION_CASES (gbb) = NULL;
 >@@ -314,7 +324,7 @@ free_gimple_bb (struct gimple_bb *gbb)
 >
 >   VEC_free (gimple, heap, GBB_CONDITIONS (gbb));
 >   VEC_free (gimple, heap, GBB_CONDITION_CASES (gbb));
 >-  GBB_BB (gbb)->aux = 0;
 >+  GBB_ENTRY (gbb)->aux = 0;
 >   XDELETE (gbb);
 > }
 >
 >@@ -352,24 +362,68 @@ free_scops (VEC (scop_p, heap) *scops)
 >    information.  */
 >
 > static void
 >-try_generate_gimple_bb (scop_p scop, basic_block bb, sbitmap reductions)
 >+try_generate_gimple_bb (scop_p scop, basic_block bb, sbitmap 
reductions, sbitmap visited)
 > {
 >   VEC (data_reference_p, heap) *drs = VEC_alloc (data_reference_p, 
heap, 5);
 >   loop_p nest = outermost_loop_in_sese (SCOP_REGION (scop), bb);
 >   gimple_stmt_iterator gsi;
 >
 >-  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
 >+  /* Check whether this block is the entry point of a black boxed
 >+     region.  */
 >+  gimple_bb_p gbb = gbb_from_bb (bb);
 >+
 >+  if (gbb == NULL || !GBB_IS_SESE (gbb))
 >     {
 >-      gimple stmt = gsi_stmt (gsi);
 >-      if (!is_gimple_debug (stmt))
 >-	graphite_find_data_references_in_stmt (nest, stmt, &drs);
 >-    }
 >+      gcc_assert (gbb == NULL);
 >+
 >+      /* Base case: this block has not been pre-black-boxed.   */
 >+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
 >+	{
 >+	  gimple stmt = gsi_stmt (gsi);
 >+	  if (!is_gimple_debug (stmt))
 >+	    graphite_find_data_references_in_stmt (nest, stmt, &drs);
 >+	}
 >
 >-  if (!graphite_stmt_p (SCOP_REGION (scop), bb, drs))
 >-    free_data_refs (drs);
 >+      if (!graphite_stmt_p (SCOP_REGION (scop), bb, drs))
 >+	free_data_refs (drs);
 >+      else
 >+	new_poly_bb (scop, new_gimple_bb (bb, NULL, drs),
 >+		     TEST_BIT (reductions, bb->index));
 >+
 >+      SET_BIT (visited, bb->index);
 >+    }
 >   else
 >-    new_poly_bb (scop, new_gimple_bb (bb, drs), TEST_BIT (reductions,
 >-							  bb->index));
 >+    {
 >+      /* This is the entry point of a SESE region that has been
 >+	 selected to be black-boxed.
 >+
 >+	 FIXME: later, there may be a possibility to avoid looking at
 >+	 the data refs within the SESE in some cases like if this
 >+	 black box corresponds to OpenMP annotations that explicit all
 >+	 memory effects.  For now, gather all data refs in the whole
 >+	 SESE.  */
 >+      VEC(basic_block,heap) *blocks_in_sese = NULL;
 >+      basic_block it_bb;
 >+      int i;
 >+
 >+      VEC_safe_push (basic_block, heap, blocks_in_sese, GBB_ENTRY (gbb));
 >+      gather_blocks_in_sese_region (GBB_ENTRY (gbb), GBB_EXIT (gbb),
 >+				    &blocks_in_sese);
 >+      for (i = 0; VEC_iterate (basic_block, blocks_in_sese, i, 
it_bb); i++)
 >+	{
 >+	  for (gsi = gsi_start_bb (it_bb); !gsi_end_p (gsi); gsi_next (&gsi))
 >+	    {
 >+	      gimple stmt = gsi_stmt (gsi);
 >+	      if (!is_gimple_debug (stmt))
 >+		graphite_find_data_references_in_stmt (nest, stmt, &drs);
 >+	    }
 >+	  SET_BIT (visited, it_bb->index);
 >+	}
 >+
 >+      GBB_DATA_REFS (gbb) = drs;
 >+      new_poly_bb (scop, gbb, TEST_BIT (reductions, bb->index));
 >+      VEC_free (basic_block, heap, blocks_in_sese);
G>+    }
This function becomes really large. Is there a chance to split it into
something easier to understand?

 > }
 >
 > /* Returns true if all predecessors of BB, that are not dominated by 
BB, are
 >@@ -433,8 +487,7 @@ build_scop_bbs_1 (scop_p scop, sbitmap visited, 
basic_block bb, sbitmap reductio
 >       || !bb_in_sese_p (bb, region))
 >     return;
 >
 >-  try_generate_gimple_bb (scop, bb, reductions);
 >-  SET_BIT (visited, bb->index);
 >+  try_generate_gimple_bb (scop, bb, reductions, visited);
Unrelated? Why is this needed?

 >
 >   dom = get_dominated_by (CDI_DOMINATORS, bb);
 >
 >@@ -962,7 +1015,7 @@ find_params_in_bb (sese region, gimple_bb_p gbb)
 >   unsigned j;
 >   data_reference_p dr;
 >   gimple stmt;
 >-  loop_p loop = GBB_BB (gbb)->loop_father;
 >+  loop_p loop = gbb_loop (gbb);
Unrelated? Can probably be commited right now.


 >   mpz_t one;
 >
 >   mpz_init (one);
 >@@ -1028,14 +1081,6 @@ find_scop_parameters (scop_p scop)
 >     (&SCOP_CONTEXT (scop), scop_nb_params (scop), 0);
 > }
 >
 >-/* Returns a gimple_bb from BB.  */
 >-
 >-static inline gimple_bb_p
 >-gbb_from_bb (basic_block bb)
 >-{
 >-  return (gimple_bb_p) bb->aux;
 >-}
 >-
 > /* Insert in the SCOP context constraints from the estimation of the
 >    number of iterations.  UB_EXPR is a linear expression describing
 >    the number of iterations in a loop.  This expression is bounded by
 >diff --git a/gcc/sese.c b/gcc/sese.c
 >index e67628e..60a0e5e 100644
 >--- a/gcc/sese.c
 >+++ b/gcc/sese.c
 >@@ -142,7 +142,65 @@ eq_ivtype_map_elts (const void *e1, const void *e2)
 >   return (elt1->cloog_iv == elt2->cloog_iv);
 > }
 >
 >-

 >+/* Construct bb_bb_def with OLD_BB and NEW_BB. */
 >+
 >+static bb_bb_def *
 >+new_bb_bb_def (basic_block old_bb, basic_block new_bb)
 >+{
 >+  bb_bb_def *bb_bb_p;
 >+  bb_bb_p = XNEW (bb_bb_def);
 >+  bb_bb_p->old_bb = old_bb;
 >+  bb_bb_p->new_bb = new_bb;
 >+  return bb_bb_p;
 >+}
 >+
 >+/* Hash function for data base element BB_BB.  */
 >+
 >+static inline hashval_t
 >+bb_bb_map_hash (const void *bb_bb)
 >+{
 >+  return (hashval_t)(((const bb_bb_def *)bb_bb)->old_bb->index);
 >+}
 >+
 >+/* Compare data base element BB_BB1 and BB_BB2.  */
 >+
 >+static inline int
 >+eq_bb_bb_map (const void *bb_bb1, const void *bb_bb2)
 >+{
 >+  const bb_bb_def *bb1 = (const bb_bb_def *) bb_bb1;
 >+  const bb_bb_def *bb2 = (const bb_bb_def *) bb_bb2;
 >+  return (bb1->old_bb->index == bb2->old_bb->index);
 >+}
 >+
 >+/* Find in BB_MAP the new copy BB of the OLD BB.  */
 >+
 >+static inline basic_block
 >+find_new_bb (basic_block old, htab_t bb_map)
 >+{
 >+  bb_bb_def tmp;
 >+  PTR *x;
 >+
 >+  tmp.old_bb = old;
 >+  x = htab_find_slot (bb_map, &tmp, NO_INSERT);
 >+  if (!x || !*x)
 >+    return NULL;
 >+
 >+  return ((bb_bb_def *) (*x))->new_bb;
 >+}
 >+
 >+/* Insert in BB_MAP the copy NEW_BB of the OLD BB.  */
 >+
 >+static inline void
 >+insert_new_bb (basic_block new_bb, basic_block old, htab_t bb_map)
 >+{
 >+  bb_bb_def tmp;
 >+  PTR *x;
 >+
 >+  tmp.old_bb = old;
 >+  x = htab_find_slot (bb_map, &tmp, INSERT);
 >+  if (x && !*x)
 >+    *x = new_bb_bb_def (old, new_bb);
 >+}
 >
 > /* Record LOOP as occuring in REGION.  */
 >
 >@@ -1426,18 +1484,93 @@ graphite_copy_stmts_from_block (basic_block 
bb, basic_block new_bb, htab_t map)
 >    and returns the next edge following this new block.  */
 >
 > edge
 >-copy_bb_and_scalar_dependences (basic_block bb, sese region,
 >-				edge next_e, htab_t map)
 >+copy_gbb_and_scalar_dependences (gimple_bb_p gbb, sese region,
 >+				 edge next_e, htab_t map)
 > {
 >-  basic_block new_bb = split_edge (next_e);
 >
 >-  next_e = single_succ_edge (new_bb);
 >-  graphite_copy_stmts_from_block (bb, new_bb, map);
 >-  remove_condition (new_bb);
 >-  remove_phi_nodes (new_bb);
 >-  expand_scalar_variables (new_bb, region, map);
 >-  rename_variables (new_bb, map);
 >+  /* If this was a simple BB and not a SESE.  */
 >+  if (!GBB_IS_SESE (gbb))
 >+    {
 >+      basic_block bb = GBB_ENTRY (gbb);
 >+      basic_block new_bb = split_edge (next_e);
 >+
 >+      next_e = single_succ_edge (new_bb);
 >+      graphite_copy_stmts_from_block (bb, new_bb, map);
 >+      remove_condition (new_bb);
 >+      remove_phi_nodes (new_bb);
 >+      expand_scalar_variables (new_bb, region, map);
 >+      rename_variables (new_bb, map);
 >+    }
 >+  else
 >+    {
 >+      /* The internal structure of the SESE must be replicated.
 >+	 Conditions in the last block only should be removed.  A SESE
 >+	 should be assumed it has at least two blocks (the guards
 >+	 should ensure that).  */
Any reason for this restriction? If yes I think we should create a 
class, struct
for a region and check in the constructor that this requirement is met.

 >+      htab_t bb_map = htab_create (10, bb_bb_map_hash, eq_bb_bb_map, 
free);
 >+      basic_block entry_bb = GBB_ENTRY (gbb);
 >+      basic_block exit_bb = GBB_EXIT (gbb);
 >+      basic_block new_post_exit_bb = next_e->dest;
 >+      VEC(basic_block,heap) *bbs = NULL;
 >+      basic_block bb;
 >+
 >+      VEC_safe_push (basic_block, heap, bbs, entry_bb);
 >+      gather_blocks_in_sese_region (entry_bb, exit_bb, &bbs);
 >+
 >+      /* Replicate all blocks.  */
 >+      FOR_BB_BETWEEN (bb, entry_bb, single_succ(exit_bb), next_bb)
 >+	{
 >+	  basic_block new_bb = split_edge (next_e);
 >+	  next_e = single_succ_edge (new_bb);
 >+
 >+	  graphite_copy_stmts_from_block (bb, new_bb, map);
 >+	  remove_phi_nodes (new_bb);
 >+	  if (bb == exit_bb)
 >+	    remove_condition (new_bb);
 >+
 >+	  expand_scalar_variables (new_bb, region, map);
 >+	  rename_variables (new_bb, map);
 >+
 >+	  insert_new_bb (new_bb, bb, bb_map);
 >+	}
 >+
 >+      /* We need to fix-up the edges in the new SESE.  */
 >+      FOR_BB_BETWEEN (bb, entry_bb, single_succ(exit_bb), next_bb)
 >+	{
 >+	  basic_block new_bb = find_new_bb (bb, bb_map);
 >+	  edge e, n;
 >+	  edge_iterator ei;
 >
 >+	  /* Always remove the existing edges as they serve no purpose
 >+	     except for the entry (which would not be removed anyway
 >+	     here) and exit ones.  */
 >+	  remove_edge (single_succ_edge (new_bb));
 >+
 >+	  /* Copy all the edges form the original.  */
 >+	  FOR_EACH_EDGE (e, ei, bb->succs)
 >+	    {
 >+	      basic_block nsrc = find_new_bb (e->src, bb_map);
 >+	      basic_block ndst;
 >+
 >+	      if (bb == exit_bb)
 >+		ndst = new_post_exit_bb;
 >+	      else
 >+		ndst = find_new_bb (e->dest, bb_map);
 >+
 >+	      gcc_assert (nsrc != NULL && ndst != NULL);
 >+
 >+	      n = unchecked_make_edge (nsrc, ndst, e->flags);
 >+	      n->probability = e->probability;
 >+	    }
 >+	}
 >+
 >+      bb = find_new_bb (exit_bb, bb_map);
 >+      next_e = single_succ_edge (bb);
 >+      recompute_all_dominators ();
 >+
 >+      VEC_free (basic_block, heap, bbs);
 >+      htab_delete (bb_map);
 >+    }
This gets long. Can you split this in smaller functions?

 >   return next_e;
 > }
 >
 >diff --git a/gcc/sese.h b/gcc/sese.h
 >index 2b05ca8..fd99110 100644
 >--- a/gcc/sese.h
 >+++ b/gcc/sese.h
 >@@ -52,12 +52,14 @@ typedef struct sese_s
 > #define SESE_LOOP_NEST(S) (S->loop_nest)
 > #define SESE_ADD_PARAMS(S) (S->add_params)
 >
 >+struct gimple_bb;
 >+
 > extern sese new_sese (edge, edge);
 > extern void free_sese (sese);
 > extern void sese_insert_phis_for_liveouts (sese, basic_block, edge, 
edge);
 > extern void sese_adjust_liveout_phis (sese, htab_t, basic_block, 
edge, edge);
 > extern void build_sese_loop_nests (sese);
 >-extern edge copy_bb_and_scalar_dependences (basic_block, sese, edge, 
htab_t);
 >+extern edge copy_gbb_and_scalar_dependences (struct gimple_bb *, 
sese, edge, htab_t);
 > extern struct loop *outermost_loop_in_sese (sese, basic_block);
 > extern void insert_loop_close_phis (htab_t, loop_p);
 > extern void insert_guard_phis (basic_block, edge, edge, htab_t, htab_t);
 >@@ -318,9 +320,22 @@ recompute_all_dominators (void)
 >   calculate_dominance_info (CDI_POST_DOMINATORS);
 > }
 >
 >+/* Mapping the new gimple BBs in the code generated from the CLAST to
 >+   the old gimple BBs.  */
 >+
 >+typedef struct bb_bb_def_s
 >+{
 >+  basic_block old_bb;
 >+  basic_block new_bb;
 >+} bb_bb_def;
 >+
 >+/* Gimple Black-Box.  */
 >+
 > typedef struct gimple_bb
 > {
 >-  basic_block bb;
 >+  /* ENTRY and EXIT blocks of what must be a SESE region.  If a single
 >+     BB is used, exit should be NULL.  */
 >+  basic_block entry, exit;
So the entry and exit are referring to an sese region. How do you define a
region based on an entry and an exit basic block. This are the same 
questions
as above. Both are contained in the region? Or one is not?  Any reason 
you do
not use edges to define the sese region?

I personally prefer a definition based on basic blocks as it seems to be 
more
general however I would like to know your reasons and your definition of 
this.

 >
 >   /* Lists containing the restrictions of the conditional statements
 >      dominating this bb.  This bb can only be executed, if all conditions
 >@@ -347,7 +362,9 @@ typedef struct gimple_bb
 >   VEC (data_reference_p, heap) *data_refs;
 > } *gimple_bb_p;
 >
 >-#define GBB_BB(GBB) GBB->bb
 >+#define GBB_ENTRY(GBB) GBB->entry
 >+#define GBB_EXIT(GBB) GBB->exit
 >+#define GBB_IS_SESE(GBB) (GBB->exit != NULL)
 > #define GBB_DATA_REFS(GBB) GBB->data_refs
 > #define GBB_CONDITIONS(GBB) GBB->conditions
 > #define GBB_CONDITION_CASES(GBB) GBB->condition_cases
 >@@ -355,9 +372,17 @@ typedef struct gimple_bb
 > /* Return the innermost loop that contains the basic block GBB.  */
 >
 > static inline struct loop *
 >-gbb_loop (struct gimple_bb *gbb)
 >+gbb_loop (gimple_bb_p gbb)
 >+{
 >+  return GBB_ENTRY (gbb)->loop_father;
Is this correct? What if the entry is part of a loop contained 
completely in the region.

 >+}
 >+
 >+/* Returns a gimple_bb from BB.  */
 >+
 >+static inline gimple_bb_p
 >+gbb_from_bb (basic_block bb)
 > {
 >-  return GBB_BB (gbb)->loop_father;
 >+  return (gimple_bb_p) bb->aux;
 > }
What happens in case the bb is part of a region? I remember the aux 
pointer is just set for the region entry.


 >
 > /* Returns the gimple loop, that corresponds to the loop_iterator_INDEX.



> On Wed, May 19, 2010 at 5:25 PM, Antoniu Pop
> <antoniu.pop@mines-paristech.fr <mailto:antoniu.pop@mines-paristech.fr>>
> wrote:
>
>     Hi,
>
>     This patch enables using SESE regions (instead of basic blocks) as
>     black-boxes in POLY_BB, so it allows hiding non-static control-flow,
>     function calls and non-affine subscripts as long as it is possible to
>     know that the region is actually well-behaved. The whole SESE region
>     is considered a single poly statement.
>
>     The functionality for deciding which code regions should be hidden
>     will come in subsequent patches.
>
>     The patch passes regression tests on the Graphite branch.
>
>     Changelog:
>
>     2010-05-19  Antoniu Pop <antoniu.pop@gmail.com
>     <mailto:antoniu.pop@gmail.com>>
>
>             * graphite-scop-detection.c (gbb_type): Added GBB_BLACK_BOX.
>             (get_bb_type): Return the new type when apropriate.
>             (scopdet_basic_block_info): New GBB_BLACK_BOX case in the
>     switch.
>
>             * sese.c (new_bb_bb_def, bb_bb_map_hash, eq_bb_bb_map,
>             find_new_bb, insert_new_bb): New.  Handle the mapping
>     between old
>             BBs and new ones.
>             (copy_bb_and_scalar_dependences): Renamed to
>             copy_gbb_and_scalar_dependences and changed the parameters.  The
>             function now can either copy a single BB or a whole black-boxed
>             SESE region when re-generating the gimple.
>
>             * sese.h (bb_bb_def): New.
>             (gimple_bb): Replaced BB by ENTRY and EXIT for SESE region
>             handling.
>             (GBB_BB): Removed.
>             (GBB_ENTRY, GBB_EXIT): New.
>             (gbb_loop): Adapted.
>             (gbb_from_bb): Moved from graphite-sese-to-poly.c.
>
>             * graphite-clast-to-gimple.c (translate_clast_user): Adapted.
>             * graphite-poly.c (print_pbb_domain, dump_gbb_cases,
>             dump_gbb_conditions, loop_to_lst, scop_to_lst): Adapted.
>             * graphite-poly.h (poly_bb, pbb_bb): Adapted and commented.
>
>             * graphite-sese-to-poly.c (new_gimple_bb, free_gimple_bb):
>             Constructor and destructor updated.
>             (try_generate_gimple_bb): Added traversal of the SESE region to
>             gather data references for the new case.  The function now also
>             marks the visited BBs itself (and takes the bitmap as parameter)
>             as it may need to mark all the BBs in the SESE.
>             (build_scop_bbs_1): Adapted.
>             (find_params_in_bb): Replaced old GBB accessor.
>             (gbb_from_bb): Moved to sese.h as it is needed.
>
>



More information about the Gcc-patches mailing list