Re: [new-regalloc-branch]: pre-reload pass

[Michael Matz <matz at kde dot org>]

Hi,

On Sat, 2 Feb 2002, Denis Chertykov wrote:

> This is a patch which realize pre-reload pass.

Cool.  Then I'll base any stuff I wanted to do on that topic on your
things.  Today I have no time anymore to really look at it, but I
nevertheless briefly browsed the diff.  Ugh.  It seems to me, you
basically took reload and slightly adjusted it to work on pseudos.  So of
course it's still 4000 lines, still we have those ugly different
RELOAD_FOR_bla types, huge find_*_reloads(), huge and involved everything,
written in a cumbersome style.  I wanted to get rid of reload, not rewrite
it (well, of course the functionality of reload I wanted to rewrite, but
not based on reload).

For instance: those different types of reloads are only necessary because
reload cummulatively collects reloads per insn for operands, perhaps in
separate passes, but doesn't touch any code.  Of course it then has to
know which reloads conflict in which way, because that information is
nowhere else.  I find this broken design.  The next reload-passes must
apply this information when considering the insn, but only sometimes, and
only partially, and later roll back this applying.  This is involved and
tricky and fragile.  Broken design.

What I would have done is to look at all operands, reload invalid ones,
_emit_ insns to reload them right now, and simply look again at those new
insns, to possibly reload also them.  Let other passes cope with
optimizations.  I think, that _much_ of the complexity of reload would go
away, if done in that way.

That having said, I nevertheless think your patch is usefull.  If nothing
else, it would at least provide a slightly smaller base than reload to
think about implementing it in the above way.

> Limitations:
> 1. This version can't reload wrong addresses.

Yes, one thing each time ;-)

>    I want to use rtl
>    defined addresses (define_address ...) in the future.

This seems like a good plan.  Do you already have thought about it some
more?  For instance how to represent self-modifying addresses (i.e.
{pre,post}-{in,de}crement) and such things?  The only thing I would fear
is that we then have to touch very many files, while right now the changes
are fairly local.  Not a real problem, but has to be considered.

> I have successfully bootstrapped on i386.
> I want to apply it, but I'm worry about another targets.

If it can be reasonably easy switched off again (say by commenting out the
call from reg_alloc()), and it still basically works (so there is an easy
way back, in case it breaks other architectures, to make it easier to
fix), I would say, just committ it.

> Also, I have almost ready patch for webclass pass - analog of
> regclass. (It was ready and tested before your big changes.)

(Sorry about that.)  Nice too.  The multiple regclass invocations are a
major time taker right now (without any real use, because we anyway don't
create new pseudos right now, which hopefully can change soon.)

> Sat Feb  2 18:58:07 2002  Denis Chertykov  <denisc@overta.ru>
>
> 	* ra.c (sort_and_combine_web_pairs): Disable 'for_move' to prevent
> 	abort inside 'combine'.

Do you have a small testcase showing that particular problem, so we can
analyze it?  (Preferably i386)


Ciao,
Michael.