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

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

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

The MIPS 74K can't issue loads when stores are in flight.  This patch tweaks the 
instruction scheduling to swap things around in the ready queue to group loads 
together and stores together, instead of interleaving them.

OK to commit?

-Sandra

[-- Attachment #2: 26a-74k-sched-ldst.log --]
[-- Type: text/x-log, Size: 458 bytes --]

2007-08-03  Sandra Loosemore  <sandra@codesourcery.com>
	    David Ung  <davidu@mips.com>

        gcc/
	* config/mips/mips.c (TARGET_SCHED_INIT): Define.
	(mips_maybe_swap_ready): New.
	(mips_last_74k_agen_insn): New.
	(mips_74k_agen_init): New.
	(mips_74k_agen_reorder): New function to group loads and stores
	in the ready queue.
	(mips_sched_init): New.
	(mips_sched_reorder): Call mips_74k_agen_reorder.
	(mips_variable_issue): Call mips_74k_agen_init.

[-- Attachment #3: 26a-74k-sched-ldst.patch --]
[-- Type: text/x-patch, Size: 4646 bytes --]

Index: gcc/config/mips/mips.c
===================================================================
*** gcc/config/mips/mips.c	(revision 127140)
--- gcc/config/mips/mips.c	(working copy)
*************** static bool vr4130_true_reg_dependence_p
*** 393,398 ****
--- 393,399 ----
  static bool vr4130_swap_insns_p (rtx, rtx);
  static void vr4130_reorder (rtx *, int);
  static void mips_promote_ready (rtx *, int, int);
+ static void mips_sched_init (FILE *, int, int);
  static int mips_sched_reorder (FILE *, int, rtx *, int *, int);
  static int mips_variable_issue (FILE *, int, rtx, int);
  static int mips_adjust_cost (rtx, rtx, rtx, int);
*************** static const unsigned char mips16e_save_
*** 1225,1230 ****
--- 1226,1233 ----
  #undef TARGET_ASM_FUNCTION_RODATA_SECTION
  #define TARGET_ASM_FUNCTION_RODATA_SECTION mips_function_rodata_section
  
+ #undef TARGET_SCHED_INIT
+ #define TARGET_SCHED_INIT mips_sched_init
  #undef TARGET_SCHED_REORDER
  #define TARGET_SCHED_REORDER mips_sched_reorder
  #undef TARGET_SCHED_VARIABLE_ISSUE
*************** mips_promote_ready (rtx *ready, int lowe
*** 10881,10886 ****
--- 10884,10994 ----
    ready[i] = new_head;
  }
  
+ /* 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;
+     }
+ }
+ 
+ /* Record whether last 74k AGEN instruction was a load or store.  */
+ 
+ static enum attr_type mips_last_74k_agen_insn = TYPE_UNKNOWN;
+ 
+ /* Initialize mips_last_74k_agen_insn from INSN.  A null argument
+    resets to TYPE_UNKNOWN state.  */
+ 
+ static void
+ mips_74k_agen_init (rtx insn)
+ {
+   if (!insn || !NONJUMP_INSN_P (insn))
+     mips_last_74k_agen_insn = TYPE_UNKNOWN;
+   else if (USEFUL_INSN_P (insn))
+     {
+       enum attr_type type = get_attr_type (insn);
+       if (type == TYPE_LOAD || type == TYPE_STORE)
+ 	mips_last_74k_agen_insn = type;
+     }
+ }
+ 
+ /* A TUNE_74K helper function.  The 74K AGEN pipeline likes multiple
+    loads to be grouped together, and multiple stores to be grouped
+    together.  Swap things around in the ready queue to make this happen.  */
+ 
+ static void
+ mips_74k_agen_reorder (rtx *ready, int nready)
+ {
+   int i;
+   int store_pos, load_pos;
+ 
+   store_pos = -1;
+   load_pos = -1;
+ 
+   for (i = nready - 1; i >= 0; i--)
+     {
+       rtx insn = ready[i];
+       if (USEFUL_INSN_P (insn))
+ 	switch (get_attr_type (insn))
+ 	  {
+ 	  case TYPE_STORE:
+ 	    if (store_pos == -1)
+ 	      store_pos = i;
+ 	    break;
+ 	    
+ 	  case TYPE_LOAD:
+ 	    if (load_pos == -1)
+ 	      load_pos = i;
+ 	    break;
+ 	    
+ 	  default:
+ 	    break;
+ 	  }
+     }
+   
+   if (load_pos == -1 || store_pos == -1)
+     return;
+   
+   switch (mips_last_74k_agen_insn)
+     {
+     case TYPE_UNKNOWN:
+       /* Prefer to schedule loads since they have a higher latency.  */
+     case TYPE_LOAD:
+       /* Swap loads to the front of the queue.  */
+       mips_maybe_swap_ready (ready, load_pos, store_pos);
+       break;
+     case TYPE_STORE:
+       /* Swap stores to the front of the queue.  */
+       mips_maybe_swap_ready (ready, store_pos, load_pos);
+       break;
+     default:
+       break;
+     }
+   /* 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.  */
+ }
+ 
+ /* Implement TARGET_SCHED_INIT.  */
+ 
+ static void
+ mips_sched_init (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
+ 		 int max_ready ATTRIBUTE_UNUSED)
+ {
+   if (TUNE_74K)
+     mips_74k_agen_init (NULL_RTX);
+ }
+ 
  /* Implement TARGET_SCHED_REORDER.  */
  
  static int
*************** mips_sched_reorder (FILE *file ATTRIBUTE
*** 10901,10906 ****
--- 11009,11016 ----
        if (*nreadyp > 1)
  	vr4130_reorder (ready, *nreadyp);
      }
+   if (TUNE_74K)
+     mips_74k_agen_reorder (ready, *nreadyp);
    return mips_issue_rate ();
  }
  
*************** static int
*** 10910,10915 ****
--- 11020,11027 ----
  mips_variable_issue (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
  		     rtx insn, int more)
  {
+   if (TUNE_74K)
+     mips_74k_agen_init (insn);
    switch (GET_CODE (PATTERN (insn)))
      {
      case USE:

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

[Richard Sandiford]

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?

Looks good otherwise, thanks.

Richard

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

[David Ung]

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.

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

[Richard Sandiford]

David Ung <davidu@mips.com> writes:
> 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.

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.

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.

Richard

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

[David Ung]

Richard Sandiford wrote:
> David Ung <davidu@mips.com> writes:
>> 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.
> 
> 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.

> 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?)!!

David

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

[Richard Sandiford]

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 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.

Richard

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

[David Ung]

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.

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

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

Richard Sandiford wrote:

> 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 think I've got in all the changes from the last round of discussion now.  The 
attached version of the patch tests without regressions both with and without 
-march=74kc.  Are we there yet?  ;-)

-Sandra


[-- Attachment #2: 26b-74k-sched-ldst.log --]
[-- Type: text/x-log, Size: 526 bytes --]

2007-08-10  Sandra Loosemore  <sandra@codesourcery.com>
	    David Ung  <davidu@mips.com>

        gcc/
	* config/mips/mips.c (TARGET_SCHED_INIT): Define.
	(TARGET_SCHED_REORDER2): Define.
	(mips_maybe_swap_ready): New.
	(mips_last_74k_agen_insn): New.
	(mips_74k_agen_init): New.
	(mips_74k_agen_reorder): New function to group loads and stores
	in the ready queue.
	(mips_sched_init): New.  
	(mips_sched_reorder): Don't do initialization here.  Call
	mips_74k_agen_reorder.
	(mips_variable_issue): Call mips_74k_agen_init.

[-- Attachment #3: 26b-74k-sched-ldst.patch --]
[-- Type: text/x-patch, Size: 5739 bytes --]

Index: gcc/config/mips/mips.c
===================================================================
*** gcc/config/mips/mips.c	(revision 127140)
--- gcc/config/mips/mips.c	(working copy)
*************** static bool vr4130_true_reg_dependence_p
*** 393,398 ****
--- 393,399 ----
  static bool vr4130_swap_insns_p (rtx, rtx);
  static void vr4130_reorder (rtx *, int);
  static void mips_promote_ready (rtx *, int, int);
+ static void mips_sched_init (FILE *, int, int);
  static int mips_sched_reorder (FILE *, int, rtx *, int *, int);
  static int mips_variable_issue (FILE *, int, rtx, int);
  static int mips_adjust_cost (rtx, rtx, rtx, int);
*************** static const unsigned char mips16e_save_
*** 1225,1232 ****
--- 1226,1237 ----
  #undef TARGET_ASM_FUNCTION_RODATA_SECTION
  #define TARGET_ASM_FUNCTION_RODATA_SECTION mips_function_rodata_section
  
+ #undef TARGET_SCHED_INIT
+ #define TARGET_SCHED_INIT mips_sched_init
  #undef TARGET_SCHED_REORDER
  #define TARGET_SCHED_REORDER mips_sched_reorder
+ #undef TARGET_SCHED_REORDER2
+ #define TARGET_SCHED_REORDER2 mips_sched_reorder
  #undef TARGET_SCHED_VARIABLE_ISSUE
  #define TARGET_SCHED_VARIABLE_ISSUE mips_variable_issue
  #undef TARGET_SCHED_ADJUST_COST
*************** mips_promote_ready (rtx *ready, int lowe
*** 10881,10906 ****
    ready[i] = new_head;
  }
  
! /* Implement TARGET_SCHED_REORDER.  */
  
! static int
! mips_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
! 		    rtx *ready, int *nreadyp, int cycle)
  {
!   if (!reload_completed && TUNE_MACC_CHAINS)
      {
!       if (cycle == 0)
! 	mips_macc_chains_last_hilo = 0;
!       if (*nreadyp > 0)
! 	mips_macc_chains_reorder (ready, *nreadyp);
      }
!   if (reload_completed && TUNE_MIPS4130 && !TARGET_VR4130_ALIGN)
      {
!       if (cycle == 0)
! 	vr4130_last_insn = 0;
!       if (*nreadyp > 1)
! 	vr4130_reorder (ready, *nreadyp);
      }
    return mips_issue_rate ();
  }
  
--- 10886,11014 ----
    ready[i] = new_head;
  }
  
! /* If the priority of the instruction at POS2 in the ready queue READY 
!    is within LIMIT units of that of the instruction at POS1, swap the 
!    instructions if POS2 is not already less than POS1.  */
  
! static void
! mips_maybe_swap_ready (rtx *ready, int pos1, int pos2, int limit)
  {
!   if (pos1 < pos2
!       && INSN_PRIORITY (ready[pos1]) + limit >= INSN_PRIORITY (ready[pos2]))
      {
!       rtx temp;
!       temp = ready[pos1];
!       ready[pos1] = ready[pos2];
!       ready[pos2] = temp;
      }
! }
! 
! /* Record whether last 74k AGEN instruction was a load or store.  */
! 
! static enum attr_type mips_last_74k_agen_insn = TYPE_UNKNOWN;
! 
! /* Initialize mips_last_74k_agen_insn from INSN.  A null argument
!    resets to TYPE_UNKNOWN state.  */
! 
! static void
! mips_74k_agen_init (rtx insn)
! {
!   if (!insn || !NONJUMP_INSN_P (insn))
!     mips_last_74k_agen_insn = TYPE_UNKNOWN;
!   else if (USEFUL_INSN_P (insn))
      {
!       enum attr_type type = get_attr_type (insn);
!       if (type == TYPE_LOAD || type == TYPE_STORE)
! 	mips_last_74k_agen_insn = type;
      }
+ }
+ 
+ /* A TUNE_74K helper function.  The 74K AGEN pipeline likes multiple
+    loads to be grouped together, and multiple stores to be grouped
+    together.  Swap things around in the ready queue to make this happen.  */
+ 
+ static void
+ mips_74k_agen_reorder (rtx *ready, int nready)
+ {
+   int i;
+   int store_pos, load_pos;
+ 
+   store_pos = -1;
+   load_pos = -1;
+ 
+   for (i = nready - 1; i >= 0; i--)
+     {
+       rtx insn = ready[i];
+       if (USEFUL_INSN_P (insn))
+ 	switch (get_attr_type (insn))
+ 	  {
+ 	  case TYPE_STORE:
+ 	    if (store_pos == -1)
+ 	      store_pos = i;
+ 	    break;
+ 	    
+ 	  case TYPE_LOAD:
+ 	    if (load_pos == -1)
+ 	      load_pos = i;
+ 	    break;
+ 	    
+ 	  default:
+ 	    break;
+ 	  }
+     }
+   
+   if (load_pos == -1 || store_pos == -1)
+     return;
+   
+   switch (mips_last_74k_agen_insn)
+     {
+     case TYPE_UNKNOWN:
+       /* Prefer to schedule loads since they have a higher latency.  */
+     case TYPE_LOAD:
+       /* Swap loads to the front of the queue.  */
+       mips_maybe_swap_ready (ready, load_pos, store_pos, 4);
+       break;
+     case TYPE_STORE:
+       /* Swap stores to the front of the queue.  */
+       mips_maybe_swap_ready (ready, store_pos, load_pos, 4);
+       break;
+     default:
+       break;
+     }
+ }
+ 
+ /* Implement TARGET_SCHED_INIT.  */
+ 
+ static void
+ mips_sched_init (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
+ 		 int max_ready ATTRIBUTE_UNUSED)
+ {
+   if (!reload_completed && TUNE_MACC_CHAINS)
+     mips_macc_chains_last_hilo = 0;
+   if (reload_completed && TUNE_MIPS4130 && !TARGET_VR4130_ALIGN)
+     vr4130_last_insn = 0;
+   if (TUNE_74K)
+     mips_74k_agen_init (NULL_RTX);
+ 
+ }
+ 
+ /* Implement TARGET_SCHED_REORDER and TARG_SCHED_REORDER2.  */
+ 
+ static int
+ mips_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
+ 		    rtx *ready, int *nreadyp, int cycle)
+ {
+   if (!reload_completed
+       && TUNE_MACC_CHAINS
+       && *nreadyp > 0)
+     mips_macc_chains_reorder (ready, *nreadyp);
+   if (reload_completed
+       && TUNE_MIPS4130
+       && !TARGET_VR4130_ALIGN
+       && *nreadyp > 1)
+     vr4130_reorder (ready, *nreadyp);
+   if (TUNE_74K)
+     mips_74k_agen_reorder (ready, *nreadyp);
    return mips_issue_rate ();
  }
  
*************** static int
*** 10910,10915 ****
--- 11018,11025 ----
  mips_variable_issue (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
  		     rtx insn, int more)
  {
+   if (TUNE_74K)
+     mips_74k_agen_init (insn);
    switch (GET_CODE (PATTERN (insn)))
      {
      case USE:

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

[Richard Sandiford]

Sandra Loosemore <sandra@codesourcery.com> writes:
> + /* Implement TARGET_SCHED_INIT.  */
> + 
> + static void
> + mips_sched_init (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
> + 		 int max_ready ATTRIBUTE_UNUSED)
> + {
> +   if (!reload_completed && TUNE_MACC_CHAINS)
> +     mips_macc_chains_last_hilo = 0;
> +   if (reload_completed && TUNE_MIPS4130 && !TARGET_VR4130_ALIGN)
> +     vr4130_last_insn = 0;
> +   if (TUNE_74K)
> +     mips_74k_agen_init (NULL_RTX);
> + 
> + }

Excess blank line before "}".  Might as well do the initialisations
unconditinally; it's one less thing to keep in sync.

OK with that change, thanks.  No need to retest; if it compiles,
it's fine.

Richard

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

[Sandra Loosemore]

Richard Sandiford wrote:
> Sandra Loosemore <sandra@codesourcery.com> writes:
>> + /* Implement TARGET_SCHED_INIT.  */
>> + 
>> + static void
>> + mips_sched_init (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
>> + 		 int max_ready ATTRIBUTE_UNUSED)
>> + {
>> +   if (!reload_completed && TUNE_MACC_CHAINS)
>> +     mips_macc_chains_last_hilo = 0;
>> +   if (reload_completed && TUNE_MIPS4130 && !TARGET_VR4130_ALIGN)
>> +     vr4130_last_insn = 0;
>> +   if (TUNE_74K)
>> +     mips_74k_agen_init (NULL_RTX);
>> + 
>> + }
> 
> Excess blank line before "}".  Might as well do the initialisations
> unconditinally; it's one less thing to keep in sync.
> 
> OK with that change, thanks.  No need to retest; if it compiles,
> it's fine.

OK, committed.  :-)

-Sandra