Re: [GSoC] Addition of ISL AST generation to Graphite
On July 20, 2014 1:29:30 PM CEST, Roman Gareev wrote: >> I am not aware of any problems with isl 0.12 and would be surprised >if such >> problems exist. Are you? > >I'm not aware of them, too. > >> P.S: As Richard suggested, we may also want to forbid CLooG 0.17. > >I've attached the patch, which adds the requirement for ClooG 0.18.1. >Is it fine for trunk? Great. Yes, I think it's fine for trunk. Tobias > >-- > Cheers, Roman Gareev. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [GSoC] Addition of ISL AST generation to Graphite
> I am not aware of any problems with isl 0.12 and would be surprised if such > problems exist. Are you? I'm not aware of them, too. > P.S: As Richard suggested, we may also want to forbid CLooG 0.17. I've attached the patch, which adds the requirement for ClooG 0.18.1. Is it fine for trunk? -- Cheers, Roman Gareev. 2014-07-20 Roman Gareev * configure.ac: Accept only CLooG 0.18.1. * configure: Regenerate. Index: configure === --- configure (revision 212861) +++ configure (working copy) @@ -6031,8 +6031,8 @@ CFLAGS="${_cloog_saved_CFLAGS} ${clooginc} ${islinc} ${gmpinc}" LDFLAGS="${_cloog_saved_LDFLAGS} ${clooglibs} ${isllibs} ${gmplib}" -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for version 0.17.0 of CLooG" >&5 -$as_echo_n "checking for version 0.17.0 of CLooG... " >&6; } +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for version 0.18.1 of CLooG" >&5 +$as_echo_n "checking for version 0.18.1 of CLooG... " >&6; } cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ #include "cloog/version.h" @@ -6040,50 +6040,8 @@ main () { #if CLOOG_VERSION_MAJOR != 0 \ -|| CLOOG_VERSION_MINOR != 17 \ -|| CLOOG_VERSION_REVISION < 0 -choke me - #endif - ; - return 0; -} -_ACEOF -if ac_fn_c_try_compile "$LINENO"; then : - gcc_cv_cloog=yes -else - gcc_cv_cloog=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_cloog" >&5 -$as_echo "$gcc_cv_cloog" >&6; } - -CFLAGS=$_cloog_saved_CFLAGS -LDFLAGS=$_cloog_saved_LDFLAGS - fi - - -if test "${gcc_cv_cloog}" = no ; then - - - - if test "${ENABLE_CLOOG_CHECK}" = yes ; then -_cloog_saved_CFLAGS=$CFLAGS -_cloog_saved_LDFLAGS=$LDFLAGS - -CFLAGS="${_cloog_saved_CFLAGS} ${clooginc} ${islinc} ${gmpinc}" -LDFLAGS="${_cloog_saved_LDFLAGS} ${clooglibs} ${isllibs} ${gmplib}" - -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for version 0.18.0 of CLooG" >&5 -$as_echo_n "checking for version 0.18.0 of CLooG... " >&6; } -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ -#include "cloog/version.h" -int -main () -{ -#if CLOOG_VERSION_MAJOR != 0 \ || CLOOG_VERSION_MINOR != 18 \ -|| CLOOG_VERSION_REVISION < 0 +|| CLOOG_VERSION_REVISION < 1 choke me #endif ; @@ -6104,7 +6062,6 @@ fi -fi Index: configure.ac === --- configure.ac(revision 212861) +++ configure.ac(working copy) @@ -1661,10 +1661,7 @@ dnl with user input. CLOOG_INIT_FLAGS dnl The versions of CLooG that work for Graphite. -CLOOG_CHECK_VERSION(0,17,0) -if test "${gcc_cv_cloog}" = no ; then - CLOOG_CHECK_VERSION(0,18,0) -fi +CLOOG_CHECK_VERSION(0,18,1) dnl Only execute fail-action, if CLooG has been requested. CLOOG_IF_FAILED([
Re: [GSoC] Addition of ISL AST generation to Graphite
On 17/07/2014 16:11, Roman Gareev wrote: I've attached the patch, which adds the requirement for isl 0.12. Tobias, is it important to accept only 0.12.1, 0.12.2 and forbid 0.12? I am not aware of any problems with isl 0.12 and would be surprised if such problems exist. Are you? The patch itself looks good. As it is trivial, fixing an annoying bootstrapping bug, and people agreed that this is the right direction, I propose that you commit it right ahead. Further reviews are still welcome. Cheers, Tobias P.S: As Richard suggested, we may also want to forbid CLooG 0.17.
Re: [GSoC] Addition of ISL AST generation to Graphite
I've attached the patch, which adds the requirement for isl 0.12. Tobias, is it important to accept only 0.12.1, 0.12.2 and forbid 0.12? -- Cheers, Roman Gareev 2014-07-12 Roman Gareev * configure.ac: Don't accept isl 0.11. * configure: Regenerate. Index: configure === --- configure (revision 212756) +++ configure (working copy) @@ -5898,54 +5898,6 @@ LDFLAGS="${_isl_saved_LDFLAGS} ${isllibs}" LIBS="${_isl_saved_LIBS} -lisl" -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for version 0.11 of ISL" >&5 -$as_echo_n "checking for version 0.11 of ISL... " >&6; } -if test "$cross_compiling" = yes; then : - gcc_cv_isl=yes -else - cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ -#include - #include -int -main () -{ -if (strncmp (isl_version (), "isl-0.11", strlen ("isl-0.11")) != 0) - return 1; - - ; - return 0; -} -_ACEOF -if ac_fn_c_try_run "$LINENO"; then : - gcc_cv_isl=yes -else - gcc_cv_isl=no -fi -rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \ - conftest.$ac_objext conftest.beam conftest.$ac_ext -fi - -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_isl" >&5 -$as_echo "$gcc_cv_isl" >&6; } - -CFLAGS=$_isl_saved_CFLAGS -LDFLAGS=$_isl_saved_LDFLAGS -LIBS=$_isl_saved_LIBS - fi - - - if test "${gcc_cv_isl}" = no ; then - - if test "${ENABLE_ISL_CHECK}" = yes ; then -_isl_saved_CFLAGS=$CFLAGS -_isl_saved_LDFLAGS=$LDFLAGS -_isl_saved_LIBS=$LIBS - -CFLAGS="${_isl_saved_CFLAGS} ${islinc} ${gmpinc}" -LDFLAGS="${_isl_saved_LDFLAGS} ${isllibs}" -LIBS="${_isl_saved_LIBS} -lisl" - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for version 0.12 of ISL" >&5 $as_echo_n "checking for version 0.12 of ISL... " >&6; } if test "$cross_compiling" = yes; then : @@ -5983,7 +5935,6 @@ fi - fi Index: configure.ac === --- configure.ac(revision 212756) +++ configure.ac(working copy) @@ -1650,10 +1650,7 @@ dnl with user input. ISL_INIT_FLAGS dnl The versions of ISL that work for Graphite - ISL_CHECK_VERSION(0,11) - if test "${gcc_cv_isl}" = no ; then -ISL_CHECK_VERSION(0,12) - fi + ISL_CHECK_VERSION(0,12) dnl Only execute fail-action, if ISL has been requested. ISL_IF_FAILED([ AC_MSG_ERROR([Unable to find a usable ISL. See config.log for details.])])
Re: [GSoC] Addition of ISL AST generation to Graphite
On 16/07/2014 14:03, Roman Gareev wrote: Could you prepare such a patch? Yes, I've started working on it. I have a question about isl. Can we require that isl is always compiled with GMP support? isl 0.12.2 is always compiled with GMP support. Only for the unreleased isl-0.14 imath was introduced as an alternative. Cheers, Tobias
Re: [GSoC] Addition of ISL AST generation to Graphite
> Could you prepare such a patch? Yes, I've started working on it. I have a question about isl. Can we require that isl is always compiled with GMP support? -- Cheers, Roman Gareev
Re: [GSoC] Addition of ISL AST generation to Graphite
On 16/07/2014 11:36, Richard Biener wrote: On Tue, Jul 15, 2014 at 5:34 PM, Tobias Grosser wrote: On 15/07/2014 17:02, Roman Gareev wrote: I'm seeing the error: gcc/graphite-isl-ast-to-gimple.c:31:25: warning: isl/val_gmp.h: No such file or directory when building for aarch64. isl/val_gmp.h is included in 0.12 AFAICS so perhaps we should demand 0.12 instead of 0.11 ? According to isl's ChangeLog, isl_val abstraction was added in version 0.12. Therefore, I think it would be right to demand on 0.12. Tobias, what do you think about this? Is this fine for the backend, which uses CLooG to generate Gimple code? I think so. The latest release of CLooG (0.18.1) was released with isl 0.12.1 and is part of ftp://gcc.gnu.org/pub/gcc/infrastructure/. So requiring isl 0.12.1 sounds reasonable. Could you prepare such a patch? Note that we also still accept CLooG 0.17.0. In this case we need to update the minimal required CLooG version to CLooG 0.18.1, right? > Btw, it's unfortunate that ISL 0.13 cannot be used because it dropped some APIs we use (it's important for testing on branches that a single cloog/isl version can be used to bootstrap and test on active branches and trunk). Right. I also believe this is very unfortunate. I copy Sven to let him now that dropping APIs too quickly regularly causes issues. Cheers, Tobias
Re: [GSoC] Addition of ISL AST generation to Graphite
On Tue, Jul 15, 2014 at 5:34 PM, Tobias Grosser wrote: > On 15/07/2014 17:02, Roman Gareev wrote: >>> >>> I'm seeing the error: >>> >>> gcc/graphite-isl-ast-to-gimple.c:31:25: warning: isl/val_gmp.h: No such >>> file >>> or directory >>> >>> when building for aarch64. >>> >>> isl/val_gmp.h is included in 0.12 AFAICS so perhaps we should demand 0.12 >>> instead of 0.11 ? >> >> >> According to isl's ChangeLog, isl_val abstraction was added in version >> 0.12. Therefore, I think it would be right to demand on 0.12. >> >> Tobias, what do you think about this? Is this fine for the backend, >> which uses CLooG to generate Gimple code? > > > I think so. The latest release of CLooG (0.18.1) was released with isl > 0.12.1 and is part of ftp://gcc.gnu.org/pub/gcc/infrastructure/. So > requiring isl 0.12.1 sounds reasonable. > > Could you prepare such a patch? Note that we also still accept CLooG 0.17.0. Btw, it's unfortunate that ISL 0.13 cannot be used because it dropped some APIs we use (it's important for testing on branches that a single cloog/isl version can be used to bootstrap and test on active branches and trunk). Richard. > Cheers, > Tobias
Re: [GSoC] Addition of ISL AST generation to Graphite
On 15/07/2014 17:02, Roman Gareev wrote: I'm seeing the error: gcc/graphite-isl-ast-to-gimple.c:31:25: warning: isl/val_gmp.h: No such file or directory when building for aarch64. isl/val_gmp.h is included in 0.12 AFAICS so perhaps we should demand 0.12 instead of 0.11 ? According to isl's ChangeLog, isl_val abstraction was added in version 0.12. Therefore, I think it would be right to demand on 0.12. Tobias, what do you think about this? Is this fine for the backend, which uses CLooG to generate Gimple code? I think so. The latest release of CLooG (0.18.1) was released with isl 0.12.1 and is part of ftp://gcc.gnu.org/pub/gcc/infrastructure/. So requiring isl 0.12.1 sounds reasonable. Could you prepare such a patch? Cheers, Tobias
Re: [GSoC] Addition of ISL AST generation to Graphite
> I'm seeing the error: > > gcc/graphite-isl-ast-to-gimple.c:31:25: warning: isl/val_gmp.h: No such file > or directory > > when building for aarch64. > > isl/val_gmp.h is included in 0.12 AFAICS so perhaps we should demand 0.12 > instead of 0.11 ? According to isl's ChangeLog, isl_val abstraction was added in version 0.12. Therefore, I think it would be right to demand on 0.12. Tobias, what do you think about this? Is this fine for the backend, which uses CLooG to generate Gimple code? -- Cheers, Roman Gareev
Re: [GSoC] Addition of ISL AST generation to Graphite
On 05/07/14 21:20, Rainer Orth wrote: Rainer Orth writes: Roman Gareev writes: It seems the patch1/patch2 files you attach have the Content-Type: application/octet-stream. This makes it impossible to view them inline. Could you send them as text files? Just calling them patch1.patch or patch1.txt should make this work. Yes, sure. This patch broke bootstrap with --enable-cloog-backend=isl when using isl 0.10: /vol/gcc/src/hg/trunk/local/gcc/graphite-isl-ast-to-gimple.c:27:27: fatal error: isl/ast_build.h: No such file or directory compilation terminated. make[3]: *** [graphite-isl-ast-to-gimple.o] Error 1 is missing in in isl 0.10. Seems it was introduced in 0.11, but no idea if that works. Toplevel configure.ac accepts 0.10, while install.texi states 0.12.2 is required. Configuration and/or docs should be changed to match reality. I've now commited the following patch as obvious: it requires isl 0.11 which is the first that includes isl/ast_build.h. I'm seeing the error: gcc/graphite-isl-ast-to-gimple.c:31:25: warning: isl/val_gmp.h: No such file or directory when building for aarch64. isl/val_gmp.h is included in 0.12 AFAICS so perhaps we should demand 0.12 instead of 0.11 ? Kyrill Bootstrapped on i386-pc-solaris2.11 with isl 0.12.2. Rainer 2014-07-05 Rainer Orth * configure.ac: Don't accept isl 0.10. * configure: Regenerate.
Re: [GSoC] Addition of ISL AST generation to Graphite
Sorry, I was going to fix this. Thank you very much! -- Cheers, Roman Gareev
Re: [GSoC] Addition of ISL AST generation to Graphite
Rainer Orth writes: > Roman Gareev writes: > >>> It seems the patch1/patch2 files you attach have the Content-Type: >>> application/octet-stream. This makes it impossible to view them inline. >>> Could you send them as text files? Just calling them patch1.patch or >>> patch1.txt should make this work. >> >> Yes, sure. > > This patch broke bootstrap with --enable-cloog-backend=isl when using > isl 0.10: > > /vol/gcc/src/hg/trunk/local/gcc/graphite-isl-ast-to-gimple.c:27:27: fatal > error: isl/ast_build.h: No such file or directory > compilation terminated. > make[3]: *** [graphite-isl-ast-to-gimple.o] Error 1 > > is missing in in isl 0.10. Seems it was introduced in > 0.11, but no idea if that works. > > Toplevel configure.ac accepts 0.10, while install.texi states 0.12.2 is > required. Configuration and/or docs should be changed to match reality. I've now commited the following patch as obvious: it requires isl 0.11 which is the first that includes isl/ast_build.h. Bootstrapped on i386-pc-solaris2.11 with isl 0.12.2. Rainer 2014-07-05 Rainer Orth * configure.ac: Don't accept isl 0.10. * configure: Regenerate. diff --git a/configure.ac b/configure.ac --- a/configure.ac +++ b/configure.ac @@ -1650,12 +1650,9 @@ if test "x$with_isl" != "xno" && dnl with user input. ISL_INIT_FLAGS dnl The versions of ISL that work for Graphite - ISL_CHECK_VERSION(0,10) + ISL_CHECK_VERSION(0,11) if test "${gcc_cv_isl}" = no ; then -ISL_CHECK_VERSION(0,11) -if test "${gcc_cv_isl}" = no ; then - ISL_CHECK_VERSION(0,12) -fi +ISL_CHECK_VERSION(0,12) fi dnl Only execute fail-action, if ISL has been requested. ISL_IF_FAILED([ -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [GSoC] Addition of ISL AST generation to Graphite
Roman Gareev writes: >> It seems the patch1/patch2 files you attach have the Content-Type: >> application/octet-stream. This makes it impossible to view them inline. >> Could you send them as text files? Just calling them patch1.patch or >> patch1.txt should make this work. > > Yes, sure. This patch broke bootstrap with --enable-cloog-backend=isl when using isl 0.10: /vol/gcc/src/hg/trunk/local/gcc/graphite-isl-ast-to-gimple.c:27:27: fatal error: isl/ast_build.h: No such file or directory compilation terminated. make[3]: *** [graphite-isl-ast-to-gimple.o] Error 1 is missing in in isl 0.10. Seems it was introduced in 0.11, but no idea if that works. Toplevel configure.ac accepts 0.10, while install.texi states 0.12.2 is required. Configuration and/or docs should be changed to match reality. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [GSoC] Addition of ISL AST generation to Graphite
On Sun, 29 Jun 2014, Tobias Grosser wrote: sorry for the breakage. This seems to be caused by forgotten include guards. I attached a patch (not bootstrapped yet), that should fix the issue. I will start a gcc (non-graphite) bootstrap to see if it works and will commit it immediately after. Please report back if it fixes the bootstrap for you. Thanks for the quick response, Tobias. I confirm that it fixes bootstrap on the system that showed the breakage originally. Gerald
Re: [GSoC] Addition of ISL AST generation to Graphite
On 29/06/2014 19:31, Gerald Pfeifer wrote: I'm pretty it's the following that causes bootstrap to fail for me: 2014-06-29 Roman Gareev * Makefile.in: Add the compilation of graphite-isl-ast-to-gimple.o. * common.opt: Add new switch fgraphite-code-generator=[isl|cloog]. * flag-types.h: Add new enum fgraphite_generator. * graphite-isl-ast-to-gimple.c: New. * graphite-isl-ast-to-gimple.h: New. * graphite.c (graphite_transform_loops): Add choice of Graphite code generator, which depends on flag_graphite_code_gen. testsuite/gcc.dg/graphite/isl-codegen-loop-dumping.c: New testcase that checks that the dump is generated. As follows: /scratch2/tmp/gerald/gcc-HEAD/gcc/graphite-isl-ast-to-gimple.c:23:10: fatal error: 'isl/set.h' file not found #include ^ I do not have ISL installed on that machine and I don't want to use graphite on that machine. PR 61650 - [4.10] Bootstrap failure due to dependency on ISL. Hi Gerald, sorry for the breakage. This seems to be caused by forgotten include guards. I attached a patch (not bootstrapped yet), that should fix the issue. I will start a gcc (non-graphite) bootstrap to see if it works and will commit it immediately after. Please report back if it fixes the bootstrap for you. Cheers, Tobias >From 2910e41368c3895a7a023133a1fcb87772b838a8 Mon Sep 17 00:00:00 2001 From: Tobias Grosser Date: Sun, 29 Jun 2014 19:34:30 +0200 Subject: [PATCH] Fix gcc bootstrap gcc/ * graphite-isl-ast-to-gimple.c: Add cloog include guards. --- gcc/graphite-isl-ast-to-gimple.c | 4 1 file changed, 4 insertions(+) diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c index 6437474..8ba7b75 100644 --- a/gcc/graphite-isl-ast-to-gimple.c +++ b/gcc/graphite-isl-ast-to-gimple.c @@ -20,10 +20,12 @@ along with GCC; see the file COPYING3. If not see #include "config.h" +#ifdef HAVE_cloog #include #include #include #include +#endif #include "system.h" #include "coretypes.h" @@ -41,6 +43,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-data-ref.h" #include "sese.h" +#ifdef HAVE_cloog #include "graphite-poly.h" #include "graphite-isl-ast-to-gimple.h" @@ -131,3 +134,4 @@ graphite_regenerate_ast_isl (scop_p scop) timevar_pop (TV_GRAPHITE_CODE_GEN); return !graphite_regenerate_error; } +#endif -- 1.8.3.2
Re: [GSoC] Addition of ISL AST generation to Graphite
I'm pretty it's the following that causes bootstrap to fail for me: 2014-06-29 Roman Gareev * Makefile.in: Add the compilation of graphite-isl-ast-to-gimple.o. * common.opt: Add new switch fgraphite-code-generator=[isl|cloog]. * flag-types.h: Add new enum fgraphite_generator. * graphite-isl-ast-to-gimple.c: New. * graphite-isl-ast-to-gimple.h: New. * graphite.c (graphite_transform_loops): Add choice of Graphite code generator, which depends on flag_graphite_code_gen. testsuite/gcc.dg/graphite/isl-codegen-loop-dumping.c: New testcase that checks that the dump is generated. As follows: /scratch2/tmp/gerald/gcc-HEAD/gcc/graphite-isl-ast-to-gimple.c:23:10: fatal error: 'isl/set.h' file not found #include ^ I do not have ISL installed on that machine and I don't want to use graphite on that machine. PR 61650 - [4.10] Bootstrap failure due to dependency on ISL. Gerald
Re: [GSoC] Addition of ISL AST generation to Graphite
> It seems the patch1/patch2 files you attach have the Content-Type: > application/octet-stream. This makes it impossible to view them inline. > Could you send them as text files? Just calling them patch1.patch or > patch1.txt should make this work. Yes, sure. diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c index 9ac9b67..49b7bc6 100644 --- a/gcc/graphite-clast-to-gimple.c +++ b/gcc/graphite-clast-to-gimple.c @@ -109,7 +109,7 @@ value_max (mpz_t res, mpz_t v1, mpz_t v2) /* This flag is set when an error occurred during the translation of CLAST to Gimple. */ -static bool gloog_error; +static bool graphite_regenerate_error; /* Verifies properties that GRAPHITE should maintain during translation. */ @@ -363,7 +363,7 @@ max_precision_type (tree type1, tree type2) if (precision > BITS_PER_WORD) { - gloog_error = true; + graphite_regenerate_error = true; return integer_type_node; } @@ -373,7 +373,7 @@ max_precision_type (tree type1, tree type2) if (!type) { - gloog_error = true; + graphite_regenerate_error = true; return integer_type_node; } @@ -456,7 +456,7 @@ clast_to_gcc_expression (tree type, struct clast_expr *e, ivs_params_p ip) if (!POINTER_TYPE_P (type)) return fold_build2 (MULT_EXPR, type, cst, name); - gloog_error = true; + graphite_regenerate_error = true; return cst; } } @@ -535,7 +535,7 @@ type_for_interval (mpz_t bound_one, mpz_t bound_two) if (precision > BITS_PER_WORD) { - gloog_error = true; + graphite_regenerate_error = true; return integer_type_node; } @@ -558,7 +558,7 @@ type_for_interval (mpz_t bound_one, mpz_t bound_two) if (!type) { - gloog_error = true; + graphite_regenerate_error = true; return integer_type_node; } @@ -1112,7 +1112,7 @@ translate_clast_user (struct clast_user_stmt *stmt, edge next_e, build_iv_mapping (iv_map, stmt, ip); next_e = copy_bb_and_scalar_dependences (GBB_BB (gbb), ip->region, - next_e, iv_map, &gloog_error); + next_e, iv_map, &graphite_regenerate_error); iv_map.release (); new_bb = next_e->src; @@ -1488,7 +1488,7 @@ build_cloog_union_domain (scop_p scop, int nb_scattering_dims) return union_domain; } -/* Return the options that will be used in GLOOG. */ +/* Return the options that will be used in graphite_regenerate_ast_cloog. */ static CloogOptions * set_cloog_options (void) @@ -1503,7 +1503,7 @@ set_cloog_options (void) /* Enable complex equality spreading: removes dummy statements (assignments) in the generated code which repeats the substitution equations for statements. This is useless for - GLooG. */ + graphite_regenerate_ast_cloog. */ options->esp = 1; /* Silence CLooG to avoid failing tests due to debug output to stderr. */ @@ -1663,7 +1663,7 @@ debug_generated_program (scop_p scop) */ bool -gloog (scop_p scop, bb_pbb_htab_type bb_pbb_mapping) +graphite_regenerate_ast_cloog (scop_p scop, bb_pbb_htab_type bb_pbb_mapping) { auto_vec newivs; loop_p context_loop; @@ -1674,7 +1674,7 @@ gloog (scop_p scop, bb_pbb_htab_type bb_pbb_mapping) struct ivs_params ip; timevar_push (TV_GRAPHITE_CODE_GEN); - gloog_error = false; + graphite_regenerate_error = false; params_index.create (10); @@ -1714,7 +1714,7 @@ gloog (scop_p scop, bb_pbb_htab_type bb_pbb_mapping) recompute_all_dominators (); graphite_verify (); - if (gloog_error) + if (graphite_regenerate_error) set_ifsese_condition (if_region, integer_zero_node); free (if_region->true_region); @@ -1739,6 +1739,6 @@ gloog (scop_p scop, bb_pbb_htab_type bb_pbb_mapping) num_no_dependency); } - return !gloog_error; + return !graphite_regenerate_error; } #endif diff --git a/gcc/graphite-clast-to-gimple.h b/gcc/graphite-clast-to-gimple.h index fc5a679..615cae8 100644 --- a/gcc/graphite-clast-to-gimple.h +++ b/gcc/graphite-clast-to-gimple.h @@ -21,6 +21,8 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_GRAPHITE_CLAST_TO_GIMPLE_H #define GCC_GRAPHITE_CLAST_TO_GIMPLE_H +#include "graphite-htab.h" + extern CloogState *cloog_state; /* Data structure for CLooG program representation. */ @@ -30,14 +32,7 @@ struct cloog_prog_clast { struct clast_stmt *stmt; }; -/* Stores BB's related PBB. */ - -struct bb_pbb_def -{ - basic_block bb; - poly_bb_p pbb; -}; - +extern bool graphite_regenerate_ast_cloog (scop_p, bb_pbb_htab_type); extern void debug_clast_stmt (struct clast_stmt *); extern void print_clast_stmt (FILE *, struct clast_stmt *); diff --git a/gcc/graphite-htab.h b/gcc/graphite-htab.h index d67dd0c..9f31fac 100644 --- a/gcc/graphite-htab.h +++ b/gcc/graphite-htab.h @@ -22,7 +22,14 @@ along
Re: [GSoC] Addition of ISL AST generation to Graphite
Thank you for the review! -- Cheers, Roman Gareev ChangeLog_entry1 Description: Binary data ChangeLog_entry2 Description: Binary data patch1 Description: Binary data patch2 Description: Binary data
Re: [GSoC] Addition of ISL AST generation to Graphite
Thanks Sebastian for the review! It is good to see you again on the mailing list! On 23/06/2014 11:29, Sebastian Pop wrote: Please add a FIXME note in graphite_regenerate_ast_isl saying that this is not yet a full implementation of the code generator with ISL ASTs. It would be useful to make the current graphite_regenerate_ast_isl working by calling graphite_regenerate_ast_cloog and adding the fixme note above saying that we rely on the cloog code generator until we implement the ISL AST parsing. I would prefer to avoid this for the following two reasons: 1) Having a clear internal compiler error on unimplemented features allows us to easily understand which parts are working and which ones are not. 2) Going forward we will implement the isl ast generation step by step. To my understanding there is no reasonable way to do parts of the ast generation with isl and parts with CLooG. Developing such a hybrid generation seems not useful at all. Instead of developing the ast code generation step-by-step in the graphite source tree, we could develop it outside of gcc and only commit a single large patch performing the switch. I personally prefer the incremental development approach. There is also a minor code style issue in: isl_set * context_isl = isl_set_params (isl_set_copy (scop->context)); please remove the space after *: isl_set *context_isl = isl_set_params (isl_set_copy (scop->context)); Otherwise the two patches look good to me. Cheers, Tobias
Re: [GSoC] Addition of ISL AST generation to Graphite
Please add a FIXME note in graphite_regenerate_ast_isl saying that this is not yet a full implementation of the code generator with ISL ASTs. It would be useful to make the current graphite_regenerate_ast_isl working by calling graphite_regenerate_ast_cloog and adding the fixme note above saying that we rely on the cloog code generator until we implement the ISL AST parsing. There is also a minor code style issue in: isl_set * context_isl = isl_set_params (isl_set_copy (scop->context)); please remove the space after *: isl_set *context_isl = isl_set_params (isl_set_copy (scop->context)); Otherwise the two patches look good to me. Thanks, Sebastian On Wed, Jun 18, 2014 at 12:04 PM, Tobias Grosser wrote: > On 18/06/2014 21:00, Roman Gareev wrote: >> >> These patches add ISL AST generation to graphite, which can be chosen >> by the fgraphite-code-generator=[isl|cloog] switch. The first patch >> makes initial renaming of gloog and gloog_error to >> graphite_regenerate_ast_cloog and graphite_regenerate_error, >> respectively. The second one adds new files with generation of ISL >> AST, new switch, new testcase that checks that the dump is generated. >> >> Is it fine for trunk? > > > I went over this from the graphite side and it looks fine. However, > as I did not commit for a while to gcc, it would be great if someone else > could have a look. > > Cheers, > Tobias >
Re: [GSoC] Addition of ISL AST generation to Graphite
On 18/06/2014 21:00, Roman Gareev wrote: These patches add ISL AST generation to graphite, which can be chosen by the fgraphite-code-generator=[isl|cloog] switch. The first patch makes initial renaming of gloog and gloog_error to graphite_regenerate_ast_cloog and graphite_regenerate_error, respectively. The second one adds new files with generation of ISL AST, new switch, new testcase that checks that the dump is generated. Is it fine for trunk? I went over this from the graphite side and it looks fine. However, as I did not commit for a while to gcc, it would be great if someone else could have a look. Cheers, Tobias