Re: Ping: [PATCH] Enable bbro for -Os

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Wed, Aug 22, 2012 at 8:49 AM, Zhenqiang Chen <zhenqiang.chen@arm.com> wrote:
>> The patch is to enable bbro for -Os. When optimizing for size, it
>> * avoid duplicating block.
>> * keep its original order if there is no chance to fall through.
>> * ignore edge frequency and probability.
>> * handle predecessor first if its index is smaller to break long trace.

You do this by inserting the index as a key. I don't fully understand
this change. You're assuming that a block with a lower index has a
lower pre-order number in the CFG's DFS spanning tree, IIUC (i.e. the
blocks are numbered sequentially)? I'm not sure that's always true. I
think you should add an explanation for this heuristic.

>> * only connect Trace n with Trace n + 1 to reduce long jump.
...
>>       * bb-reorder.c (connect_better_edge_p): New added.
>>       (find_traces_1_round): When optimizing for size, ignore edge
>> frequency
>>       and probability, and handle all in one round.
>>       (bb_to_key): Use bb->index as key for size.
>>       (better_edge_p): The smaller bb index is better for size.
>>       (connect_traces): Connect block n with block n + 1;
>>       connect trace m with trace m + 1 if falling through.
>>       (copy_bb_p): Avoid duplicating blocks.
>>       (gate_handle_reorder_blocks): Enable bbro when optimizing for -Os.

This probably fixes PR54364.


> @@ -1169,6 +1272,10 @@ copy_bb_p (const_basic_block bb, int code_may_grow)
>    int max_size = uncond_jump_length;
>    rtx insn;
>
> +  /* Avoid duplicating blocks for size.  */
> +  if (optimize_function_for_size_p (cfun))
> +    return false;
> +
>    if (!bb->frequency)
>      return false;

This shouldn't be necessary, due to the CODE_MAY_GROW argument, and
this change should result in a code size increase because jumps to
conditional jumps aren't removed anymore. What did you make this
change for, do you have a test case where code size increases if you
allow copy_bb_p to return true?

Ciao!
Steven