Re: [Bug tree-optimization/35805] [ira] error in start_allocno_priorities, at ira-color.c:1806

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

Paolo Bonzini wrote:
> Kenneth Zadeck wrote:
>   
>> 2009-01-01  Kenneth Zadeck <zadeck@naturalbridge.com>
>>
>>     PR rtl-optimization/35805
>>     * df-problems.c (df_lr_finalize): Add recursive call to resolve lr
>>     problem if fast dce is able to remove any instructions.
>>     * dce.c (dce_process_block): Fix dump message.
>>    
>> This patch fixes the problem.  The comment in the patch describes the
>> issue.    Since this was not really a failure, it would be hard to make
>> this issue into a testcase.
>>     
>
> IIUC the bugzilla comment trail, this caused
> gcc.c-torture/compile/930523-1.c to fail with --enable-checking=df;
> that's already a testcase.
>
>   
>> Ok to commit?
>>     
>
> Hmmm... I am not sure I like this patch, for two reasons.
>
> 1) it might incur a compile-time penalty for the sake of verification,
> even with df checking disabled.  OTOH having possibly different code for
> checking and non-checking compilation is even worse.
>
>   
There is a compile time penalty here but it is not for the sake of
verification.   It is for the sake of getting the best answer going
forward, into the computation of live.

There was a deeper bug here.   The code that was removed which cleared
the solutions_dirty flag is really wrong, because it lets the
conservative solution go forward and the next call to df_analyze will
not even try to redo anything and thus improve the solution. That was
how vlad saw the extra bits even though he was calling df_analyze before
using the bits.

 On the other hand, if you do not clear that flag in the old way, the
verifier will fail.
> 2) there are already provisions in dce.c to redo the analysis.  But they
> do not get to the least fixed point because they just rebuild the local
> bitmaps and iterate from the existing solution.  Instead of iterating
> "while (global_changed)", we could try doing only one iteration (it's a
> fast DCE after all, and the pessimistic dataflow makes me guess that
> subsequent DCE iterations won't find much?) and zap the solution there.
>  This has the advantage that we can skip the recomputation if
> global_changed is false.
>
> Did I miss anything?
>
>   
I think so.   The global changed flag allows it to delete the case:

loop:
  ... <- x  // This is dead.
 x- <- ...
go to loop

it just is not going to get rid of it if there is is no kill of x inside
the loop.

Anyway. the loop inside the fast dce code will only cause one extra
iteration of the blocks, and because of that it is still pessimistic.
>   


> Paolo
>