This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [graphite] Move to cloog.org interface
On 08/11/2011 07:16 PM, Sebastian Pop wrote:
On Thu, Aug 11, 2011 at 13:03, Sebastian Pop<sebpop@gmail.com> wrote:
> On Thu, Aug 11, 2011 at 12:52, Tobias Grosser<tobias@grosser.es> wrote:
>> I will commit this patch after the configure changes are in (and meanwhile
>> no further improvements were suggested for this patch).
>
> Ok, thanks. Let's hope we will have a configure maintainer that has some
> spare cycles to go over your first 3 patches.
>
> I will post my patches that convert graphite to ISL on top of your patches.
I am following the PET (Polyhedral Extraction Tool) git://repo.or.cz/pet.git
code as suggested by Tobias and Sven in order to help with the translation
of graphite to ISL.
Here is where I am right now: I am building the ISL counterpart for
scop->context
pbb->domain
pdr->accesses
and I still have to translate to ISL the original and transformed scattering.
The plan is to build the ISL representation in parallel with PPL data structs,
then make sure that ISL sets and maps contain valid data, and then move
away from PPL data structures one by one.
I would appreciate preliminary remarks on these patches.
Hey,
wonderful. Here my first remarks:
0005-Remove-ATTRIBUTE_UNUSED.patch
No comment.
0006-Add-ISL-data-structures.patch
0007-Add-scop-context.patch
/* Compute the lower bound LOW and upper bound UP for the induction
@@ -1360,10 +1368,32 @@ build_cloog_prog (scop_p scop, CloogProgram *prog,
This needs to be adapted to my cloog.org interface patch.
+/* Prints an isl_set S to stderr. */
+
+DEBUG_FUNCTION void
+debug_isl_set (isl_set *s)
+{
+ isl_ctx *ctx = isl_ctx_alloc ();
+ isl_printer *pp = isl_printer_to_file (ctx, stderr);
+ pp = isl_printer_print_set (pp, s);
+ isl_printer_free (pp);
+ isl_ctx_free (ctx);
+}
+
+/* Prints an isl_pw_aff A to stderr. */
+
+DEBUG_FUNCTION void
+debug_isl_pwaff (isl_pw_aff *a)
+{
+ isl_ctx *ctx = isl_ctx_alloc ();
+ isl_printer *pp = isl_printer_to_file (ctx, stderr);
+ pp = isl_printer_print_pw_aff (pp, a);
+ isl_printer_free (pp);
+ isl_ctx_free (ctx);
+}
+
+/* Prints an isl_aff A to stderr. */
+
+DEBUG_FUNCTION void
+debug_isl_aff (isl_aff *a)
+{
+ isl_ctx *ctx = isl_ctx_alloc ();
+ isl_printer *pp = isl_printer_to_file (ctx, stderr);
+ pp = isl_printer_print_aff (pp, a);
+ isl_printer_free (pp);
+ isl_ctx_free (ctx);
+}
No need to define these. isl_set_dump(isl_set *) should do what you
want. Similar functions are defined for other data types.
Furthermore, gdb should also automatically an islprint method, that
allows you to dump (almost) any isl type.
+static inline bool
+isl_set_is_equal_ppl_polyhedron (isl_set *s1, ppl_const_Polyhedron_t ph,
+ int nb_params, CloogState *state)
+{
+ isl_set *s2 = isl_set_from_cloog_domain
+ (new_Cloog_Domain_from_ppl_Polyhedron (ph, nb_params, state));
+ int res = isl_set_is_equal (s1, s2);
+ isl_set_free (s2);
+ return res;
+}
Clever. ;-)
+/*
+ gcc_assert (isl_set_is_equal_ppl_powerset (scop->context,
+ SCOP_CONTEXT (scop),
+ scop_nb_params (scop),
+ cloog_state));
+*/
Seems to be useless.
#endif /* GRAPHITE_CLOOG_UTIL_H */
diff --git a/gcc/graphite-poly.c b/gcc/graphite-poly.c
index af40d20..225c8a2 100644
--- a/gcc/graphite-poly.c
+++ b/gcc/graphite-poly.c
@@ -1012,6 +1012,7 @@ new_scop (void *region)
scop_p scop = XNEW (struct scop);
SCOP_CONTEXT (scop) = NULL;
+ scop->context = NULL;
scop_set_region (scop, region);
SCOP_BBS (scop) = VEC_alloc (poly_bb_p, heap, 3);
SCOP_ORIGINAL_PDDRS (scop) = htab_create (10, hash_poly_ddr_p,
@@ -1040,6 +1041,9 @@ free_scop (scop_p scop)
if (SCOP_CONTEXT (scop))
ppl_delete_Pointset_Powerset_C_Polyhedron (SCOP_CONTEXT (scop));
+ if (scop->context)
+ isl_set_free (scop->context);
The if is most probably not needed. Sven recently stated, that passing
NULL to
isl_*_free functions has defined semantics.
diff --git a/gcc/graphite-poly.h b/gcc/graphite-poly.h
index 3a5fddb..775c853 100644
--- a/gcc/graphite-poly.h
+++ b/gcc/graphite-poly.h
@@ -1409,6 +1409,9 @@ struct scop
ppl_Pointset_Powerset_C_Polyhedron_t _context;
isl_set *context;
+ /* The context used internally by ISL. */
+ isl_ctx *ctx;
+
/* A hashtable of the data dependence relations for the original
scattering. */
htab_t original_pddrs;
+/* Return an ISL identifier from the name of the ssa_name E. */
+
+static isl_id *
+isl_id_for_ssa_name (scop_p s, tree e)
+{
+ const char *name = get_name (e);
+ isl_id *id;
+
+ if (name)
+ id = isl_id_alloc (s->ctx, name, e);
Does get_name() return always a unique name or is just the tuple
(get_name(e), SSA_NAME_VERSION(e)) unique?
+/* Return an ISL identifier from the name of the ssa_name E. */
+
+static isl_id *
+isl_id_for_loop (scop_p s, loop_p l)
The comment for this function does not match.
+/* Compute pwaff mod 2^width. */
+
+static isl_pw_aff *
+wrap (isl_pw_aff *pwaff, unsigned width)
+{
+ isl_int mod;
+
+ isl_int_init (mod);
+ isl_int_set_si (mod, 1);
+ isl_int_mul_2exp (mod, mod, width);
+
+ pwaff = isl_pw_aff_mod (pwaff, mod);
+
+ isl_int_clear (mod);
+
+ return pwaff;
+}
You may just want to use isl_pw_aff_mod().
diff --git a/gcc/graphite.c b/gcc/graphite.c
index 8f6d8a1..b2cf7c6 100644
--- a/gcc/graphite.c
+++ b/gcc/graphite.c
@@ -260,10 +260,12 @@ graphite_transform_loops (void)
bool need_cfg_cleanup_p = false;
VEC (scop_p, heap) *scops = NULL;
htab_t bb_pbb_mapping;
+ isl_ctx *ctx;
if (!graphite_initialize ())
return;t
+ ctx = isl_ctx_alloc ();
build_scops (&scops);
if (dump_file&& (dump_flags& TDF_DETAILS))
@@ -277,6 +279,7 @@ graphite_transform_loops (void)
FOR_EACH_VEC_ELT (scop_p, scops, i, scop)
if (dbg_cnt (graphite_scop))
{
+ scop->ctx = ctx;
build_poly_scop (scop);
if (POLY_SCOP_P (scop)
@@ -288,6 +291,7 @@ graphite_transform_loops (void)
htab_delete (bb_pbb_mapping);
free_scops (scops);
graphite_finalize (need_cfg_cleanup_p);
+ isl_ctx_free (ctx);
We should check with Sven if there will be any troubles/problems,
because ClooG is allocating its own context and we are passing
isl_objects between those two contexts.
I think the best would be to provide our ctx to cloog when allocating
the CloogState.
0008-fix-memory-leak.patch
No comment.
0009-add-pbb-domain.patch
diff --git a/gcc/graphite-poly.c b/gcc/graphite-poly.c
index 225c8a2..c5b32d6 100644
--- a/gcc/graphite-poly.c
+++ b/gcc/graphite-poly.c
@@ -901,7 +902,11 @@ free_poly_bb (poly_bb_p pbb)
int i;
poly_dr_p pdr;
- ppl_delete_Pointset_Powerset_C_Polyhedron (PBB_DOMAIN (pbb));
+ if (PBB_DOMAIN (pbb))
+ ppl_delete_Pointset_Powerset_C_Polyhedron (PBB_DOMAIN (pbb));
+
+ if (pbb->domain)
+ isl_set_free (pbb->domain);
The if is not needed here.
if (PBB_TRANSFORMED (pbb))
poly_scattering_free (PBB_TRANSFORMED (pbb));
@@ -1060,6 +1065,9 @@ openscop_print_pbb_domain (FILE *file, poly_bb_p pbb, int verbosity)
graphite_dim_t i;
gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
+ if (isl_set_plain_is_empty (pbb->domain))
+ return;
Why do we return if a domain is empty. There may be cases where the
domain is empty because some constraints show us that the code will
never be executed. Still, I think it is good to show that this PBB
has a valid, though empty, domain.
0010-add-pdr-accesses-and-pdr-extent.patch
+/* Prints an isl_map M to stderr. */
+
+DEBUG_FUNCTION void
+debug_isl_map (isl_map *m)
+{
+ isl_ctx *ctx = isl_ctx_alloc ();
+ isl_printer *pp = isl_printer_to_file (ctx, stderr);
+ pp = isl_printer_print_map (pp, m);
+ isl_printer_free (pp);
+ isl_ctx_free (ctx);
+}
+
Not needed. Use isl_map_dump(isl_map*).
+DEBUG_FUNCTION void
+debug_isl_id (isl_id *i)
+{
+ isl_ctx *ctx = isl_ctx_alloc ();
+ isl_printer *pp = isl_printer_to_file (ctx, stderr);
+ pp = isl_printer_print_id (pp, i);
+ isl_printer_free (pp);
+ isl_ctx_free (ctx);
+}
Does isl also have a dump function for this?
That's from my side. I am extremely impressed by your progress, and
believe this move makes the code of graphite both a lot more readable
and hopefully also a lot more correct.
Cheers and thanks for all the work!
Tobi