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

[Denis Chertykov <denisc at overta dot ru>]

> > This is a patch which realize pre-reload pass.
> nevertheless briefly browsed the diff.  Ugh.  It seems to me, you
> basically took reload and slightly adjusted it to work on pseudos.

Yes.

> 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.

I'm agree with you and I'm know about bad design of pre-reload pass,
but it was an easiest way to implement this pass. Now we can improve it
infinitely.

> 
> 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.

*Very* easy to implement on existing pre-reload.

> Let other passes cope with optimizations.
> I think, that _much_ of the complexity of reload would go
> away, if done in that way.

Original reload have a many rematerialization things based on
insn notes. I have removed it. Do you think it's right ?
IMHO: new allocator will have a rematerialization.

> 
> 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?

I'm think about it already about two years. I'm work on new allocator because
I want to implement (define_address ...). I have tried to implement
define_address on top of reload, but it's very difficult and not effective.
I have implemented define_address for ??? all parts exclude reload.

You can try to search a gcc@gcc.gnu.org archive for:
Subject: (define_address ...) suggestions needed
Date: Tue, 11 Jan 2000 23:25:12 +0300

One bad thing I lose my archives with implementation of define_address.


> For instance how to represent self-modifying addresses (i.e.
> {pre,post}-{in,de}crement) and such things?

(define_address "post_inc"
  (mem:HI (post_inc:HI (match_operand:SI 0  "register_operand" "b")))
  ""
  "%a")

> 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.

It's not so many files if you exclude changes in reload.

I can describe my thinks about define_address more precisely. If you
interested. 


> > 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)

Probably original version of ra.c don't generate abort inside 'combine'
because you use union of reg_preferred_class and reg_alternate_class.
reg_alternate_class always GENERAL_REGS or ALL_REGS.
But imagine. for_move = 1 and you have a call to sort_and_combine_web_pairs
and register classes of combining webs are not intersects.
Condition (in which I have removed 'for_move ||') will be true and combine ()
will be called. So, combine () will be called for non intersecting webs.
IE fragment of combine () (without original comments):

  AND_HARD_REG_SET (u->usable_regs, v->usable_regs);
  u->num_freedom = hard_regs_count (u->usable_regs);
  if (!u->num_freedom)
    abort();

This abort will be called because usable_regs not intersects.


Denis.

PS: My plans:
1) commit pre-reload;
2) prepare and commit web_class;
2) Improve pre-reload and web_class.