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

[David Ung <davidu at mips dot 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.