Re: PATCH: MIPS 74K load/store scheduling tweak (take 2)

[David Ung <davidu@mips.com>]

Richard Sandiford wrote:
> Sandra Loosemore <sandra@codesourcery.com> writes:
>> + /* Conditionally swap the instructions at POS1 and POS2 in ready queue
>> +    READY, also adjusting the priority of the instruction formerly at
>> +    POS1 when we do so.  */
>> + 
>> + static void
>> + mips_maybe_swap_ready (rtx *ready, int pos1, int pos2)
>> + {
>> +   if (pos1 < pos2
>> +       && INSN_PRIORITY (ready[pos1]) + 4 >= INSN_PRIORITY (ready[pos2]))
>> +     {
>> +       rtx temp;
>> +       INSN_PRIORITY (ready[pos1]) = INSN_PRIORITY (ready[pos2]);
>> +       temp = ready[pos1];
>> +       ready[pos1] = ready[pos2];
>> +       ready[pos2] = temp;
>> +     }
>> + }
> 
> To be a general function rather than a 74k function, the magic value
> 4 should be a parameter too.  The comment seems a bit vague: how about
> "Make sure the instruction at POS1 in ready queue READY is ahead of
> the instruction at POS2, but only if its priority is no less than
> LIMIT units of the other instruction's priority.  Assume that
> only one of the instructions may issue this cycle."  Copy-edit
> as necessary.
> 
> It isn't obvious without the last bit why you're only swapping,
> rather than inserting POS1 directly ahead of POS2.  With the
> comment there, you can remove:
> 
> +   /* At this point the ready queue may no longer be sorted, but that's
> +      OK since 74k can't schedule concurrent load/store on the same
> +      cycle.  */
> 
> I'm uncertain whether setting INSN_PRIORITY is really the
> right thing to do here.  David, why isn't the sorting done by
> mips_sched_reorder enough?
> 

This is the multi-issue problem.  ready_sort is called more than once during 
each cycle.  The 1st instruction chosen during that cycle may not be an AGEN one 
(eg a floating point insn or an ALU insn), which means mips_sched_reorder2 needs 
to be implemented.  So instead of doing the same thing in mips_sched_reorder2, 
just changing the INSN_PRIORITY gets the same effect done automatically by 
ready_sort.

David.