Re: [PATCH] Fix PR81090, properly free niter estimates

[Christophe Lyon <christophe dot lyon at linaro dot org>]

Hi Richard,

On 16 June 2017 at 14:18, Richard Biener <rguenther@suse.de> wrote:
> On Wed, 14 Jun 2017, Richard Biener wrote:
>
>>
>> niter estimates are not kept up-to-date (they reference gimple stmts
>> and trees) in the keep-loop-stuff infrastructure so similar to the
>> SCEV cache we rely on people freeing it after passes.
>>
>> The following brings us a step closer to that by freeing them whenever
>> SCEV is invalidated (we only compute them when SCEV is active) plus
>> removing the odd record-bounds pass that just computes them, leaving
>> scavenging to following passes.
>>
>> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> Some awkward interactions with peeling means I'm installing the
> following less aggressive variant.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>
> Richard.
>
> 2017-06-16  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/81090
>         * passes.def (pass_record_bounds): Remove.
>         * tree-pass.h (make_pass_record_bounds): Likewise.
>         * tree-ssa-loop.c (pass_data_record_bounds, pass_record_bounds,
>         make_pass_record_bounds): Likewise.
>         * tree-ssa-loop-ivcanon.c (canonicalize_induction_variables): Do
>         not free niter estimates at the beginning but at the end.
>         * tree-scalar-evolution.c (scev_finalize): Free niter estimates.
>
>         * gcc.dg/graphite/pr81090.c: New testcase.
>

Sorry to bother you again...
With this commit (r249249), I've noticed regressions on aarch64/arm:
FAIL:    gcc.dg/vect/pr65947-9.c -flto -ffat-lto-objects
scan-tree-dump-not vect "LOOP VECTORIZED"
FAIL:    gcc.dg/vect/pr65947-9.c scan-tree-dump-not vect "LOOP VECTORIZED"

Christophe

> Index: gcc/passes.def
> ===================================================================
> --- gcc/passes.def      (revision 249246)
> +++ gcc/passes.def      (working copy)
> @@ -276,7 +276,6 @@ along with GCC; see the file COPYING3.
>           /* All unswitching, final value replacement and splitting can expose
>              empty loops.  Remove them now.  */
>           NEXT_PASS (pass_cd_dce);
> -         NEXT_PASS (pass_record_bounds);
>           NEXT_PASS (pass_iv_canon);
>           NEXT_PASS (pass_loop_distribution);
>           NEXT_PASS (pass_copy_prop);
> Index: gcc/testsuite/gcc.dg/graphite/pr81090.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/graphite/pr81090.c     (nonexistent)
> +++ gcc/testsuite/gcc.dg/graphite/pr81090.c     (working copy)
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -floop-nest-optimize" } */
> +
> +int x3, za;
> +int hg[1];
> +
> +void
> +yw (int dq)
> +{
> +  const int r7 = 2;
> +
> +  while (dq < 1)
> +    {
> +      for (x3 = 0; x3 < r7; ++x3)
> +       for (za = 0; za < r7; ++za)
> +         hg[1] = 0;
> +      ++dq;
> +    }
> +
> +  x3 = 0;
> +  while (x3 < r7)
> +    {
> +      ++x3;
> +      if (x3 == 0)
> +       break;
> +    }
> +}
> Index: gcc/tree-pass.h
> ===================================================================
> --- gcc/tree-pass.h     (revision 249246)
> +++ gcc/tree-pass.h     (working copy)
> @@ -373,7 +373,6 @@ extern gimple_opt_pass *make_pass_predco
>  extern gimple_opt_pass *make_pass_iv_canon (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_scev_cprop (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_empty_loop (gcc::context *ctxt);
> -extern gimple_opt_pass *make_pass_record_bounds (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_graphite (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_graphite_transforms (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_if_conversion (gcc::context *ctxt);
> Index: gcc/tree-scalar-evolution.c
> ===================================================================
> --- gcc/tree-scalar-evolution.c (revision 249246)
> +++ gcc/tree-scalar-evolution.c (working copy)
> @@ -3636,6 +3636,7 @@ scev_finalize (void)
>      return;
>    scalar_evolution_info->empty ();
>    scalar_evolution_info = NULL;
> +  free_numbers_of_iterations_estimates (cfun);
>  }
>
>  /* Returns true if the expression EXPR is considered to be too expensive
> Index: gcc/tree-ssa-loop-ivcanon.c
> ===================================================================
> --- gcc/tree-ssa-loop-ivcanon.c (revision 249246)
> +++ gcc/tree-ssa-loop-ivcanon.c (working copy)
> @@ -1212,7 +1212,6 @@ canonicalize_induction_variables (void)
>    bool irred_invalidated = false;
>    bitmap loop_closed_ssa_invalidated = BITMAP_ALLOC (NULL);
>
> -  free_numbers_of_iterations_estimates (cfun);
>    estimate_numbers_of_iterations ();
>
>    FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)
> @@ -1230,6 +1229,7 @@ canonicalize_induction_variables (void)
>
>    /* Clean up the information about numbers of iterations, since brute force
>       evaluation could reveal new information.  */
> +  free_numbers_of_iterations_estimates (cfun);
>    scev_reset ();
>
>    if (!bitmap_empty_p (loop_closed_ssa_invalidated))
> Index: gcc/tree-ssa-loop.c
> ===================================================================
> --- gcc/tree-ssa-loop.c (revision 249246)
> +++ gcc/tree-ssa-loop.c (working copy)
> @@ -459,54 +459,6 @@ make_pass_scev_cprop (gcc::context *ctxt
>    return new pass_scev_cprop (ctxt);
>  }
>
> -/* Record bounds on numbers of iterations of loops.  */
> -
> -namespace {
> -
> -const pass_data pass_data_record_bounds =
> -{
> -  GIMPLE_PASS, /* type */
> -  "*record_bounds", /* name */
> -  OPTGROUP_NONE, /* optinfo_flags */
> -  TV_TREE_LOOP_BOUNDS, /* tv_id */
> -  ( PROP_cfg | PROP_ssa ), /* properties_required */
> -  0, /* properties_provided */
> -  0, /* properties_destroyed */
> -  0, /* todo_flags_start */
> -  0, /* todo_flags_finish */
> -};
> -
> -class pass_record_bounds : public gimple_opt_pass
> -{
> -public:
> -  pass_record_bounds (gcc::context *ctxt)
> -    : gimple_opt_pass (pass_data_record_bounds, ctxt)
> -  {}
> -
> -  /* opt_pass methods: */
> -  virtual unsigned int execute (function *);
> -
> -}; // class pass_record_bounds
> -
> -unsigned int
> -pass_record_bounds::execute (function *fun)
> -{
> -  if (number_of_loops (fun) <= 1)
> -    return 0;
> -
> -  estimate_numbers_of_iterations ();
> -  scev_reset ();
> -  return 0;
> -}
> -
> -} // anon namespace
> -
> -gimple_opt_pass *
> -make_pass_record_bounds (gcc::context *ctxt)
> -{
> -  return new pass_record_bounds (ctxt);
> -}
> -
>  /* Induction variable optimizations.  */
>
>  namespace {