diff mbox

Make max_issue honor issue_rate

Message ID 4CC71394.1080001@codesourcery.com
State New
Headers show

Commit Message

Jie Zhang Oct. 26, 2010, 5:44 p.m. UTC 
On 10/26/2010 05:58 AM, Maxim Kuvyrkov wrote:
> On 10/25/10 2:36 PM, Jie Zhang wrote:
>
>> 4 units: a, b, c, d.
>>
>> insn 1 uses a, insn 2 uses b, insn 3 uses c, insn 4 uses d.
>>
>> and we set issue_rate to 3. In this case, we can only issue insn 1, 2, 3
>> but not 4 although its unit is free.
>>
>> To extend this example a little, i.e. insn4 uses nothing, (it does not
>> change the state, right? I'm not very sure), it still can not be issued.
>>
>> So I think we should use "top - choice_stack >= more_issue" instead of
>> "top->n >= more_issue". Am I right?
>
> I think the 'n' field was introduced for a reason. DFA description has
> the "nothing" reservation that specifies that a particular instruction
> does not have a noticeable effect on a CPU (imagine a nop instruction
> that gets optimized out by decoder and does not get into the pipeline).
>
OK. This version of the patch removes some unnecessary changes in your 
version to ease the scheduler maintainer review.

Comments

Vladimir Makarov Oct. 27, 2010, 1:14 p.m. UTC | #1 
On 10/26/2010 01:44 PM, Jie Zhang wrote:
> On 10/26/2010 05:58 AM, Maxim Kuvyrkov wrote:
>> On 10/25/10 2:36 PM, Jie Zhang wrote:
>>
>>> 4 units: a, b, c, d.
>>>
>>> insn 1 uses a, insn 2 uses b, insn 3 uses c, insn 4 uses d.
>>>
>>> and we set issue_rate to 3. In this case, we can only issue insn 1, 
>>> 2, 3
>>> but not 4 although its unit is free.
>>>
>>> To extend this example a little, i.e. insn4 uses nothing, (it does not
>>> change the state, right? I'm not very sure), it still can not be 
>>> issued.
>>>
>>> So I think we should use "top - choice_stack >= more_issue" instead of
>>> "top->n >= more_issue". Am I right?
>>
>> I think the 'n' field was introduced for a reason. DFA description has
>> the "nothing" reservation that specifies that a particular instruction
>> does not have a noticeable effect on a CPU (imagine a nop instruction
>> that gets optimized out by decoder and does not get into the pipeline).
>>
> OK. This version of the patch removes some unnecessary changes in your 
> version to ease the scheduler maintainer review.
>
This version of the patch is ok for me.  You can commit it into the trunk.

Thanks for the patch.
Jie Zhang Oct. 27, 2010, 2:31 p.m. UTC | #2 
On 10/27/2010 09:14 PM, Vladimir Makarov wrote:
> On 10/26/2010 01:44 PM, Jie Zhang wrote:
>> On 10/26/2010 05:58 AM, Maxim Kuvyrkov wrote:
>>> On 10/25/10 2:36 PM, Jie Zhang wrote:
>>>
>>>> 4 units: a, b, c, d.
>>>>
>>>> insn 1 uses a, insn 2 uses b, insn 3 uses c, insn 4 uses d.
>>>>
>>>> and we set issue_rate to 3. In this case, we can only issue insn 1,
>>>> 2, 3
>>>> but not 4 although its unit is free.
>>>>
>>>> To extend this example a little, i.e. insn4 uses nothing, (it does not
>>>> change the state, right? I'm not very sure), it still can not be
>>>> issued.
>>>>
>>>> So I think we should use "top - choice_stack >= more_issue" instead of
>>>> "top->n >= more_issue". Am I right?
>>>
>>> I think the 'n' field was introduced for a reason. DFA description has
>>> the "nothing" reservation that specifies that a particular instruction
>>> does not have a noticeable effect on a CPU (imagine a nop instruction
>>> that gets optimized out by decoder and does not get into the pipeline).
>>>
>> OK. This version of the patch removes some unnecessary changes in your
>> version to ease the scheduler maintainer review.
>>
> This version of the patch is ok for me. You can commit it into the trunk.
>
Committed on trunk. Thanks!
diff mbox

Patch


	* haifa-sched.c (ISSUE_POINTS): Remove.
	(max_issue): Don't issue more than issue_rate instructions.

Index: haifa-sched.c
===================================================================
--- haifa-sched.c	(revision 165970)
+++ haifa-sched.c	(working copy)
@@ -199,10 +199,6 @@  struct common_sched_info_def *common_sch
 /* The minimal value of the INSN_TICK of an instruction.  */
 #define MIN_TICK (-max_insn_queue_index)
 
-/* Issue points are used to distinguish between instructions in max_issue ().
-   For now, all instructions are equally good.  */
-#define ISSUE_POINTS(INSN) 1
-
 /* List of important notes we must keep around.  This is a pointer to the
    last element in the list.  */
 rtx note_list;
@@ -2444,8 +2440,7 @@  static int cached_issue_rate = 0;
    insns is insns with the best rank (the first insn in READY).  To
    make this function tries different samples of ready insns.  READY
    is current queue `ready'.  Global array READY_TRY reflects what
-   insns are already issued in this try.  MAX_POINTS is the sum of points
-   of all instructions in READY.  The function stops immediately,
+   insns are already issued in this try.  The function stops immediately,
    if it reached the such a solution, that all instruction can be issued.
    INDEX will contain index of the best insn in READY.  The following
    function is used only for first cycle multipass scheduling.
@@ -2458,7 +2453,7 @@  int
 max_issue (struct ready_list *ready, int privileged_n, state_t state,
 	   int *index)
 {
-  int n, i, all, n_ready, best, delay, tries_num, max_points;
+  int n, i, all, n_ready, best, delay, tries_num;
   int more_issue;
   struct choice_entry *top;
   rtx insn;
@@ -2477,19 +2472,9 @@  max_issue (struct ready_list *ready, int
     }
 
   /* Init max_points.  */
-  max_points = 0;
   more_issue = issue_rate - cycle_issued_insns;
   gcc_assert (more_issue >= 0);
 
-  for (i = 0; i < n_ready; i++)
-    if (!ready_try [i])
-      {
-	if (more_issue-- > 0)
-	  max_points += ISSUE_POINTS (ready_element (ready, i));
-	else
-	  break;
-      }
-
   /* The number of the issued insns in the best solution.  */
   best = 0;
 
@@ -2513,12 +2498,17 @@  max_issue (struct ready_list *ready, int
       if (/* If we've reached a dead end or searched enough of what we have
 	     been asked...  */
 	  top->rest == 0
-	  /* Or have nothing else to try.  */
-	  || i >= n_ready)
+	  /* or have nothing else to try...  */
+	  || i >= n_ready
+	  /* or should not issue more.  */
+	  || top->n >= more_issue)
 	{
 	  /* ??? (... || i == n_ready).  */
 	  gcc_assert (i <= n_ready);
 
+	  /* We should not issue more than issue_rate instructions.  */
+	  gcc_assert (top->n <= more_issue);
+
 	  if (top == choice_stack)
 	    break;
 
@@ -2541,7 +2531,7 @@  max_issue (struct ready_list *ready, int
 		  /* This is the index of the insn issued first in this
 		     solution.  */
 		  *index = choice_stack [1].index;
-		  if (top->n == max_points || best == all)
+		  if (top->n == more_issue || best == all)
 		    break;
 		}
 	    }
@@ -2574,7 +2564,7 @@  max_issue (struct ready_list *ready, int
 
 	      n = top->n;
 	      if (memcmp (top->state, state, dfa_state_size) != 0)
-		n += ISSUE_POINTS (insn);
+		n++;
 
 	      /* Advance to the next choice_entry.  */
 	      top++;