diff mbox

[4/4] Make SMS schedule register moves

Message ID g4ipnwgcwq.fsf@richards-thinkpad.stglab.manchester.uk.ibm.com
State New
Headers show

Commit Message

Richard Sandiford Oct. 11, 2011, 8:17 a.m. UTC
Ayal Zaks <ayal.zaks@gmail.com> writes:
>>> The issue of assigning stages to reg-moves is mostly relevant for
>>> prolog and epilog generation, which requires and receives special
>>> attention -- handled very nicely by ps_num_consecutive_stages! Note
>>> that currently a simple boolean indicator for (the exceptional case
>>> of) double stages would suffice, instead of generalizing to arbitrary
>>> nums of consecutive stages (see any potential use for them?).
>>
>> Not in the immediate term.  But I think having a boolean indicator
>> would be inconsistent.  If the distance field is an int (even though
>> we only expect distance-0 and distance-1 register dependencies)
>> then I think the number of stages should be too.
>>
>> I did wonder originally about using a boolean, but IMO, it makes
>> the code less readable rather than more.  Instead of a simple
>> range check like:
>>
>>     if (first_stage_for_insn <= last_stage_in_range
>>         && last_stage_for_insn >= first_stage_in_range)
>>
>> we end up with the equivalent of:
>>
>>     if (first_stage_for_insn <= last_stage_in_range
>>         && (double_stage_move_p (...)
>>             ? first_stage_for_insn + 1 >= first_stage_in_range
>>             : first_stage_for_insn >= first_stage_in_range))
>>
>> with no corresponding simplification elsewhere.
>>
>
> Sure. But setting the range can be done by consulting an simple
> indicator, rather than generalizing to arbitrary stage numbers; e.g.:
>
> +ps_num_consecutive_stages (partial_schedule_ptr ps, int id)
> +{
> +  if (id >= ps->g->num_nodes && ps_reg_move (ps, id)->double_stages)
> +    return 2;
> +  else
> +    return 1;
> +}
>
> or
>
> -  last_u = first_u + ps_num_consecutive_stages (ps, u) - 1;
> +  if (...double_stages) last_u = first_u + 1;
> +  else last_u = first_u;

Understood.  I still prefer the posted version though.

>> E.g. adding something like this at the end:
>>
>>   ??? The algorithm restricts the scheduling window to II cycles.
>>   In rare cases, it may be better to allow windows of II+1 cycles.
>>   The window would then start and end on the same row, but with
>>   different "must precede" and "must follow" requirements.
>>
>> Let me know what you think and I'll add it as a follow-on patch.
>>
>
> great, thanks.

OK, added with the patch below.

>>>> +
>>>> +   The move is part of a chain that satisfies register dependencies
>>>> +   between a producing ddg node and various consuming ddg nodes.
>>>> +   If some of these dependencies cross a loop iteration (that is,
>>>> +   have a distance of 1) then DISTANCE1_USES is nonnull and contains
>>>> +   the set of uses with distance-1 dependencies.  DISTANCE1_USES
>>>> +   is null otherwise.
>>>> +
>>>
>>> Maybe clarify that they are upwards-exposed or live-in uses.
>>
>> OK, changed to:
>>
>>   The move is part of a chain that satisfies register dependencies
>>   between a producing ddg node and various consuming ddg nodes.
>>   If some of these dependencies have a distance of 1 (meaning that
>>   the use is upward-exposoed) then DISTANCE1_USES is nonnull and
>
> exposed (typo)

Oops, also fixed below (and applied).

Richard


gcc/
	* modulo-sched.c: Fix comment typo.  Mention the possibility
	of using scheduling windows of II+1 cycles.
diff mbox

Patch

Index: gcc/modulo-sched.c
===================================================================
--- gcc/modulo-sched.c	2011-10-10 12:42:41.000000000 +0100
+++ gcc/modulo-sched.c	2011-10-11 09:07:08.069166743 +0100
@@ -545,7 +545,7 @@  set_columns_for_ps (partial_schedule_ptr
    The move is part of a chain that satisfies register dependencies
    between a producing ddg node and various consuming ddg nodes.
    If some of these dependencies have a distance of 1 (meaning that
-   the use is upward-exposoed) then DISTANCE1_USES is nonnull and
+   the use is upward-exposed) then DISTANCE1_USES is nonnull and
    contains the set of uses with distance-1 dependencies.
    DISTANCE1_USES is null otherwise.
 
@@ -1810,7 +1810,11 @@  sms_schedule (void)
    41. endif
    42. compute epilogue & prologue
    43. finish - succeeded to schedule
-*/
+
+   ??? The algorithm restricts the scheduling window to II cycles.
+   In rare cases, it may be better to allow windows of II+1 cycles.
+   The window would then start and end on the same row, but with
+   different "must precede" and "must follow" requirements.  */
 
 /* A limit on the number of cycles that resource conflicts can span.  ??? Should
    be provided by DFA, and be dependent on the type of insn scheduled.  Currently