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

[Richard Biener <rguenther at suse dot de>]

On Mon, 19 Jun 2017, Christophe Lyon wrote:

> 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"

So the testcase gets vectorized now (for whatever reason) and still passes
execution.  Not sure why the testcase checked for not being vectorized.

Alan?

Richard.

> 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 {
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)