> On Thu, May 22, 2014 at 11:36 PM, Dehao Chen <de...@google.com> wrote:
> > If a loop's header count is less than iteration count, the iteration
> > estimation is apparently incorrect for this loop. Thus disable
> > unrolling of such loops.
> >
> > Testing on going. OK for trunk if test pass?
>
> No. Why don't you instead plug the hole in expected_loop_iterations ()?
> That is, why may not loop->header be bogus? Isn't it maybe the bounding
It is autoFDO thing. Dehao is basically pushing out changes that should make
compiler
more sane about bogus profiles.
At the moment we have tests bb->count != 0 to figure out if the profile is
reliable. I.e. we assume that profile feedback is always good, guessed profile
is never good. Loop optimizers then do not trust guessed profile to give
realistic estimates on number of iterations and bail out into simple heuristics
for estimated profiles that are usually not good on this job - i.e.
int niters = 0;
if (desc->const_iter)
niters = desc->niter;
else if (loop->header->count)
niters = expected_loop_iterations (loop);
...
With FDO this change, as the FDO profiles are sometimes bogus (and there is not
much
to do about it).
I would say that for loop optimization, we want a flag whether expected number
of iterations is reliable. We probably want
if (expected_loop_iterations_reliable_p (loop))
niters = expected_loop_iterations (loop);
We probably also want to store this information into loop structure rather than
computing
it all the time from profile, since the profile may get inaccurate and we
already know
how to maintain upper bounds on numbers of iterations, so it should be easy to
implement.
This would allow us to preserve info like
for (i=0 ;i < __bulitin_expect (n,10); i++)
that would be nice feature to have.
Honza
> you run into?
>
> /* Returns expected number of LOOP iterations. The returned value is bounded
> by REG_BR_PROB_BASE. */
>
> unsigned
> expected_loop_iterations (const struct loop *loop)
> {
> gcov_type expected = expected_loop_iterations_unbounded (loop);
> return (expected > REG_BR_PROB_BASE ? REG_BR_PROB_BASE : expected);
> }
>
> I miss a testcase as well.
>
> Richard.
>
> > Thanks,
> > Dehao
> >
> > gcc/ChangeLog:
> > 2014-05-21 Dehao Chen <de...@google.com>
> >
> > * cfgloop.h (expected_loop_iterations_reliable_p): New func.
> > * cfgloopanal.c (expected_loop_iterations_reliable_p): Likewise.
> > * loop-unroll.c (decide_unroll_runtime_iterations): Disable unroll
> > loop that has unreliable iteration counts.
> >
> > Index: gcc/cfgloop.h
> > ===================================================================
> > --- gcc/cfgloop.h (revision 210717)
> > +++ gcc/cfgloop.h (working copy)
> > @@ -307,8 +307,8 @@ extern bool just_once_each_iteration_p (const stru
> > gcov_type expected_loop_iterations_unbounded (const struct loop *);
> > extern unsigned expected_loop_iterations (const struct loop *);
> > extern rtx doloop_condition_get (rtx);
> > +extern bool expected_loop_iterations_reliable_p (const struct loop *);
> >
> > -
> > /* Loop manipulation. */
> > extern bool can_duplicate_loop_p (const struct loop *loop);
> >
> > Index: gcc/cfgloopanal.c
> > ===================================================================
> > --- gcc/cfgloopanal.c (revision 210717)
> > +++ gcc/cfgloopanal.c (working copy)
> > @@ -285,6 +285,15 @@ expected_loop_iterations (const struct loop *loop)
> > return (expected > REG_BR_PROB_BASE ? REG_BR_PROB_BASE : expected);
> > }
> >
> > +/* Returns true if the loop header's profile count is smaller than expected
> > + loop iteration. */
> > +
> > +bool
> > +expected_loop_iterations_reliable_p (const struct loop *loop)
> > +{
> > + return expected_loop_iterations (loop) < loop->header->count;
> > +}
> > +
> > /* Returns the maximum level of nesting of subloops of LOOP. */
> >
> > unsigned
> > Index: gcc/loop-unroll.c
> > ===================================================================
> > --- gcc/loop-unroll.c (revision 210717)
> > +++ gcc/loop-unroll.c (working copy)
> > @@ -988,6 +988,15 @@ decide_unroll_runtime_iterations (struct loop *loo
> > return;
> > }
> >
> > + if (profile_status_for_fn (cfun) == PROFILE_READ
> > + && expected_loop_iterations_reliable_p (loop))
> > + {
> > + if (dump_file)
> > + fprintf (dump_file, ";; Not unrolling loop, loop iteration "
> > + "not reliable.");
> > + return;
> > + }
> > +
> > /* Check whether the loop rolls. */
> > if ((get_estimated_loop_iterations (loop, &iterations)
> > || get_max_loop_iterations (loop, &iterations))
