Re: [PATCH] Reset insn priority after inc/ref replacement in haifa sched

[Robin Dapp]

Hi,

> See my last message.  I find myself wondering if we need to reset
> INSN_PRIORITY_STATUS in update_insn_after_change and/or calling
> update_insn_after_change on INSN in additional to calling it on DESC->insn.

I tried calling update_insn_after_change even before sending my message
but it seems to modify variables that are assumed to be set at this
stage of the algorithm (resetting INSN_TICK (insn) and INSN_COST (insn)
causes ICEs in other places).  It suffices, however, to call priority
like in the attached patch to achieve the same result.

Test suite is still running.

Regards
 Robin

gcc/ChangeLog:

2018-10-15  Robin Dapp  <rd...@linux.ibm.com>

        * haifa-sched.c (priority): Add force_recompute parameter.
        (apply_replacement):
        Call priority () with force_recompute = true.
        (restore_pattern): Likewise.

diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 1fdc9df9fb2..8aab37a2ba8 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -1590,7 +1590,7 @@ bool sched_fusion;
 
 /* Compute the priority number for INSN.  */
 static int
-priority (rtx_insn *insn)
+priority (rtx_insn *insn, bool force_recompute)
 {
   if (! INSN_P (insn))
     return 0;
@@ -1598,7 +1598,7 @@ priority (rtx_insn *insn)
   /* We should not be interested in priority of an already scheduled insn.  */
   gcc_assert (QUEUE_INDEX (insn) != QUEUE_SCHEDULED);
 
-  if (!INSN_PRIORITY_KNOWN (insn))
+  if (force_recompute || !INSN_PRIORITY_KNOWN (insn))
     {
       int this_priority = -1;
 
@@ -1695,6 +1695,12 @@ priority (rtx_insn *insn)
 
   return INSN_PRIORITY (insn);
 }
+
+static int
+priority (rtx_insn *insn)
+{
+  return priority (insn, false);
+}
 
 /* Macros and functions for keeping the priority queue sorted, and
    dealing with queuing and dequeuing of instructions.  */
@@ -4713,7 +4719,12 @@ apply_replacement (dep_t dep, bool immediately)
       success = validate_change (desc->insn, desc->loc, desc->newval, 0);
       gcc_assert (success);
 
+      rtx_insn *insn = DEP_PRO (dep);
+
+      /* Recompute priority since dependent priorities have changed.  */
+      priority (insn, true);
       update_insn_after_change (desc->insn);
+
       if ((TODO_SPEC (desc->insn) & (HARD_DEP | DEP_POSTPONED)) == 0)
 	fix_tick_ready (desc->insn);
 
@@ -4767,7 +4778,13 @@ restore_pattern (dep_t dep, bool immediately)
 
       success = validate_change (desc->insn, desc->loc, desc->orig, 0);
       gcc_assert (success);
+
+      rtx_insn *insn = DEP_PRO (dep);
+      /* Recompute priority since dependent priorities have changed.  */
+      priority (insn, true);
+
       update_insn_after_change (desc->insn);
+
       if (backtrack_queue != NULL)
 	{
 	  backtrack_queue->replacement_deps.safe_push (dep);