Re: Bug in ra-colorize.c:merge_moves?

[Daniel Berlin <dberlin at dberlin dot org>]

On Tuesday, March 4, 2003, at 06:55  AM, Michael Matz wrote:
> > I've got a tree
> > with everything but your stack slots changes merged (including
> > pre-reload), and the number of coloring passes is about the same.
> > However, with your stack slots changes , it takes 3-5x as many passes
> > in those functions that spill (some functions went from 3 passes to 15
> > passes).
>
> Ugh, that's much.  I wouldn't have expected this.  The only thing which
> _should_ have changed is, that now sometimes there are stack-accesses
> while coloring, where formerly there were simply only accesses to
> SPILL_SLOT_P() pseudos.  I believe Denis also makes some local
> optimizations while creating the code for the stack accesses early.
>
> Are you sure, that you don't coalesce SPILL_SLOT_P pseudos with 
> non-spill
> slots?

I specifically had the fix in my tree to make sure it wasn't the 
problem.

It seems we spill more webs again after allocating stack slots, for 
some reason.
Then we do two passes, can't color, assign stack slots, then spill 
again.
Repeat a few times.

I don't have time to track down *why* it happens.

It would have been nice if Denis had time tested the code before adding 
it, to be honest. It's nice to improve code, but when the allocation 
times go from 20 to 60 seconds, it's probably not worth it.

> > I should also point out the code i pointed out in the previous message
> > is still broken, if you think it will ever do something.
> > It will never trigger, afaict.
>
> It will, with pre-reload.  One sub function of build_i_graph() is
> select_regclass(), which calls web_class(), which can put a web into
> SPILLED.  This happens if one reference of a pseudo has conflicting
> constraints with all other references.  In that case this reference is
> "spilled" (for a lack of a better word, not to be confused with 
> spilling
> resulting from register pressure; it's similar to reloads just on
> pseudos).  Such a web is then marked as changed, and Denis uses the
> SPILLED list for this.
>
> The integration with the allocator is rather ugly, I agree.

This is the other reason i didn't consider merging it.
The new-register allocator is supposed to be clean, above all.
> > I change it to an abort in that case, and bootstrapped, and it doesn't
> > trigger the abort (meaning it never falls into that case during a
> > bootstrap).
>
> On your platform.  Take one with stranger constraints, and involving
> sources which cast between funny types, and you somewhen will hit it.
Err,  I tried this too (three very odd platforms). Never triggers.
You sure it's correct?

> Ciao,
> Michael.