Re: Unreviewed patches

[Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>]

Hello,

> > http://gcc.gnu.org/ml/gcc-patches/2003-02/msg01238.html
> >   wrong new files atached here, fixed by
> > http://gcc.gnu.org/ml/gcc-patches/2003-02/msg01262.html
> >   -- foundation for toplevel driver cleanup
> 
> I feel that this is still trying to do too much at once.  Suggested
> plan of attack:
> 
> Patch 0, rip all the dump-file code out of toplev.c into its own
> file, call it rtl-dump.c.
> 
> Patch 1..n, one optimizer pass at a time, either modify the existing
> entry point from toplev.c (preferred) or create a new entry point (if
> the existing interface is needed elsewhere) to match a restricted
> signature [bool (*)(void) suggested, where the return value means
> 'changed something'].  Push all the surrounding paraphernalia from
> rest_of_compilation -- timevar pushes and pops, dump file handling,
> etc -- down into the entry point function.  The entry point functions
> should go in the same file as the rest of the pass, not in a separate
> file.
> 
> Patch n+1 then introduces passes.def and converts rest_of_compilation
> to a loop over a vector of function pointers.

I don't like this idea too much:
1) I don't think I am able to create *that* much clean solution in one
   pass.
2) thus I would prefer to have all driver functions in one file so
   that they will be easier to access during following incremental
   updates.

I may split this patch somewhat like you suggest at point 2 (not exactly happy about
it, as it will require me to do about 30 trivial patches, but will do if
you insist).

> > http://gcc.gnu.org/ml/gcc-patches/2003-01/msg02059.html
> >   -- fix for latent bug in liveness analysis
> 
> This looks like a problem worth fixing, but (a) it's not obvious to me
> why we are starting from nonempty sets in the first place,

because it is faster than to compute it always from scratch.

> (b) it's
> not obvious why your fallback strategy is guaranteed to terminate,

see the proof in the comment I have added to calculate_global_regs_live.

> and
> (c) I'd like to see a test case that triggers nontermination.

actually I am not aware of any just now. It have bitten me already
several times, and fixing it (by improving local updating of liveness
info) took me quite some time. Yes, probably this patch is not strictly
neccessary, but unless we are able to prove the current algorithm
always terminates on inputs we give to it, I would prefer to have
it corrected.

> > http://gcc.gnu.org/ml/gcc-patches/2003-02/msg01655.html
> >   -- fix for ugly code generated for small array initializers
> 
> Feels like papering over the underlying problem; however, the
> underlying problem is probably that we have ADDRESSOF in the first
> place...  I'd like a reaction from someone better familiar with the
> issues here.

me too.

> > http://gcc.gnu.org/ml/gcc-patches/2003-02/msg01285.html
> >   -- split log links creation from flow.c
> 
> Looks to *me* like a good cleanup.  But, since Richard objected, I
> would rather hear what he thought of your response to his objection.
> 
> (create_log_links is only ever called with NULL parameter -- why is
> that parameter present?)

because it might come in handy in future if we decided that we want/need
to update just some of log links (not quite probable).

Zdenek