Re: Basic block renumbering removal

[Richard Henderson <rth at redhat dot com>]

On Wed, May 15, 2002 at 05:11:10PM +0200, Zdenek Dvorak wrote:
> + expunge_block (b)
> +      basic_block b;
> + {
>   
> !   unlink_block (b);

Careful with the extra whitespace.

> ! /*      fprintf (file, "prev %d, next %d, ",
> ! 	       bb->prev_bb->sindex, bb->next_bb->sindex);*/

Don't leave stuff like this.

> !   FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR, EXIT_BLOCK_PTR, next_bb)
>       {
>   
>         for (e = bb->succ; e; e = e->succ_next)

Again.  I'll quit mentioning it for this patch.  But there's quite
a few others.

>     /* 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?

> ! 	  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;

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.

> *************** back_edge_of_syntactic_loop_p (bb1, bb2)
> +   for (bb = bb1; bb && bb != bb2; bb = bb->next_bb);

Again with the continue.

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

> !   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;


r~