Re: RFA: patch to solve IRA problem for PR37448

[Vladimir Makarov <vmakarov at redhat dot com>]

Jeff Law wrote:
> Vladimir Makarov wrote:
> >  The following patch solves problem 37448 in IRA for -O2 mode.  To
> > achieve decent IRA speed I rewrote coalescing spilled allocnos
> > analogous to code used for IRA allocno allocation in -O0 mode.  It
> > permitted to decrease IRA work time from >1hour to 50sec on a Core2
> > machine.  The patch also contains compression of live ranges which
> > permits to decrease the time to 45sec so IRA spends 23% of all
> > compiler time in -O2 for this pathological test.  The compression
> > usually decreases live ranges in 3 times and should also speed up IRA
> > in -O0 (although IRA in -O0 is already faster than the old RA).
> >
> >  The patch does not change (and should not change) the generated
> > code.  The patch was successfully bootstrapped on x86_64.
> >
> > 2008-09-12 Vladimir Makarov <vmakarov@redhat.com>
> >
> >    PR middle-end/37448
> >       * ira-int.h (IRA_ALLOCNO_TEMP): Rename to ALLOCNO_TEMP.
> >    (ira_compress_allocno_live_ranges): New prototype.
> >
> >    * ira-color.c: Rename IRA_ALLOCNO_TEMP to ALLOCNO_TEMP.
> >    (coalesced_allocnos_living_at_program_points): New.
> >    (coalesced_allocnos_live_at_points_p,
> >    set_coalesced_allocnos_live_points): New functions.
> >    (coalesce_spill_slots): Rewrite.
> >       * ira-lives.c (remove_some_program_points_and_update_live_ranges,
> >    ira_compress_allocno_live_ranges): New functions.
> >
> >    * ira-build.c (ira_flattening): Call
> >    ira_compress_allocno_live_ranges.
> >    (ira_build): Ditto.
> In coalesced_allocnos_live_at_points_p and 
> set_coalesced_allocnos_live_points you have the following loop:
>
>
>
> +  for (a = ALLOCNO_NEXT_COALESCED_ALLOCNO (allocno);;
> +       a = ALLOCNO_NEXT_COALESCED_ALLOCNO (a))
> +    {
> ... body ...
> +      if (a == allocno)
> +    break;
> +    }
>
> Aren't these just do ... while (a != allocno) loops?  I think that 
> would be marginally clearer (I certainly had to look at them a couple 
> times to realize that we didn't miss handling ALLOCNO).
> Not something I'd require you to change, just curious to hear your 
> thoughts.
Yes, I think you are right.  I realized that I use for-statements mostly 
when while is more clear.  That is probably because I used a lot a 
C-like programming language where there is only for-statements for loops.
> Otherwise it looks fine.  For the bootstrap comparison failure -- in 
> the future it'd be useful to provide more information about how we did 
> an uninitialized memory read -- just saying we did an uninitialized 
> read isn't terribly helpful.  The only reason I'm not requiring that 
> analysis is it's pretty clear to me that you just made a typo when 
> inverting the sense of the conditional in the changes for 37448.
Thanks, Jeff.
>  
>
> ps.  You mentioned 37488 in your original message, it's pretty clear 
> to me that you really wanted 37448.  37488 is an IRA runtime 
> performance issue, 37448 is an IRA compile-time issue.
Yea, right.  It is easy to be lost when you have a lot of PRs :)