On Mon, Nov 28, 2016 at 10:23 PM, Jeff Law <l...@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) >
