Re: [PATCH] Change num_loop_insns to better match what it has been doing before VTA merge

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Sep 30, 2009 at 11:53 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> While looking at 252.eon eon.cc differences on ia64 caused by merge from VTA
> branch, I've noticed that some loops are no longer unrolled. ?The problem is
> that num_loop_insns now returns higher number of insns than before and so in
> some cases it goes over the limit.
> The VTA merge changed:
> @@ -176,8 +176,8 @@ num_loop_insns (const struct loop *loop)
> ? ? {
> ? ? ? bb = bbs[i];
> ? ? ? ninsns++;
> - ? ? ?for (insn = BB_HEAD (bb); insn != BB_END (bb); insn = NEXT_INSN (insn))
> - ? ? ? if (INSN_P (insn))
> + ? ? ?FOR_BB_INSNS (bb, insn)
> + ? ? ? if (NONDEBUG_INSN_P (insn))
> ? ? ? ? ?ninsns++;
> ? ? }
> ? free(bbs);
> The ninsns++ before the loop standed probably for the BB_END (bb) insn that
> wasn't counted in the loop. ?But FOR_BB_INSNS counts it, so after VTA merge
> we count number of instructions in the loop plus number of basic blocks.
> The following patch changes it to what average_num_loop_insns does, i.e.
> don't add one for each bb, just count real insns, and at the end just avoid
> returning 0 (because that causes ICs in places where something is divided by
> loop->ninsns).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2009-09-30 ?Jakub Jelinek ?<jakub@redhat.com>
>
> ? ? ? ?PR target/41279
> ? ? ? ?* cfgloopanal.c (num_loop_insns): Don't increment ninsns for each bb
> ? ? ? ?before insn counting loop now that BB_END (bb) is counted. ?Ensure
> ? ? ? ?the return value isn't zero.
>
> --- gcc/cfgloopanal.c.jj ? ? ? ?2009-09-03 09:59:33.000000000 +0200
> +++ gcc/cfgloopanal.c ? 2009-09-30 09:48:35.000000000 +0200
> @@ -175,12 +175,14 @@ num_loop_insns (const struct loop *loop)
> ? for (i = 0; i < loop->num_nodes; i++)
> ? ? {
> ? ? ? bb = bbs[i];
> - ? ? ?ninsns++;
> ? ? ? FOR_BB_INSNS (bb, insn)
> ? ? ? ?if (NONDEBUG_INSN_P (insn))
> ? ? ? ? ?ninsns++;
> ? ? }
> - ?free(bbs);
> + ?free (bbs);
> +
> + ?if (!ninsns)
> + ? ?ninsns = 1; ? ? ? ?/* To avoid division by zero. ?*/
>
> ? return ninsns;
> ?}
> @@ -209,7 +211,7 @@ average_num_loop_insns (const struct loo
> ? ? ? ? ? ? ?: (bb->frequency * BB_FREQ_MAX) / loop->header->frequency;
> ? ? ? ninsns += binsns * ratio;
> ? ? }
> - ?free(bbs);
> + ?free (bbs);
>
> ? ninsns /= BB_FREQ_MAX;
> ? if (!ninsns)
>
> ? ? ? ?Jakub
>