This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> -----Original Message-----
> From: Steven Bosscher [mailto:stevenb.gcc@gmail.com]
> Sent: Friday, August 24, 2012 8:17 PM
> To: Zhenqiang Chen
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: Ping: [PATCH] Enable bbro for -Os
> 
> 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.

Thank you for the comments.

cleanup_cfg is called at the end cfg_layout_initialize before
reorder_basic_blocks. cleanup_cfg does lots of optimization on cfg and
renumber the basic blocks. After cleanup_cfg, the blocks are roughly
numbered sequentially.

The heuristic bases on the result of cleanup_cfg. It just wants to keep the
order of cleanup_cfg since logs show we will have code size improvement (by
cleanup_cfg) even if we do not call reorder_basic_blocks. "index as a key"
is a simple way keep the original order.

Comments are added in the updated patch.

> >> * 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.

Try the case in PR54364, the patch does reduce several jmp.

> > @@ -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?

Thanks. It is not necessary.

Here is the updated ChangeLog. The updated patch is attached.

ChangeLog
2012-08-29  Zhenqiang Chen <zhenqiang.chen@arm.com>

	PR middle-end/54364
	* 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.
	(gate_handle_reorder_blocks): Enable bbro when optimizing for -Os.

Attachment: Enable-bbro-for-size-updated.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]