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

[David Ung <davidu@mips.com>]

Richard Sandiford wrote:
> David Ung <davidu@mips.com> writes:
>> Richard Sandiford wrote:
>>> David Ung <davidu@mips.com> writes:
>>>> 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.
>>> But the problem with that is that the priority sticks.  If we change
>>> the priority of a load L1 (say) from A to B, the scheduler may later
>>> insert a newly-ready load L2 with a priority between A and B.
>>> We would then wrongly treat L1 as having a higher priority than L2.
>> The priority sticking is not really a problem, because if there are
>> insn in the ready queue, it mean that they are good to go at the
>> current cycle.  A newly-ready load L2 you talked about is not going to
>> happen during the current cycle, and the now-modified load/store would
>> have been issued.
> 
> It means that they are "good to go" from a data-dependence point of view,
> but it doesn't mean that one load or store will issue this cycle.
> Consider the case where a conditional move has been issued on the
> previous cycle, and unit restrictions mean no agen insn can be issued
> on this one.  A new, higher-priority load may then become ready on
> the next cycle.
> 
> A similar situation would occur if we're in the wake of a "multi" insn.

I don't think its going to matter that much in the end since those are rough 
estimates and the load latencies are already 3+ (more for cache-miss).  The big 
killer is that 74k is out-of-order, so there's no guarantee what order the loads 
actually get issue at.  The bundling of loads helps abit though.
But I am fine for the idea of defining TARGET_SCHED_REORDER2, my concern at the 
time was compile speed and having to deal with 2 reorder functions for changes.

As a side note, I had some concern of issuing the following sequence in the same 
cycle

addu $2, $3, $4
lw   $3, foo

here the load is added to the ready queue after the final use from the addu. 
This is not so good for the pipe since the AGEN (for loads) selects its operands 
earlier than the ALU, so the load is blocked potentially.  The original patch 
accidentally happens to handle some of these cases (it won't handle load L2 
having higher priority than L1 of course).  Anyway, I think a better strategy is 
needed regardless, may be something like adding REG_DEP_ANTI costs in 
mips_adjust_cost.

> 
>>> I think defining TARGET_SCHED_REORDER2 is the right thing.  Let's
>>> move the "cycle == 0" stuff in mips_sched_reorg into Sandra's new
>>> mips_sched_init function.  I think mips_sched_reorder will then be
>>> suitable for both TARGET_SCHED_REORDER and TARGET_SCHED_REORDER2.
>> I have no objection to doing it again in TARGET_SCHED_REORDER2.  The
>> only worry is that TARGET_SCHED_REORDER2 will get called multiple
>> times in the same cycle (upto 3 for 74k?)!!
> 
> True, but remember that we're doing an O(n log n) sort each time
> we call one of these hooks.  A linear walk isn't much overhead
> in comparison.
> 

I am ok if you think its fine.

David.