Basic block renumbering removal

Zdenek Dvorak rakdver@atrey.karlin.mff.cuni.cz
Thu May 16 11:42:00 GMT 2002


Hello.

> > > 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).
> 
> Hum.  I guess we can leave this for now, but I'm certain this
> will turn into a quadratic performance problem with some test
> case.  Please figure out how to address this.
> 
> I suspect that both here and in back_edge_of_syntactic_loop_p
> you can arrange for the block indicies to be monotonicaly 
> increasing.

I will look on this.

> Anyway:
> 
> > ! compact_blocks ()
> >   {
> > !   basic_block *bbs = xcalloc (num_basic_blocks, sizeof (basic_block));
> > !   int i;
> > !   basic_block bb;
> > !  
> > !   i = 0;
> > !   FOR_ALL_BB (bb)
> > !     bbs[i++] = bb;
> > ! 
> > !   if (i != num_basic_blocks)
> > !     abort ();
> >   
> > !   for (i = 0; i < num_basic_blocks; i++)
> >       {
> > !       bbs[i]->sindex = i;
> > !       BASIC_BLOCK (i) = bbs[i];
> >       }
> > +   last_basic_block = num_basic_blocks;
> > + 
> > +   free (bbs);
> 
> You don't need to allocate extra memory here.
> 
> 	VARRAY_GROW (basic_block_info, num_basic_blocks);
> 
> 	i = 0;
> 	FOR_ALL_BB (bb)
> 	  {
> 	    BASIC_BLOCK (i) = bb;
> 	    bb->sindex = i++;
> 	  }
> 	if (i != num_basic_blocks)
> 	  abort ();

Right.

> > !   /* Place the new block to the end.  */
> > !   VARRAY_GROW (basic_block_info, last_basic_block);
> 
> Probably VARRAY_BB_PUSH would be better, since that expands
> the array geometricly rather than linearly.

OK.

> > *************** alloc_rd_mem (n_blocks, n_insns)
> >     rd_kill = (sbitmap *) sbitmap_vector_alloc (n_blocks, n_insns);
> > !   sbitmap_vector_zero (rd_kill, last_basic_block);
> 
> You didn't change these like I asked.
> 
> > *************** alloc_avail_expr_mem (n_blocks, n_exprs)
> >     ae_kill = (sbitmap *) sbitmap_vector_alloc (n_blocks, n_exprs);
> > !   sbitmap_vector_zero (ae_kill, n_blocks);
> 
> Oh, sorry, you did.  But there's another function that
> wants the same treatment.

OK.

> >   #define BB_TO_GCOV_INDEX(bb)  ((bb) == ENTRY_BLOCK_PTR ? 0		\
> >   			       : ((bb) == EXIT_BLOCK_PTR		\
> > ! 				  ? last_basic_block + 1 : (bb)->sindex + 1))
> 
> I didn't notice a compact_blocks before profile?  If I didn't just
> miss it, seems like it would be a Good Idea, since last_basic_block
> will then affect the size of the data in the object file.

compact_blocks is run in every cfg_cleanup (and cfg_cleanup is run before
branch_prob); but cfg_cleanup then can make some changes, so you're right
in that we should rerun it again immediatelly before.

Zdenek



More information about the Gcc-patches mailing list