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: rtlopt merge part 2 - bb-reorder rewrite (updated)


> On Fri, Feb 07, 2003 at 10:05:31AM +0100, Josef Zlomek wrote:
> > this is a rewrite of bb-reorder. It performs also a loop rotation
> > and duplication of some small hot basic blocks (especially duplicating 
> > the return instruction should have significant effect).
> 
> This I wanted to see very much.  Thanks.
> In general, the code looks pretty good.
> 
> > !   start_of_trace = xmalloc (array_size * sizeof (int));
> > !   end_of_trace = xmalloc (array_size * sizeof (int));
> > !   bb_heap = xmalloc (array_size * sizeof (fibheap_t));
> > !   bb_node = xmalloc (array_size * sizeof (fibnode_t));
> 
> You'll get better cache locality if you rearrange these
> into one array of structures.  Also, don't I see bb_heap
> always initialized?  Perhaps
> 
> > !       if (bb_heap)
> > ! 	{
> > ! 	  bb_heap = xrealloc (bb_heap, new_size * sizeof (fibheap_t));
> 
> is leftover from somewhen?
> 
> > ! static int
> > ! get_uncond_jump_length ()
> > ! {
> > !   rtx label, jump;
> > !   int length;
> > ! 
> > !   label = emit_label_before (gen_label_rtx (), get_insns ());
> > !   jump = emit_jump_insn (gen_jump (label));
> > ! 
> > !   length = get_attr_length (jump);
> > ! 
> > !   delete_insn (jump);
> > !   delete_insn (label);
> > !   return length;
> >   }
> 
> It'd be nice to call this only once. 
> 
> Have you looked to see that this produces reasonable results
> for targets for which branch shortening is used?  Mostly I 
> mean "doesn't crash", since this is only a heuristic, but
> "returns max size" would be nice.

It does return minimal size.  get_attr_length is having code to set
0 addresses.  PA port does have C function that access addresses
unconditionally, but I sent patch for that some time ago.
Minimal size is probably good estimate.
> 
> (1) At present this is being run after sched2, which means that the block
>     duplication we're doing isn't as useful as it could be.  If we duplicate
>     block C and place it after block B, we'd like to be able to interleave
>     the insns from C and B.

I sent patch for that already in the merge patch adding switch for
superblock and trace scheduling.  it is disabled for normal scheduling
but we can run bb-reorder before sched2 uncodnitonally and only rerun it
when reg-stack or crossjumping happent.
> 
>     The current placement of bb-reorder stems from wanting to do the reorder
>     after reg-stack does it's nastiness.  Have you looked into what the
>     effects of running bb-reorder twice?  If this is too painful, we could

Yes, it works well in general.

>     The thing about these relocations is, they have a sequence number
>     attached to them for the assembler to process (the 10 and 11 in the
>     example).  This sequence number indicates certain data flow properties
>     about these instructions.
> 
>     With your patch, however, we've destroyed that sequence information,
>     since we now for the first time we duplicate instructions after
>     peep2.  Indeed, the assembler issues error messages for duplicate
>     sequence numbers.
> 
>     The solution, I think is to have a target hook that the backend can
>     use to frob these sequence numbers.  Everywhere but Alpha we'd do
>     nothing (or just call copy_rtx, however it gets set up).
> 
>     I'll take care of this myself, I guess, since I know what's going
>     on and can test the result.

Hmm, I didn't noticed this. Thanks!

Honza

> 
> 
> r~


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