Patchwork Make max_issue honor issue_rate

login
register
mail settings
Submitter Maxim Kuvyrkov
Date Oct. 25, 2010, 6:13 p.m.
Message ID <4CC5C8E4.9050205@codesourcery.com>
Download mbox | patch
Permalink /patch/69176/
State New
Headers show

Comments

Maxim Kuvyrkov - Oct. 25, 2010, 6:13 p.m.
On 10/22/10 2:35 PM, Jie Zhang wrote:

> 	* haifa-sched.c (ISSUE_POINTS): Remove.

Removing ISSUE_POINTS is the right thing to do, thanks for doing this.

> 	(struct choice_entry): Remove field n.

I believe the choice_entry->n field is still useful.  It can be used to 
handle incomplete pipeline descriptions for which
gcc_assert (memcmp (top->state, state, dfa_state_size) != 0);
would be false.  Or to avoid counting pseudo instructions that don't 
affect CPU state.

> 	(max_issue): Don't issue more than issue_rate instructions.

This also makes sense to me.

Attached is a version of your patch that keeps choice_entry->n field.

The patch looks very simple to me (it mostly expands the definition of 
ISSUE_POINTS to '1'), but it still requires a formal review by a 
scheduler maintainer.

Thank you,

Patch

diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 5b5459f..2af3da7 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -199,10 +199,6 @@  struct common_sched_info_def *common_sched_info;
 /* 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;
@@ -2401,7 +2397,8 @@  struct choice_entry
   int index;
   /* The number of the rest insns whose issues we should try.  */
   int rest;
-  /* The number of issued essential insns.  */
+  /* The number of issued essential insns, e.i, insns that change state
+     of DFA.  */
   int n;
   /* State after issuing the insn.  */
   state_t state;
@@ -2444,8 +2441,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 +2454,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,7 +2473,6 @@  max_issue (struct ready_list *ready, int privileged_n, state_t state,
     }
 
   /* Init max_points.  */
-  max_points = 0;
   more_issue = issue_rate - cycle_issued_insns;
 
   /* ??? We used to assert here that we never issue more insns than issue_rate.
@@ -2488,15 +2483,6 @@  max_issue (struct ready_list *ready, int privileged_n, state_t state,
 
      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;
 
@@ -2520,11 +2506,15 @@  max_issue (struct ready_list *ready, int privileged_n, state_t state,
       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)
+	/* ... then check if the current state is an acceptable solution.  */
 	{
-	  /* ??? (... || 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;
@@ -2548,7 +2538,7 @@  max_issue (struct ready_list *ready, int privileged_n, state_t state,
 		  /* 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;
 		}
 	    }
@@ -2581,7 +2571,7 @@  max_issue (struct ready_list *ready, int privileged_n, state_t state,
 
 	      n = top->n;
 	      if (memcmp (top->state, state, dfa_state_size) != 0)
-		n += ISSUE_POINTS (insn);
+		n++;
 
 	      /* Advance to the next choice_entry.  */
 	      top++;