On 10/18/2010 03:41 PM, Andrey Belevantsev wrote:
On 18.10.2010 11:31, Jie Zhang wrote:
Hi Andrey,

On 10/18/2010 03:13 PM, Andrey Belevantsev wrote:
Hi Jie,

On 18.10.2010 10:49, Jie Zhang wrote:

When this error happens, FENCE_ISSUED_INSNS (fence) is 2 and
issue_rate is
1. PowerPC 8540 is capable to issue 2 instructions in one cycle, but
rs6000_issue_rate lies to scheduler that it can only issue 1
instruction
before register relocation is done. See the following code:

See PR 45352. I've tried to fix this in the selective scheduler by
modeling the lying behavior in line with the haifa scheduler. Let me
know if the last patch from the PR audit trail doesn't work for you.

In addition, after the above patch goes in, I can make the selective
scheduler not try to jump through the hoops with putting correct sched
cycles on insns for targets which don't need it in their target_finish
hook. I guess powerpc needs this though, but x86-64 (for which PR 45342
was opened) almost surely does not.

Thanks for your reply. I just tried. That patch does not help for this
issue.
I see, I didn't touch the failing assert with the patch. Can you just
remove the assert and see if that helps for you? I cannot think of how
it can be relaxed and still be useful.

Removing the failing assert fixes the test case. But I wonder why not just get max_issue correct. I'm testing the attached patch. IMHO, max_issue looks confusing.

* The concept of ISSUE POINT has never been used since the code landed in repository.

* In the comment just before the function, it's mentioned that MAX_POINTS is the sum of points of all instructions in READY. But it does not match the code. The code only summarizes the points of the first MORE_ISSUE instructions. If later ISSUE_POINTS become not uniform, that piece of code should be redesigned.

So I think it's good to remove it now. And "top - choice_stack" is a good replacement for top->n. So we can remove field n from struct choice_entry, too.

Now I'm looking at MIPS target to find out why this change in the would cause PR37360.

/* ??? We used to assert here that we never issue more insns than issue_rate. However, some targets (e.g. MIPS/SB1) claim lower issue rate than can be achieved to get better performance. Until these targets are fixed to use scheduler hooks to manipulate insns priority instead, the assert should
-     be disabled.
-
-     gcc_assert (more_issue >= 0);  */
+     be disabled.  */


--
Jie Zhang
CodeSourcery
	* haifa-sched.c (ISSUE_POINTS): Remove.
	(struct choice_entry): Remove field n.
	(max_issue): Don't issue more than issue_rate instructions.

Index: haifa-sched.c
===================================================================
--- haifa-sched.c	(revision 165642)
+++ 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;
@@ -2401,8 +2397,6 @@ struct choice_entry
   int index;
   /* The number of the rest insns whose issues we should try.  */
   int rest;
-  /* The number of issued essential insns.  */
-  int n;
   /* State after issuing the insn.  */
   state_t state;
 };
@@ -2444,8 +2438,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 +2451,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 i, all, n_ready, best, delay, tries_num;
   int more_issue;
   struct choice_entry *top;
   rtx insn;
@@ -2477,25 +2470,15 @@ max_issue (struct ready_list *ready, int
     }
 
   /* 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.
      However, some targets (e.g. MIPS/SB1) claim lower issue rate than can be
      achieved to get better performance.  Until these targets are fixed to use
      scheduler hooks to manipulate insns priority instead, the assert should
-     be disabled.
-
-     gcc_assert (more_issue >= 0);  */
+     be disabled.  */
 
-  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;
-      }
+  gcc_assert (more_issue >= 0);
 
   /* The number of the issued insns in the best solution.  */
   best = 0;
@@ -2505,7 +2488,6 @@ max_issue (struct ready_list *ready, int
   /* Set initial state of the search.  */
   memcpy (top->state, state, dfa_state_size);
   top->rest = dfa_lookahead;
-  top->n = 0;
 
   /* Count the number of the insns to search among.  */
   for (all = i = 0; i < n_ready; i++)
@@ -2521,16 +2503,23 @@ max_issue (struct ready_list *ready, int
 	     been asked...  */
 	  top->rest == 0
 	  /* Or have nothing else to try.  */
-	  || i >= n_ready)
+	  || i >= n_ready
+	  /* Or can't issue more.  */
+	  || top - choice_stack >= more_issue)
 	{
 	  /* ??? (... || i == n_ready).  */
 	  gcc_assert (i <= n_ready);
 
+	  /* We should not issue more than issue_rate instructions.  */
+	  gcc_assert (top - choice_stack <= more_issue);
+
 	  if (top == choice_stack)
 	    break;
 
 	  if (best < top - choice_stack)
 	    {
+	      int n;
+
 	      if (privileged_n)
 		{
 		  n = privileged_n;
@@ -2548,7 +2537,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 - choice_stack == more_issue || best == all)
 		    break;
 		}
 	    }
@@ -2579,16 +2568,11 @@ max_issue (struct ready_list *ready, int
 	      else
 		top->rest--;
 
-	      n = top->n;
-	      if (memcmp (top->state, state, dfa_state_size) != 0)
-		n += ISSUE_POINTS (insn);
-
 	      /* Advance to the next choice_entry.  */
 	      top++;
 	      /* Initialize it.  */
 	      top->rest = dfa_lookahead;
 	      top->index = i;
-	      top->n = n;
 	      memcpy (top->state, state, dfa_state_size);
 
 	      ready_try [i] = 1;

Reply via email to