Re: [PING][PATCH] move superblock formation to Tree-SSA

[Robert Kidd <rkidd at crhc dot uiuc dot edu>]

Please find attached an updated version of the patch.  This corrects  
an inadverent omission of code to reconsider the original block after  
tail duplication.

Thanks
Robert Kidd
rkidd@crhc.uiuc.edu

Changelog:

2007-05-14 Robert Kidd <rkidd@crhc.uiuc.edu>

	* bb-reorder.c (rest_of_handler_reorder_blocks): Removed call to
	RTL level tracer pass.
	* passes.c (init_optimization_passes): Move pass_tracer from
	after pass_rtl_ifcvt to after pass_dce.
	* tracer.c: Update copyright.
	(layout_superblocks): Remove function.
	(mark_bb_seen): New.
	(bb_seen_p): New.
	(count_insns): Change to estimate instructions in a Tree-SSA
	statement.
	(find_trace): Use bb_seen_p.
	(tail_duplicate): Use bb_seen_p.  Call add_phi_args_after_copy
	after duplicate_block.
	(tracer): Change prototype to match that of a pass execute
	callback.
	(gate_tracer): Rename from gate_handle_tracer.
	(rest_of_handle_tracer): Remove function.
	* rtl.h: Remove prototype for tracer.
	* testsuite/gcc.dg/tree-prof/tracer-1.c: New.

Attachment: superblock-1-20070511.patch.txt
Description: Text document

On Apr 10, 2007, at 2:44 PM, Steven Bosscher wrote:

> On Tuesday 10 April 2007 20:51, Robert Kidd wrote:
> > On Apr 9, 2007, at 11:07 AM, Steven Bosscher wrote:
> > > Can you please send an updated patch?  The one from the link
> > > does not apply anymore to the current trunk.
> >
> > Certainly.
>
> Thank you.
> Looks all pretty good to me.  I still have a few questions though:
>
> > @@ -276,26 +301,25 @@ tail_duplicate (void)
> >  	      && can_duplicate_block_p (bb2))
> >  	    {
> >  	      edge e;
> > -	      basic_block old = bb2;
> > -
> > -	      e = find_edge (bb, bb2);
> > +	      basic_block copy;
> >
> >  	      nduplicated += counts [bb2->index];
> > -	      bb2 = duplicate_block (bb2, e, bb);
> >
> > -	      /* Reconsider the original copy of block we've duplicated.
> > -	         Removing the most common predecessor may make it to be
> > -	         head.  */
> > -	      blocks[old->index] =
> > -		fibheap_insert (heap, -old->frequency, old);
> > +	      e = find_edge (bb, bb2);
> > +	
> > +	      copy = duplicate_block (bb2, e, bb);
> > +	      flush_pending_stmts (e);
> > +
> > +	      add_phi_args_after_copy (&copy, 1);
>
> I don't understand why you've removed the code to reconsider the
> original copy.  This is not in your ChangeLog entry, but I assume
> you have a reason why you remove this.  Can you explain this please?
>
>
>
> > -/* Connect the superblocks into linear sequence.  At the moment  
> > we attempt to keep
> > -   the original order as much as possible, but the algorithm may  
> > be made smarter
> > -   later if needed.  BB reordering pass should void most of the  
> > benefits of such
> > -   change though.  */
> > -
> > -static void
> > -layout_superblocks (void)
> > -{
>
> *snif* goodbye...
>
> I suppose removing this is a good idea, but it makes me wonder if we
> should perhaps run tracer only if we know that the BB reordering pass
> will run.  In other words:
>
> > +gate_tracer (void)
> >  {
> >    return (optimize > 0 && flag_tracer);
> >  }
>
> Check "(optimize && flag_tracer && flag_reorder_blocks)".  What do you
> think of this?
>
> The rest of the patch looks correct to me, and apart from the twolf
> problem, I think the results show that this change is worth it.
>
> Gr.
> Steven