Re: IRA plans and latest version of IRA patch

[Bernd Schmidt <bernds_cb1 at t-online dot de>]

Vladimir Makarov wrote:
> I broke reload changes in several parts.  The parts are in the
> attachments.  I should mention they are not real patches because they
> are got from editing reload1.c patch.  They help to understand what
> part is for particular improvement.

I'll have to go through these in stages.  Sorry for the slowness.

> Allocation of hard registers to pseudos spilled by RA.  It is possible
> because reload can spill some pseudos and pseudos spilled earlier by
> RA living in live range of pseudos spilled by the reload can get the
> correspoding hard registers.  Such thing was absent before the patch:
>   - Pseudos spilled by RA is added to chain live_throughout.

This should be documented in the structure definition.  What isn't 
immediately obvious is why IRA needs this (reassign_pseudos does not 
appear to make use of it) while the old RA's retry_global_alloc 
functions without it.
Please give some more information about the reasons for this change.

Also,

> +                       /* Consider spilled pseudos too for IRA
> +                          because they still have a chance to get
> +                          hard-registers in the reload when IRA is
> +                          used.  */
> +                       else if (reg_renumber[regno] >= 0
> +                                || (flag_ira && optimize))

When the same comment appears multiple times I think it's time to add a 
new function.
On the whole I'm not very happy about having two separate meanings for 
the data structure and two separate code paths, so if this can be 
unified further, it would be better.

>   EXECUTE_IF_SET_IN_REG_SET (&spilled_pseudos, FIRST_PSEUDO_REGISTER, 
i, rsi)
[...]
> +    if (! flag_ira || ! optimize || reg_renumber[i] >= 0)
> +      {
> +       /* Record the current hard register the pseudo is allocated to
> +          in pseudo_previous_regs so we avoid reallocating it to the
> +          same hard reg in a later pass.  */
> +       gcc_assert (reg_renumber[i] >= 0);

The if statement is new with the patch.  It looks like it'll abort for 
!flag_ira || !optimize, but I guess that can't happen due to the changed 
meaning of spilled_pseudos?  As above, having these two alternate paths 
in the code is confusing.

> -  spill_add_cost[r] -= REG_FREQ (reg);
> +  spill_add_cost[r] -= freq;
>    while (nregs-- > 0)
>     spill_cost[r + nregs] -= REG_FREQ (reg);

Somewhat inconsistent here in the use of freq instead of REG_FREQ.


Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif