diff mbox

fix PR53852: stop ISL after a given number of operations

Message ID 1441233255-24721-1-git-send-email-s.pop@samsung.com
State New
Headers show

Commit Message

Sebastian Pop Sept. 2, 2015, 10:34 p.m. UTC
2015-09-02  Sebastian Pop  <s.pop@samsung.com>

            * config.in: Regenerate.
            * configure: Regenerate.
            * configure.ac (HAVE_ISL_CTX_MAX_OPERATIONS): Detect.
            * graphite-optimize-isl.c (optimize_isl): Stop computation when
            PARAM_MAX_ISL_OPERATIONS is reached.
            * params.def (PARAM_MAX_ISL_OPERATIONS): Add.

            * graphite-dependences.c (extend_schedule): Remove gcc_asserts on
            result equal to isl_stat_ok as the status now can be isl_error_quota.
            (subtract_commutative_associative_deps): Same.
            (compute_deps): Same.

testsuite/
            * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to pass with
            both isl-0.12 and isl-0.15.
            * gcc.dg/graphite/uns-interchange-14.c: Same.
            * gcc.dg/graphite/uns-interchange-15.c: Same.
            * gcc.dg/graphite/uns-interchange-mvt.c: Same.
---
 gcc/config.in                                      |  6 ++
 gcc/configure                                      | 28 ++++++++
 gcc/configure.ac                                   | 11 +++
 gcc/graphite-dependences.c                         | 83 +++++++++-------------
 gcc/graphite-optimize-isl.c                        | 49 ++++++++-----
 gcc/params.def                                     |  5 ++
 gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c |  2 +-
 gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c |  2 +-
 gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c |  2 +-
 .../gcc.dg/graphite/uns-interchange-mvt.c          |  2 +-
 10 files changed, 120 insertions(+), 70 deletions(-)

Comments

Tobias Grosser Sept. 2, 2015, 10:52 p.m. UTC | #1
On 09/03/2015 12:34 AM, Sebastian Pop wrote:
> 2015-09-02  Sebastian Pop  <s.pop@samsung.com>
>
>              * config.in: Regenerate.
>              * configure: Regenerate.
>              * configure.ac (HAVE_ISL_CTX_MAX_OPERATIONS): Detect.
>              * graphite-optimize-isl.c (optimize_isl): Stop computation when
>              PARAM_MAX_ISL_OPERATIONS is reached.
>              * params.def (PARAM_MAX_ISL_OPERATIONS): Add.
>
>              * graphite-dependences.c (extend_schedule): Remove gcc_asserts on
>              result equal to isl_stat_ok as the status now can be isl_error_quota.
>              (subtract_commutative_associative_deps): Same.
>              (compute_deps): Same.
>
> testsuite/
>              * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to pass with
>              both isl-0.12 and isl-0.15.
>              * gcc.dg/graphite/uns-interchange-14.c: Same.
>              * gcc.dg/graphite/uns-interchange-15.c: Same.
>              * gcc.dg/graphite/uns-interchange-mvt.c: Same.

Hi Sebastian,

this looks good to me.

Tobias
Jeff Law Sept. 2, 2015, 11:08 p.m. UTC | #2
On 09/02/2015 04:52 PM, Tobias Grosser wrote:
> On 09/03/2015 12:34 AM, Sebastian Pop wrote:
>> 2015-09-02  Sebastian Pop  <s.pop@samsung.com>
>>
>>              * config.in: Regenerate.
>>              * configure: Regenerate.
>>              * configure.ac (HAVE_ISL_CTX_MAX_OPERATIONS): Detect.
>>              * graphite-optimize-isl.c (optimize_isl): Stop
>> computation when
>>              PARAM_MAX_ISL_OPERATIONS is reached.
>>              * params.def (PARAM_MAX_ISL_OPERATIONS): Add.
>>
>>              * graphite-dependences.c (extend_schedule): Remove
>> gcc_asserts on
>>              result equal to isl_stat_ok as the status now can be
>> isl_error_quota.
>>              (subtract_commutative_associative_deps): Same.
>>              (compute_deps): Same.
>>
>> testsuite/
>>              * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to
>> pass with
>>              both isl-0.12 and isl-0.15.
>>              * gcc.dg/graphite/uns-interchange-14.c: Same.
>>              * gcc.dg/graphite/uns-interchange-15.c: Same.
>>              * gcc.dg/graphite/uns-interchange-mvt.c: Same.
>
> Hi Sebastian,
>
> this looks good to me.
Thanks for taking care of this Sebastian!

jeff
Richard Biener Sept. 3, 2015, 9:46 a.m. UTC | #3
On Thu, Sep 3, 2015 at 12:34 AM, Sebastian Pop <s.pop@samsung.com> wrote:
> 2015-09-02  Sebastian Pop  <s.pop@samsung.com>
>
>             * config.in: Regenerate.
>             * configure: Regenerate.
>             * configure.ac (HAVE_ISL_CTX_MAX_OPERATIONS): Detect.
>             * graphite-optimize-isl.c (optimize_isl): Stop computation when
>             PARAM_MAX_ISL_OPERATIONS is reached.
>             * params.def (PARAM_MAX_ISL_OPERATIONS): Add.
>
>             * graphite-dependences.c (extend_schedule): Remove gcc_asserts on
>             result equal to isl_stat_ok as the status now can be isl_error_quota.
>             (subtract_commutative_associative_deps): Same.
>             (compute_deps): Same.
>
> testsuite/
>             * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to pass with
>             both isl-0.12 and isl-0.15.

Does it mean with 0.15 we now "time out" on some of the cases?  Or is this
just a general difference between 0.12 and 0.15?  In which case, like for
this testcase, is there a better way to verify whether the loops J and K were
interchanged?

>             * gcc.dg/graphite/uns-interchange-14.c: Same.
>             * gcc.dg/graphite/uns-interchange-15.c: Same.
>             * gcc.dg/graphite/uns-interchange-mvt.c: Same.
> ---
>  gcc/config.in                                      |  6 ++
>  gcc/configure                                      | 28 ++++++++
>  gcc/configure.ac                                   | 11 +++
>  gcc/graphite-dependences.c                         | 83 +++++++++-------------
>  gcc/graphite-optimize-isl.c                        | 49 ++++++++-----
>  gcc/params.def                                     |  5 ++
>  gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c |  2 +-
>  gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c |  2 +-
>  gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c |  2 +-
>  .../gcc.dg/graphite/uns-interchange-mvt.c          |  2 +-
>  10 files changed, 120 insertions(+), 70 deletions(-)
>
> diff --git a/gcc/config.in b/gcc/config.in
> index 22a4e6b..98c4647 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -1332,6 +1332,12 @@
>  #endif
>
>
> +/* Define if isl_ctx_get_max_operations exists. */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_ISL_CTX_MAX_OPERATIONS
> +#endif
> +
> +
>  /* Define if isl_options_set_schedule_serialize_sccs exists. */
>  #ifndef USED_FOR_TARGET
>  #undef HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS
> diff --git a/gcc/configure b/gcc/configure
> index 0d31383..07d39f9 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -28625,6 +28625,29 @@ rm -f core conftest.err conftest.$ac_objext \
>    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_has_isl_options_set_schedule_serialize_sccs" >&5
>  $as_echo "$ac_has_isl_options_set_schedule_serialize_sccs" >&6; }
>
> +  { $as_echo "$as_me:${as_lineno-$LINENO}: checking Checking for isl_ctx_get_max_operations" >&5
> +$as_echo_n "checking Checking for isl_ctx_get_max_operations... " >&6; }
> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +#include <isl/ctx.h>
> +int
> +main ()
> +{
> +isl_ctx_get_max_operations (isl_ctx_alloc ());
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_cxx_try_link "$LINENO"; then :
> +  ac_has_isl_ctx_get_max_operations=yes
> +else
> +  ac_has_isl_ctx_get_max_operations=no
> +fi
> +rm -f core conftest.err conftest.$ac_objext \
> +    conftest$ac_exeext conftest.$ac_ext
> +  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_has_isl_ctx_get_max_operations" >&5
> +$as_echo "$ac_has_isl_ctx_get_max_operations" >&6; }
> +
>    LIBS="$saved_LIBS"
>    CXXFLAGS="$saved_CXXFLAGS"
>
> @@ -28639,6 +28662,11 @@ $as_echo "#define HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE 1" >>confdefs.h
>  $as_echo "#define HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS 1" >>confdefs.h
>
>    fi
> +  if test x"$ac_has_isl_ctx_get_max_operations" = x"yes"; then
> +
> +$as_echo "#define HAVE_ISL_CTX_MAX_OPERATIONS 1" >>confdefs.h
> +
> +  fi
>  fi
>
>  # Check for plugin support
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 846651d..b6e8bed 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -5790,6 +5790,13 @@ if test "x${ISLLIBS}" != "x" ; then
>                [ac_has_isl_options_set_schedule_serialize_sccs=no])
>    AC_MSG_RESULT($ac_has_isl_options_set_schedule_serialize_sccs)
>
> +  AC_MSG_CHECKING([Checking for isl_ctx_get_max_operations])
> +  AC_TRY_LINK([#include <isl/ctx.h>],
> +              [isl_ctx_get_max_operations (isl_ctx_alloc ());],
> +              [ac_has_isl_ctx_get_max_operations=yes],
> +              [ac_has_isl_ctx_get_max_operations=no])
> +  AC_MSG_RESULT($ac_has_isl_ctx_get_max_operations)
> +
>    LIBS="$saved_LIBS"
>    CXXFLAGS="$saved_CXXFLAGS"
>
> @@ -5802,6 +5809,10 @@ if test "x${ISLLIBS}" != "x" ; then
>       AC_DEFINE(HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS, 1,
>                 [Define if isl_options_set_schedule_serialize_sccs exists.])
>    fi
> +  if test x"$ac_has_isl_ctx_get_max_operations" = x"yes"; then
> +     AC_DEFINE(HAVE_ISL_CTX_MAX_OPERATIONS, 1,
> +               [Define if isl_ctx_get_max_operations exists.])
> +  fi
>  fi
>
>  GCC_ENABLE_PLUGINS
> diff --git a/gcc/graphite-dependences.c b/gcc/graphite-dependences.c
> index c3c2090..85f16f3 100644
> --- a/gcc/graphite-dependences.c
> +++ b/gcc/graphite-dependences.c
> @@ -256,17 +256,12 @@ __isl_give isl_union_map *
>  extend_schedule (__isl_take isl_union_map *x)
>  {
>    int max = 0;
> -  isl_stat res;
>    struct extend_schedule_str str;
>
> -  res = isl_union_map_foreach_map (x, max_number_of_out_dimensions, (void *) &max);
> -  gcc_assert (res == isl_stat_ok);
> -
> +  isl_union_map_foreach_map (x, max_number_of_out_dimensions, (void *) &max);
>    str.max = max;
>    str.umap = isl_union_map_empty (isl_union_map_get_space (x));
> -  res = isl_union_map_foreach_map (x, extend_schedule_1, (void *) &str);
> -  gcc_assert (res == isl_stat_ok);
> -
> +  isl_union_map_foreach_map (x, extend_schedule_1, (void *) &str);
>    isl_union_map_free (x);
>    return str.umap;
>  }
> @@ -395,7 +390,6 @@ subtract_commutative_associative_deps (scop_p scop,
>    FOR_EACH_VEC_ELT (pbbs, i, pbb)
>      if (PBB_IS_REDUCTION (pbb))
>        {
> -       int res;
>         isl_union_map *r = isl_union_map_empty (isl_space_copy (space));
>         isl_union_map *must_w = isl_union_map_empty (isl_space_copy (space));
>         isl_union_map *may_w = isl_union_map_empty (isl_space_copy (space));
> @@ -432,27 +426,24 @@ subtract_commutative_associative_deps (scop_p scop,
>           (isl_union_map_copy (must_w), isl_union_map_copy (may_w));
>         empty = isl_union_map_empty (isl_union_map_get_space (all_w));
>
> -       res = isl_union_map_compute_flow (isl_union_map_copy (r),
> -                                         isl_union_map_copy (must_w),
> -                                         isl_union_map_copy (may_w),
> -                                         isl_union_map_copy (original),
> -                                         &x_must_raw, &x_may_raw,
> -                                         &x_must_raw_no_source,
> -                                         &x_may_raw_no_source);
> -       gcc_assert (res == 0);
> -       res = isl_union_map_compute_flow (isl_union_map_copy (all_w),
> -                                         r, empty,
> -                                         isl_union_map_copy (original),
> -                                         &x_must_war, &x_may_war,
> -                                         &x_must_war_no_source,
> -                                         &x_may_war_no_source);
> -       gcc_assert (res == 0);
> -       res = isl_union_map_compute_flow (all_w, must_w, may_w,
> -                                         isl_union_map_copy (original),
> -                                         &x_must_waw, &x_may_waw,
> -                                         &x_must_waw_no_source,
> -                                         &x_may_waw_no_source);
> -       gcc_assert (res == 0);
> +       isl_union_map_compute_flow (isl_union_map_copy (r),
> +                                   isl_union_map_copy (must_w),
> +                                   isl_union_map_copy (may_w),
> +                                   isl_union_map_copy (original),
> +                                   &x_must_raw, &x_may_raw,
> +                                   &x_must_raw_no_source,
> +                                   &x_may_raw_no_source);
> +       isl_union_map_compute_flow (isl_union_map_copy (all_w),
> +                                   r, empty,
> +                                   isl_union_map_copy (original),
> +                                   &x_must_war, &x_may_war,
> +                                   &x_must_war_no_source,
> +                                   &x_may_war_no_source);
> +       isl_union_map_compute_flow (all_w, must_w, may_w,
> +                                   isl_union_map_copy (original),
> +                                   &x_must_waw, &x_may_waw,
> +                                   &x_must_waw_no_source,
> +                                   &x_may_waw_no_source);
>
>         if (must_raw)
>           *must_raw = isl_union_map_subtract (*must_raw, x_must_raw);
> @@ -551,26 +542,22 @@ compute_deps (scop_p scop, vec<poly_bb_p> pbbs,
>    isl_space *space = isl_union_map_get_space (all_writes);
>    isl_union_map *empty = isl_union_map_empty (space);
>    isl_union_map *original = scop_get_original_schedule (scop, pbbs);
> -  int res;
>
> -  res = isl_union_map_compute_flow (isl_union_map_copy (reads),
> -                                   isl_union_map_copy (must_writes),
> -                                   isl_union_map_copy (may_writes),
> -                                   isl_union_map_copy (original),
> -                                   must_raw, may_raw, must_raw_no_source,
> -                                   may_raw_no_source);
> -  gcc_assert (res == 0);
> -  res = isl_union_map_compute_flow (isl_union_map_copy (all_writes),
> -                                   reads, empty,
> -                                   isl_union_map_copy (original),
> -                                   must_war, may_war, must_war_no_source,
> -                                   may_war_no_source);
> -  gcc_assert (res == 0);
> -  res = isl_union_map_compute_flow (all_writes, must_writes, may_writes,
> -                                   isl_union_map_copy (original),
> -                                   must_waw, may_waw, must_waw_no_source,
> -                                   may_waw_no_source);
> -  gcc_assert (res == 0);
> +  isl_union_map_compute_flow (isl_union_map_copy (reads),
> +                             isl_union_map_copy (must_writes),
> +                             isl_union_map_copy (may_writes),
> +                             isl_union_map_copy (original),
> +                             must_raw, may_raw, must_raw_no_source,
> +                             may_raw_no_source);
> +  isl_union_map_compute_flow (isl_union_map_copy (all_writes),
> +                             reads, empty,
> +                             isl_union_map_copy (original),
> +                             must_war, may_war, must_war_no_source,
> +                             may_war_no_source);
> +  isl_union_map_compute_flow (all_writes, must_writes, may_writes,
> +                             isl_union_map_copy (original),
> +                             must_waw, may_waw, must_waw_no_source,
> +                             may_waw_no_source);
>
>    subtract_commutative_associative_deps
>      (scop, pbbs, original,
> diff --git a/gcc/graphite-optimize-isl.c b/gcc/graphite-optimize-isl.c
> index ffa4465..811a510 100644
> --- a/gcc/graphite-optimize-isl.c
> +++ b/gcc/graphite-optimize-isl.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include <isl/band.h>
>  #include <isl/aff.h>
>  #include <isl/options.h>
> +#include <isl/ctx.h>
>
>  #include "system.h"
>  #include "coretypes.h"
> @@ -422,26 +423,25 @@ static const int CONSTANT_BOUND = 20;
>  bool
>  optimize_isl (scop_p scop)
>  {
> -
> -  isl_schedule *schedule;
> -#ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
> -  isl_schedule_constraints *schedule_constraints;
> +#ifdef HAVE_ISL_CTX_MAX_OPERATIONS
> +  int old_max_operations = isl_ctx_get_max_operations(scop->ctx);
> +  int max_operations = PARAM_VALUE (PARAM_MAX_ISL_OPERATIONS);
> +  if (max_operations)
> +    isl_ctx_set_max_operations(scop->ctx, max_operations);
>  #endif
> -  isl_union_set *domain;
> -  isl_union_map *validity, *proximity, *dependences;
> -  isl_union_map *schedule_map;
> +  isl_options_set_on_error (scop->ctx, ISL_ON_ERROR_CONTINUE);
>
> -  domain = scop_get_domains (scop);
> -  dependences = scop_get_dependences (scop);
> +  isl_union_set *domain = scop_get_domains (scop);
> +  isl_union_map *dependences = scop_get_dependences (scop);
>    dependences = isl_union_map_gist_domain (dependences,
>                                            isl_union_set_copy (domain));
>    dependences = isl_union_map_gist_range (dependences,
>                                           isl_union_set_copy (domain));
> -  validity = dependences;
> -
> -  proximity = isl_union_map_copy (validity);
> +  isl_union_map *validity = dependences;
> +  isl_union_map *proximity = isl_union_map_copy (validity);
>
>  #ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
> +  isl_schedule_constraints *schedule_constraints;
>    schedule_constraints = isl_schedule_constraints_on_domain (domain);
>    schedule_constraints
>         = isl_schedule_constraints_set_proximity (schedule_constraints,
> @@ -461,26 +461,39 @@ optimize_isl (scop_p scop)
>  #else
>    isl_options_set_schedule_fuse (scop->ctx, ISL_SCHEDULE_FUSE_MIN);
>  #endif
> -  isl_options_set_on_error (scop->ctx, ISL_ON_ERROR_CONTINUE);
>
>  #ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
> -  schedule = isl_schedule_constraints_compute_schedule(schedule_constraints);
> +  isl_schedule *schedule
> +    = isl_schedule_constraints_compute_schedule (schedule_constraints);
>  #else
> -  schedule = isl_union_set_compute_schedule (domain, validity, proximity);
> +  isl_schedule *schedule
> +    = isl_union_set_compute_schedule (domain, validity, proximity);
>  #endif
>
>    isl_options_set_on_error (scop->ctx, ISL_ON_ERROR_ABORT);
>
> +#ifdef HAVE_ISL_CTX_MAX_OPERATIONS
> +  isl_ctx_reset_operations(scop->ctx);
> +  isl_ctx_set_max_operations(scop->ctx, old_max_operations);
> +  if (!schedule || isl_ctx_last_error (scop->ctx) == isl_error_quota)
> +    {
> +      if (dump_file && dump_flags)
> +       fprintf (dump_file, "ISL timed out at %d operations\n",
> +                max_operations);
> +      if (schedule)
> +       isl_schedule_free (schedule);
> +      return false;
> +    }
> +#else
>    if (!schedule)
>      return false;
> +#endif
>
> -  schedule_map = getScheduleMap (schedule);
> -
> +  isl_union_map *schedule_map = getScheduleMap (schedule);
>    apply_schedule_map_to_scop (scop, schedule_map);
>
>    isl_schedule_free (schedule);
>    isl_union_map_free (schedule_map);
> -
>    return true;
>  }
>
> diff --git a/gcc/params.def b/gcc/params.def
> index c8b3a90..6f572fa3 100644
> --- a/gcc/params.def
> +++ b/gcc/params.def
> @@ -844,6 +844,11 @@ DEFPARAM (PARAM_GRAPHITE_MAX_BBS_PER_FUNCTION,
>           "maximum number of basic blocks per function to be analyzed by Graphite",
>           100, 0, 0)
>
> +DEFPARAM (PARAM_MAX_ISL_OPERATIONS,
> +         "max-isl-operations",
> +         "maximum number of ISL operations, 0 means unlimited",
> +         350000, 0, 0)
> +
>  /* Avoid data dependence analysis on very large loops.  */
>  DEFPARAM (PARAM_LOOP_MAX_DATAREFS_FOR_DATADEPS,
>           "loop-max-datarefs-for-datadeps",
> diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c
> index d9c07e2..4e3c705 100644
> --- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c
> +++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c
> @@ -54,4 +54,4 @@ main (void)
>    return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "tiled by" 4 "graphite" } } */
> +/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
> diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c
> index 7ef575b..a9d4950 100644
> --- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c
> +++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c
> @@ -55,4 +55,4 @@ main (void)
>    return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "tiled by" 6 "graphite" } } */
> +/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
> diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c
> index 0e32fd6..fe2669f 100644
> --- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c
> +++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c
> @@ -49,4 +49,4 @@ main (void)
>    return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "tiled by" 2 "graphite" } } */
> +/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
> diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c
> index eebece3..211c9ab 100644
> --- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c
> +++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c
> @@ -59,4 +59,4 @@ main (void)
>    return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "tiled by" 3 "graphite" } } */
> +/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
> --
> 2.1.0.243.g30d45f7
>
Sebastian Pop Sept. 3, 2015, 3:22 p.m. UTC | #4
Richard Biener wrote:
> >             * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to pass with
> >             both isl-0.12 and isl-0.15.
> 
> Does it mean with 0.15 we now "time out" on some of the cases?  

"time out" will not trigger on the testcases modified in this patch.

> Or is this
> just a general difference between 0.12 and 0.15?  In which case, like for
> this testcase, is there a better way to verify whether the loops J and K were
> interchanged?

We have more "tiled by" with isl-0.15 than with isl-0.12, so that means that the
pattern we are looking for is not stable enough between isl versions: I will
have to find and test for another pattern to check that loops have been blocked,
interchanged, etc., which in my opinion is hard as we currently use different
schedulers for different versions of isl.

I have tuned the time out such that it will not trigger on the interchange
testcases.  It will trigger on a fortran testcase pr42334-1.f on which I have
seen warnings of dejagnu timing out, and I have also tried on the reduced
testcase attached to PR53852 which will time out with isl-0.15.  I have not
added PR53852's testcase as there still are people using isl-0.12 that would get
another testcase that uses large amounts of memory and compile time.

Sebastian
Richard Biener Sept. 15, 2015, 1:54 p.m. UTC | #5
On Thu, Sep 3, 2015 at 5:22 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> Richard Biener wrote:
>> >             * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to pass with
>> >             both isl-0.12 and isl-0.15.
>>
>> Does it mean with 0.15 we now "time out" on some of the cases?
>
> "time out" will not trigger on the testcases modified in this patch.
>
>> Or is this
>> just a general difference between 0.12 and 0.15?  In which case, like for
>> this testcase, is there a better way to verify whether the loops J and K were
>> interchanged?
>
> We have more "tiled by" with isl-0.15 than with isl-0.12, so that means that the
> pattern we are looking for is not stable enough between isl versions: I will
> have to find and test for another pattern to check that loops have been blocked,
> interchanged, etc., which in my opinion is hard as we currently use different
> schedulers for different versions of isl.
>
> I have tuned the time out such that it will not trigger on the interchange
> testcases.  It will trigger on a fortran testcase pr42334-1.f on which I have
> seen warnings of dejagnu timing out, and I have also tried on the reduced
> testcase attached to PR53852 which will time out with isl-0.15.  I have not
> added PR53852's testcase as there still are people using isl-0.12 that would get
> another testcase that uses large amounts of memory and compile time.

Btw, as all versions from GCC 4.8 on support ISL 0.14 it is reasonable to remove
support for older ISL versions from trunk (so require 0.14 at least
which has the
timeout mechanism IIRC)

Thanks,
Richard.

> Sebastian
Sebastian Pop Sept. 15, 2015, 3:19 p.m. UTC | #6
On Tue, Sep 15, 2015 at 8:54 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Sep 3, 2015 at 5:22 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>> Richard Biener wrote:
>>> >             * gcc.dg/graphite/uns-interchange-12.c: Adjust pattern to pass with
>>> >             both isl-0.12 and isl-0.15.
>>>
>>> Does it mean with 0.15 we now "time out" on some of the cases?
>>
>> "time out" will not trigger on the testcases modified in this patch.
>>
>>> Or is this
>>> just a general difference between 0.12 and 0.15?  In which case, like for
>>> this testcase, is there a better way to verify whether the loops J and K were
>>> interchanged?
>>
>> We have more "tiled by" with isl-0.15 than with isl-0.12, so that means that the
>> pattern we are looking for is not stable enough between isl versions: I will
>> have to find and test for another pattern to check that loops have been blocked,
>> interchanged, etc., which in my opinion is hard as we currently use different
>> schedulers for different versions of isl.
>>
>> I have tuned the time out such that it will not trigger on the interchange
>> testcases.  It will trigger on a fortran testcase pr42334-1.f on which I have
>> seen warnings of dejagnu timing out, and I have also tried on the reduced
>> testcase attached to PR53852 which will time out with isl-0.15.  I have not
>> added PR53852's testcase as there still are people using isl-0.12 that would get
>> another testcase that uses large amounts of memory and compile time.
>
> Btw, as all versions from GCC 4.8 on support ISL 0.14 it is reasonable to remove
> support for older ISL versions from trunk (so require 0.14 at least
> which has the
> timeout mechanism IIRC)

I will prepare a patch for trunk.
Thanks for the idea.

Sebastian
diff mbox

Patch

diff --git a/gcc/config.in b/gcc/config.in
index 22a4e6b..98c4647 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1332,6 +1332,12 @@ 
 #endif
 
 
+/* Define if isl_ctx_get_max_operations exists. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_ISL_CTX_MAX_OPERATIONS
+#endif
+
+
 /* Define if isl_options_set_schedule_serialize_sccs exists. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS
diff --git a/gcc/configure b/gcc/configure
index 0d31383..07d39f9 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -28625,6 +28625,29 @@  rm -f core conftest.err conftest.$ac_objext \
   { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_has_isl_options_set_schedule_serialize_sccs" >&5
 $as_echo "$ac_has_isl_options_set_schedule_serialize_sccs" >&6; }
 
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking Checking for isl_ctx_get_max_operations" >&5
+$as_echo_n "checking Checking for isl_ctx_get_max_operations... " >&6; }
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <isl/ctx.h>
+int
+main ()
+{
+isl_ctx_get_max_operations (isl_ctx_alloc ());
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  ac_has_isl_ctx_get_max_operations=yes
+else
+  ac_has_isl_ctx_get_max_operations=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_has_isl_ctx_get_max_operations" >&5
+$as_echo "$ac_has_isl_ctx_get_max_operations" >&6; }
+
   LIBS="$saved_LIBS"
   CXXFLAGS="$saved_CXXFLAGS"
 
@@ -28639,6 +28662,11 @@  $as_echo "#define HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE 1" >>confdefs.h
 $as_echo "#define HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS 1" >>confdefs.h
 
   fi
+  if test x"$ac_has_isl_ctx_get_max_operations" = x"yes"; then
+
+$as_echo "#define HAVE_ISL_CTX_MAX_OPERATIONS 1" >>confdefs.h
+
+  fi
 fi
 
 # Check for plugin support
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 846651d..b6e8bed 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5790,6 +5790,13 @@  if test "x${ISLLIBS}" != "x" ; then
               [ac_has_isl_options_set_schedule_serialize_sccs=no])
   AC_MSG_RESULT($ac_has_isl_options_set_schedule_serialize_sccs)
 
+  AC_MSG_CHECKING([Checking for isl_ctx_get_max_operations])
+  AC_TRY_LINK([#include <isl/ctx.h>],
+              [isl_ctx_get_max_operations (isl_ctx_alloc ());],
+              [ac_has_isl_ctx_get_max_operations=yes],
+              [ac_has_isl_ctx_get_max_operations=no])
+  AC_MSG_RESULT($ac_has_isl_ctx_get_max_operations)
+
   LIBS="$saved_LIBS"
   CXXFLAGS="$saved_CXXFLAGS"
 
@@ -5802,6 +5809,10 @@  if test "x${ISLLIBS}" != "x" ; then
      AC_DEFINE(HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS, 1,
                [Define if isl_options_set_schedule_serialize_sccs exists.])
   fi
+  if test x"$ac_has_isl_ctx_get_max_operations" = x"yes"; then
+     AC_DEFINE(HAVE_ISL_CTX_MAX_OPERATIONS, 1,
+               [Define if isl_ctx_get_max_operations exists.])
+  fi
 fi
 
 GCC_ENABLE_PLUGINS
diff --git a/gcc/graphite-dependences.c b/gcc/graphite-dependences.c
index c3c2090..85f16f3 100644
--- a/gcc/graphite-dependences.c
+++ b/gcc/graphite-dependences.c
@@ -256,17 +256,12 @@  __isl_give isl_union_map *
 extend_schedule (__isl_take isl_union_map *x)
 {
   int max = 0;
-  isl_stat res;
   struct extend_schedule_str str;
 
-  res = isl_union_map_foreach_map (x, max_number_of_out_dimensions, (void *) &max);
-  gcc_assert (res == isl_stat_ok);
-
+  isl_union_map_foreach_map (x, max_number_of_out_dimensions, (void *) &max);
   str.max = max;
   str.umap = isl_union_map_empty (isl_union_map_get_space (x));
-  res = isl_union_map_foreach_map (x, extend_schedule_1, (void *) &str);
-  gcc_assert (res == isl_stat_ok);
-
+  isl_union_map_foreach_map (x, extend_schedule_1, (void *) &str);
   isl_union_map_free (x);
   return str.umap;
 }
@@ -395,7 +390,6 @@  subtract_commutative_associative_deps (scop_p scop,
   FOR_EACH_VEC_ELT (pbbs, i, pbb)
     if (PBB_IS_REDUCTION (pbb))
       {
-	int res;
 	isl_union_map *r = isl_union_map_empty (isl_space_copy (space));
 	isl_union_map *must_w = isl_union_map_empty (isl_space_copy (space));
 	isl_union_map *may_w = isl_union_map_empty (isl_space_copy (space));
@@ -432,27 +426,24 @@  subtract_commutative_associative_deps (scop_p scop,
 	  (isl_union_map_copy (must_w), isl_union_map_copy (may_w));
 	empty = isl_union_map_empty (isl_union_map_get_space (all_w));
 
-	res = isl_union_map_compute_flow (isl_union_map_copy (r),
-					  isl_union_map_copy (must_w),
-					  isl_union_map_copy (may_w),
-					  isl_union_map_copy (original),
-					  &x_must_raw, &x_may_raw,
-					  &x_must_raw_no_source,
-					  &x_may_raw_no_source);
-	gcc_assert (res == 0);
-	res = isl_union_map_compute_flow (isl_union_map_copy (all_w),
-					  r, empty,
-					  isl_union_map_copy (original),
-					  &x_must_war, &x_may_war,
-					  &x_must_war_no_source,
-					  &x_may_war_no_source);
-	gcc_assert (res == 0);
-	res = isl_union_map_compute_flow (all_w, must_w, may_w,
-					  isl_union_map_copy (original),
-					  &x_must_waw, &x_may_waw,
-					  &x_must_waw_no_source,
-					  &x_may_waw_no_source);
-	gcc_assert (res == 0);
+	isl_union_map_compute_flow (isl_union_map_copy (r),
+				    isl_union_map_copy (must_w),
+				    isl_union_map_copy (may_w),
+				    isl_union_map_copy (original),
+				    &x_must_raw, &x_may_raw,
+				    &x_must_raw_no_source,
+				    &x_may_raw_no_source);
+	isl_union_map_compute_flow (isl_union_map_copy (all_w),
+				    r, empty,
+				    isl_union_map_copy (original),
+				    &x_must_war, &x_may_war,
+				    &x_must_war_no_source,
+				    &x_may_war_no_source);
+	isl_union_map_compute_flow (all_w, must_w, may_w,
+				    isl_union_map_copy (original),
+				    &x_must_waw, &x_may_waw,
+				    &x_must_waw_no_source,
+				    &x_may_waw_no_source);
 
 	if (must_raw)
 	  *must_raw = isl_union_map_subtract (*must_raw, x_must_raw);
@@ -551,26 +542,22 @@  compute_deps (scop_p scop, vec<poly_bb_p> pbbs,
   isl_space *space = isl_union_map_get_space (all_writes);
   isl_union_map *empty = isl_union_map_empty (space);
   isl_union_map *original = scop_get_original_schedule (scop, pbbs);
-  int res;
 
-  res = isl_union_map_compute_flow (isl_union_map_copy (reads),
-				    isl_union_map_copy (must_writes),
-				    isl_union_map_copy (may_writes),
-				    isl_union_map_copy (original),
-				    must_raw, may_raw, must_raw_no_source,
-				    may_raw_no_source);
-  gcc_assert (res == 0);
-  res = isl_union_map_compute_flow (isl_union_map_copy (all_writes),
-				    reads, empty,
-				    isl_union_map_copy (original),
-				    must_war, may_war, must_war_no_source,
-				    may_war_no_source);
-  gcc_assert (res == 0);
-  res = isl_union_map_compute_flow (all_writes, must_writes, may_writes,
-				    isl_union_map_copy (original),
-				    must_waw, may_waw, must_waw_no_source,
-				    may_waw_no_source);
-  gcc_assert (res == 0);
+  isl_union_map_compute_flow (isl_union_map_copy (reads),
+			      isl_union_map_copy (must_writes),
+			      isl_union_map_copy (may_writes),
+			      isl_union_map_copy (original),
+			      must_raw, may_raw, must_raw_no_source,
+			      may_raw_no_source);
+  isl_union_map_compute_flow (isl_union_map_copy (all_writes),
+			      reads, empty,
+			      isl_union_map_copy (original),
+			      must_war, may_war, must_war_no_source,
+			      may_war_no_source);
+  isl_union_map_compute_flow (all_writes, must_writes, may_writes,
+			      isl_union_map_copy (original),
+			      must_waw, may_waw, must_waw_no_source,
+			      may_waw_no_source);
 
   subtract_commutative_associative_deps
     (scop, pbbs, original,
diff --git a/gcc/graphite-optimize-isl.c b/gcc/graphite-optimize-isl.c
index ffa4465..811a510 100644
--- a/gcc/graphite-optimize-isl.c
+++ b/gcc/graphite-optimize-isl.c
@@ -33,6 +33,7 @@  along with GCC; see the file COPYING3.  If not see
 #include <isl/band.h>
 #include <isl/aff.h>
 #include <isl/options.h>
+#include <isl/ctx.h>
 
 #include "system.h"
 #include "coretypes.h"
@@ -422,26 +423,25 @@  static const int CONSTANT_BOUND = 20;
 bool
 optimize_isl (scop_p scop)
 {
-
-  isl_schedule *schedule;
-#ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
-  isl_schedule_constraints *schedule_constraints;
+#ifdef HAVE_ISL_CTX_MAX_OPERATIONS
+  int old_max_operations = isl_ctx_get_max_operations(scop->ctx);
+  int max_operations = PARAM_VALUE (PARAM_MAX_ISL_OPERATIONS);
+  if (max_operations)
+    isl_ctx_set_max_operations(scop->ctx, max_operations);
 #endif
-  isl_union_set *domain;
-  isl_union_map *validity, *proximity, *dependences;
-  isl_union_map *schedule_map;
+  isl_options_set_on_error (scop->ctx, ISL_ON_ERROR_CONTINUE);
 
-  domain = scop_get_domains (scop);
-  dependences = scop_get_dependences (scop);
+  isl_union_set *domain = scop_get_domains (scop);
+  isl_union_map *dependences = scop_get_dependences (scop);
   dependences = isl_union_map_gist_domain (dependences,
 					   isl_union_set_copy (domain));
   dependences = isl_union_map_gist_range (dependences,
 					  isl_union_set_copy (domain));
-  validity = dependences;
-
-  proximity = isl_union_map_copy (validity);
+  isl_union_map *validity = dependences;
+  isl_union_map *proximity = isl_union_map_copy (validity);
 
 #ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
+  isl_schedule_constraints *schedule_constraints;
   schedule_constraints = isl_schedule_constraints_on_domain (domain);
   schedule_constraints
 	= isl_schedule_constraints_set_proximity (schedule_constraints,
@@ -461,26 +461,39 @@  optimize_isl (scop_p scop)
 #else
   isl_options_set_schedule_fuse (scop->ctx, ISL_SCHEDULE_FUSE_MIN);
 #endif
-  isl_options_set_on_error (scop->ctx, ISL_ON_ERROR_CONTINUE);
 
 #ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
-  schedule = isl_schedule_constraints_compute_schedule(schedule_constraints);
+  isl_schedule *schedule
+    = isl_schedule_constraints_compute_schedule (schedule_constraints);
 #else
-  schedule = isl_union_set_compute_schedule (domain, validity, proximity);
+  isl_schedule *schedule
+    = isl_union_set_compute_schedule (domain, validity, proximity);
 #endif
 
   isl_options_set_on_error (scop->ctx, ISL_ON_ERROR_ABORT);
 
+#ifdef HAVE_ISL_CTX_MAX_OPERATIONS
+  isl_ctx_reset_operations(scop->ctx);
+  isl_ctx_set_max_operations(scop->ctx, old_max_operations);
+  if (!schedule || isl_ctx_last_error (scop->ctx) == isl_error_quota)
+    {
+      if (dump_file && dump_flags)
+	fprintf (dump_file, "ISL timed out at %d operations\n",
+		 max_operations);
+      if (schedule)
+	isl_schedule_free (schedule);
+      return false;
+    }
+#else
   if (!schedule)
     return false;
+#endif
 
-  schedule_map = getScheduleMap (schedule);
-
+  isl_union_map *schedule_map = getScheduleMap (schedule);
   apply_schedule_map_to_scop (scop, schedule_map);
 
   isl_schedule_free (schedule);
   isl_union_map_free (schedule_map);
-
   return true;
 }
 
diff --git a/gcc/params.def b/gcc/params.def
index c8b3a90..6f572fa3 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -844,6 +844,11 @@  DEFPARAM (PARAM_GRAPHITE_MAX_BBS_PER_FUNCTION,
 	  "maximum number of basic blocks per function to be analyzed by Graphite",
 	  100, 0, 0)
 
+DEFPARAM (PARAM_MAX_ISL_OPERATIONS,
+	  "max-isl-operations",
+	  "maximum number of ISL operations, 0 means unlimited",
+	  350000, 0, 0)
+
 /* Avoid data dependence analysis on very large loops.  */
 DEFPARAM (PARAM_LOOP_MAX_DATAREFS_FOR_DATADEPS,
 	  "loop-max-datarefs-for-datadeps",
diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c
index d9c07e2..4e3c705 100644
--- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c
+++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-12.c
@@ -54,4 +54,4 @@  main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "tiled by" 4 "graphite" } } */
+/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c
index 7ef575b..a9d4950 100644
--- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c
+++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-14.c
@@ -55,4 +55,4 @@  main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "tiled by" 6 "graphite" } } */
+/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c
index 0e32fd6..fe2669f 100644
--- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c
+++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-15.c
@@ -49,4 +49,4 @@  main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "tiled by" 2 "graphite" } } */
+/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */
diff --git a/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c b/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c
index eebece3..211c9ab 100644
--- a/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c
+++ b/gcc/testsuite/gcc.dg/graphite/uns-interchange-mvt.c
@@ -59,4 +59,4 @@  main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "tiled by" 3 "graphite" } } */
+/* { dg-final { scan-tree-dump "tiled by" "graphite" } } */