Re: Basic block renumbering removal

[Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>]

Hello.

> > + expunge_block (b)
> > +      basic_block b;
> > + {
> >   
> > !   unlink_block (b);
> 
> Careful with the extra whitespace.

I have removed as much as I found (but surely some of them remains;
I really am not able to find all in 16000 lines of patch, sorry).

> > ! /*      fprintf (file, "prev %d, next %d, ",
> > ! 	       bb->prev_bb->sindex, bb->next_bb->sindex);*/
> 
> Don't leave stuff like this.

OK.

> >     /* Allocate the tree.  */
> > !   dfst = (struct dfst_node *) xcalloc (last_basic_block,
> >   				       sizeof (struct dfst_node));
> >   
> > !   memset (dfst, 0, last_basic_block * sizeof (struct dfst_node));
> 
> You added this memset.  Why?

Probably some artefact from merging. I've removed it.

> > ! 	  if (e->src->index > e2->src->index)
> > ! 	    continue;
> >   
> >   	  if (try_crossjump_to_edge (mode, e, e2))
> >   	    {
> > --- 1547,1555 ----
> >   	     checks of crossjump(A,B).  In order to prevent redundant
> >   	     checks of crossjump(B,A), require that A be the block
> >   	     with the lowest index.  */
> > ! 	  for (foll = e->src; foll && foll != e2->src; foll = foll->next_bb);
> > !           if (!foll)
> > !             continue;
> 
> First, the trailing semicolon is extremely hard to read.  Especially
> with the if mis-indented.  Write things like this as
> 
> 	for (...)
> 	  continue;

OK.

> Second, this test exists only to cut down the number of redundant
> tests.  Thus we are not interested in the actual ordering of the
> blocks, only that every block has a unique index.  Thus the original
> test is still correct.

I had two reasons for this:
1) I wanted to keep semantics the same as before, in order to make bughunting
easier.
2) It did not work other way (in cfg-branch; I didn't check this after adapting
the patch for mainline).

> > *************** back_edge_of_syntactic_loop_p (bb1, bb2)
> > +   for (bb = bb1; bb && bb != bb2; bb = bb->next_bb);
> 
> Again with the continue.

OK.

> >     ae_kill = (sbitmap *) sbitmap_vector_alloc (n_blocks, n_exprs);
> > !   sbitmap_vector_zero (ae_kill, last_basic_block);
> 
> These should all be consistent and use n_blocks.

OK.

> > !   for (block_num = 0; block_num < n_basic_blocks; )
> > !     {
> > !       basic_block bb = BASIC_BLOCK (block_num);
> > !       if (find_if_header (bb))
> > ! 	block_num = bb->index;
> > !       else 
> > ! 	block_num++;
> > !     }
> > --- 2690,2704 ----
> > !   FOR_ALL_BB (bb)
> > !     find_if_header (bb);
> 
> Not the same.  The replacement would be
> 
> 	for (bb = ENTRY_BLOCK_PTR->next_bb; bb != EXIT_BLOCK_PTR ;)
> 	  if (! find_if_header (bb))
> 	    bb = bb->next_bb;
> 
> or
> 
> 	FOR_ALL_BB (bb)
> 	  while (find_if_header (bb))
> 	    continue;

OK.

Here is the patch with proposed changes.

Zdenek

Attachment: diff_noren_head.diff.bz2
Description: Binary data