Re: [RFA] Handle target with no length attributes sanely in bb-reorder.c

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

On Mon, Nov 28, 2016 at 10:23 PM, Jeff Law <law@redhat.com> wrote:
>
>
> I was digging into  issues around the patches for 78120 when I stumbled upon
> undesirable bb copying in bb-reorder.c on the m68k.
>
> The core issue is that the m68k does not define a length attribute and
> therefore generic code assumes that the length of all insns is 0 bytes.

What other targets behave like this?

> That in turn makes bb-reorder think it is infinitely cheap to copy basic
> blocks.  In the two codebases I looked at (GCC's runtime libraries and
> newlib) this leads to a 10% and 15% undesirable increase in code size.
>
> I've taken a slight variant of this patch and bootstrapped/regression tested
> it on x86_64-linux-gnu to verify sanity as well as built the m68k target
> libraries noted above.
>
> OK for the trunk?

I wonder if it isn't better to default to a length of 1 instead of zero when
there is no length attribute.  There are more users of the length attribute
in bb-reorder.c (and elsewhere as well I suppose).

from get_attr_length_1 it looks like a "cheap" target dependent way
would be to define ADJUST_INSN_LENGTH ...

Richard.

> Jeff
>
>         * bb-reorder.c (copy_bb_p): Sanely handle case where the target
>         has not defined length attributes for its insns.
>
> diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
> index 6873b4f..0b8d1d9 100644
> --- a/gcc/bb-reorder.c
> +++ b/gcc/bb-reorder.c
> @@ -115,6 +115,7 @@
>  #include "bb-reorder.h"
>  #include "except.h"
>  #include "fibonacci_heap.h"
> +#include "insn-attr.h"
>
>  /* The number of rounds.  In most cases there will only be 4 rounds, but
>     when partitioning hot and cold basic blocks into separate sections of
> @@ -1355,6 +1356,9 @@ copy_bb_p (const_basic_block bb, int code_may_grow)
>    int max_size = uncond_jump_length;
>    rtx_insn *insn;
>
> +  if (!HAVE_ATTR_length)
> +    return false;
> +
>    if (!bb->frequency)
>      return false;
>    if (EDGE_COUNT (bb->preds) < 2)
>