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: [PATCH]: update to df for mainline.


This is really a reply to all of the mail up to 11:02 est but those
other messages deleted some of the things I wanted to say.

Bernd Schmidt wrote:
> Thunderbird crashed a few times while I was trying to reply in-line,
> hence please read the attachment.
>
>
> Bernd
> ------------------------------------------------------------------------
>
>   
>> @@ -1845,7 +1952,7 @@ df_record_exit_block_uses (struct datafl
>>       If we end up eliminating it, it will be removed from the live
>>       list of each basic block by reload.  */
>>    
>> -  if (! reload_completed || frame_pointer_needed)
>> +  if ((! reload_completed) || frame_pointer_needed)
>>    {
>>      bitmap_set_bit (df->exit_block_uses, FRAME_POINTER_REGNUM);
>> if FRAME_POINTER_REGNUM != HARD_FRAME_POINTER_REGNUM
>>     
>
> Not an improvement.  Several such cases.
>
> The patch is inconsistent in the formatting of the ! operator; and while I
> dislike the coding style recommendation, I'll have to ask you to consistently
> format things without the space.
>   
I will fix this consistently as per the coding standards.  I had done
some greps of this and found the compiler is very inconsistent and did
not put much effort into this one based on that. 
>   
>>    /* Init Data Flow analysis, to be used in interloop dep calculation.  */
>>    df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES |	DF_SUBREGS);
>>     
>
> Stray tab character.  Elsewhere too.
>
>
> The patch is ok with those changes.  I have some additional requests though.
>
> In the future, please submit one patch per functional change.  This set had
> coding style fixups, code movement, bugfixes, and extra features all mixed
> into one.  You're much more likely to get a review for something that doesn't
> need to be disentangled.
>
>   
I will try.

> Also, I've been looking at the code for the urec problem, and it looks a bit
> dodgy at first glance.  What can you tell me about this code - when does it
> run, who uses it?
>
>   

I would love to see this code die. This is basically a repackaging of
the global.c:make_accurate_live_analysis but done so that it fits into
the df framework.   It is an improvement over make_accurate_live
analysis and the plan is to replace that with this as a temporary solution.

I have privately talked to vlad and to peter bergner about getting rid
of this and using just the ur problem. The problem is that the way the
early clobber info is used (both here and in
make_accurate_live_analysis) is way too conservative.  In urec as in
make_accurate_live_analysis, if a register input to and insn is early
clobber, that register is marked live from the instruction all of the
way back to the beginning of the basic block.

The proper way to model early clobber is to have the interference graph
builder add an interference from the ec register to the other registers
live across this instruction and do not look at early clobber at all
when building the dataflow. 

> Have you done any testing to ensure that your version of the flow code finds
> the same REG_DEAD/REG_UNUSED notes, and computes the same values for things
> like REG_FREQ or REG_LIVE_LENGTH?
>
>
>   
On the dataflow branch these have been tested extensively.  At this
point I have removed almost all of flow and replaced it with this code. 
The dataflow branch works regression free on the three platforms I have
tested on. 

In general, we find lower number for the reg info than does the flow
based solution.  This is because the df version of the numbers are based
on computing live variables ANDed with ur or urec rather than just using
live variables as flow does.  This means that the paths pruned by the
addition of ur or urec do not get to contribute to the statistics.  
This does not cause any problems because urec is used by the allocator
to assign the registers so this just means that the stats match the
register allocator.

As far as the REG_DEAD and REG_UNUSED, we did extensive testing and hand
tracked down the differences.  There are differences since we tend to
have more accurate dataflow info.  Again, the compilers built that use
this exclusively bootstrap and regression test on three platforms.
> Bernd
>   

Kenny


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