Re: Ping: IRA-based register pressure calculation for RTL loop invariant motion

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Oct 14, 2009 at 6:27 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> Zdenek Dvorak wrote:
>>
>> Hi,
>>
>>>>> + ? ? ?if (i < ira_reg_class_cover_size)
>>>>> + ? ? ? size_cost = comp_cost + 10;
>>>>> + ? ? ?else
>>>>> + ? ? ? size_cost = 0;
>>>>>
>>>>
>>>> Including comp_cost in size_cost makes no sense (this would prevent us
>>>> from
>>>> moving even very costly invariants out of the loop if we run out of
>>>> registers).
>>>>
>>>>
>>>
>>> That is exactly what I intended. ?As I wrote above, I tried a lot of
>>> ?heuristics with different parameters which decided to move loop ?invariant
>>> depending on spill cost and loop invariant cost. ?But they ?don't ?work well
>>> at least for x86/x86_64 and power6. ?I have some ?speculation for this.
>>> ?x86/x86_64 is OOO processors these days. ?And ?costly invariant will be
>>> hidden because usually the invariant has a lot ?of freedom to be executed
>>> out-of-order. ?For power6, long latency is ?hidden by insn scheduling. ?It
>>> is hard to me find a processor where it ?will be important. ?Another reason
>>> for this, it is very hard to evaluate ?accurately spill cost at this stage.
>>> ?So I decided not to use ?combination of register pressure and invariant
>>> cost in my approach.
>>
>> could you please add this reasoning to the comment? ?Another reason why
>> preventing the invariant motion does not hurt might be that all expensive
>> invariants were already moved out of the loop by PRE and gimple invariant
>> motion pass.
>>
>>> + ? ? ?for (i = 0; i < ira_reg_class_cover_size; i++)
>>> + ? ? ? {
>>> + ? ? ? ? cover_class = ira_reg_class_cover[i];
>>> + ? ? ? ? if ((int) new_regs[cover_class]
>>> + ? ? ? ? ? ? + (int) regs_needed[cover_class]
>>> + ? ? ? ? ? ? + LOOP_DATA (curr_loop)->max_reg_pressure[cover_class]
>>> + ? ? ? ? ? ? + IRA_LOOP_RESERVED_REGS
>>> + ? ? ? ? ? ? - ira_available_class_regs[cover_class] > 0)
>>> + ? ? ? ? ? break;
>>> + ? ? ? }
>>
>> It might be clearer to write this as ... >
>> ira_available_class_regs[cover_class] instead
>> of ... - ira_available_class_regs[cover_class] > 0. ?Otherwise, the patch
>> is OK.
>>
> Zdenek, thanks for the additional comments. ?I incorporated them into the
> patch just before committing. ?Here is the affected patch part:

I think this consistently regressed both compile-time and runtime for
Polyhedron on x86_64.  For Itanium the story isn't clear, but effects
are seen there as well (it's also the only one I see off-noise effects
on SPEC 2000 - significant ups and downs).

Richard.

> @@ -1066,15 +1168,62 @@ get_inv_cost (struct invariant *inv, int
>
> static int
> gain_for_invariant (struct invariant *inv, unsigned *regs_needed,
> - ? ? ? ? ? ?unsigned new_regs, unsigned regs_used, bool speed)
> + ? ? ? ? ? ?unsigned *new_regs, unsigned regs_used, bool speed)
> {
> ?int comp_cost, size_cost;
>
> - ?get_inv_cost (inv, &comp_cost, regs_needed);
> ?actual_stamp++;
>
> - ?size_cost = (estimate_reg_pressure_cost (new_regs + *regs_needed,
> regs_used, speed)
> - ? ? ? ? ? - estimate_reg_pressure_cost (new_regs, regs_used, speed));
> + ?get_inv_cost (inv, &comp_cost, regs_needed);
> +
> + ?if (! flag_ira_loop_pressure)
> + ? ?{
> + ? ? ?size_cost = (estimate_reg_pressure_cost (new_regs[0] +
> regs_needed[0],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? regs_used, speed)
> + ? ? ? ? ? - estimate_reg_pressure_cost (new_regs[0],
> + ? ? ? ? ? ? ? ? ? ? ? ? regs_used, speed));
> + ? ?}
> + ?else
> + ? ?{
> + ? ? ?int i;
> + ? ? ?enum reg_class cover_class;
> +
> + ? ? ?for (i = 0; i < ira_reg_class_cover_size; i++)
> + ? ?{
> + ? ? ?cover_class = ira_reg_class_cover[i];
> + ? ? ?if ((int) new_regs[cover_class]
> + ? ? ? ? ?+ (int) regs_needed[cover_class]
> + ? ? ? ? ?+ LOOP_DATA (curr_loop)->max_reg_pressure[cover_class]
> + ? ? ? ? ?+ IRA_LOOP_RESERVED_REGS
> + ? ? ? ? ?> ira_available_class_regs[cover_class])
> + ? ? ? ?break;
> + ? ?}
> + ? ? ?if (i < ira_reg_class_cover_size)
> + ? ?/* There will be register pressure excess and we want not to
> + ? ? ? make this loop invariant motion. ?All loop invariants with
> + ? ? ? non-positive gains will be rejected in function
> + ? ? ? find_invariants_to_move. ?Therefore we return the negative
> + ? ? ? number here.
> +
> + ? ? ? One could think that this rejects also expensive loop
> + ? ? ? invariant motions and this will hurt code performance.
> + ? ? ? However numerous experiments with different heuristics
> + ? ? ? taking invariant cost into account did not confirm this
> + ? ? ? assumption. ?There are possible explanations for this
> + ? ? ? result:
> + ? ? ? ? ? o probably all expensive invariants were already moved out
> + ? ? ? ? ? ? of the loop by PRE and gimple invariant motion pass.
> + ? ? ? ? ? o expensive invariant execution will be hidden by insn
> + ? ? ? ? ? ? scheduling or OOO processor hardware because usually such
> + ? ? ? ? ? ? invariants have a lot of freedom to be executed
> + ? ? ? ? ? ? out-of-order.
> + ? ? ? Another reason for ignoring invariant cost vs spilling cost
> + ? ? ? heuristics is also in difficulties to evaluate accurately
> + ? ? ? spill cost at this stage. ?*/
> + ? ?return -1;
> + ? ? ?else
> + ? ?size_cost = 0;
> + ? ?}
>
> ?return comp_cost - size_cost;
> }
>
>