Hi!

Current modulo-sched implementation is a bit faulty from -fcompile-debug 
perspective.

I found that few years ago, the most simple example is pr42631.c which fails 
(with just -fmodulo-sched or together with -fmodulo-sched-allow-regmoves) on 
powerpc64le with at least any gcc-4.9 or newer compiler.
I've investigated that difference about 3 years ago, it is mostly technical, 
dumps shows there are some "flying" NOTE_INSN_DELETED items.
I understood that it is minor, and I planned to commit the fix only when my 
other modulo-sched stuff will be ready.

But right now I see that when I enable -fmodulo-sched by default, powerpc64le 
bootstrap give comparison failure as of r10-7056.

Comparing stages 2 and 3
Bootstrap comparison failure!
gcc/ggc-page.o differs

That doesn't happen on released branches, so it is a kind of "regression" 
(certainly, nobody runs bootstrap with -fmodulo-sched).

Is that a good reason to commit the patch right now in stage4?

Patch was successfully regstrapped (based on r10-7056) using x86_64 and 
powerpc64le, both with default options and with -fmodulo-sched enabled.

Roman

--
modulo-sched: fix compare-debug issues
    
This patch fixes bootstrap comparison failure on powerpc64le when running it
with -fmodulo-sched enabled.
    
When applying the schedule we have to move debug insns in the same
way as we move note insns.  Also we have to discard adding debug insns
to SCCs in DDG graph.
    
20YY-MM-DD  Roman Zhuykov  <zhr...@ispras.ru>
   
        * ddg.c (create_ddg_dep_from_intra_loop_link): Adjust assertions.
        (create_ddg_dep_no_link): Likewise.
        (add_inter_loop_mem_dep): Do not create "debug --> non-debug" anti-deps.
        (create_ddg): Adjust first_note field filling.
        (check_sccs): Assert if any debug instruction is in SCC.
        * modulo-sched.c (ps_first_note): Add an assertion if first_note
        is empty.
    
testsuite:
    
20YY-MM-DD  Roman Zhuykov  <zhr...@ispras.ru>
    
        * gcc.dg/pr42631-sms1.c: New test.
        * gcc.dg/pr42631-sms2.c: New test.

diff --git a/gcc/ddg.c b/gcc/ddg.c
index ca8cb74823..048207a354 100644
--- a/gcc/ddg.c
+++ b/gcc/ddg.c
@@ -185,8 +185,8 @@ create_ddg_dep_from_intra_loop_link (ddg_ptr g, 
ddg_node_ptr src_node,
   else if (DEP_TYPE (link) == REG_DEP_OUTPUT)
     t = OUTPUT_DEP;
 
-  gcc_assert (!DEBUG_INSN_P (dest_node->insn) || t == ANTI_DEP);
-  gcc_assert (!DEBUG_INSN_P (src_node->insn) || t == ANTI_DEP);
+  gcc_assert (NONDEBUG_INSN_P (dest_node->insn) || t == ANTI_DEP);
+  gcc_assert (NONDEBUG_INSN_P (src_node->insn) || DEBUG_INSN_P 
(dest_node->insn));
 
   /* We currently choose not to create certain anti-deps edges and
      compensate for that by generating reg-moves based on the life-range
@@ -222,9 +222,9 @@ create_ddg_dep_from_intra_loop_link (ddg_ptr g, 
ddg_node_ptr src_node,
         }
     }
 
-   latency = dep_cost (link);
-   e = create_ddg_edge (src_node, dest_node, t, dt, latency, distance);
-   add_edge_to_ddg (g, e);
+  latency = dep_cost (link);
+  e = create_ddg_edge (src_node, dest_node, t, dt, latency, distance);
+  add_edge_to_ddg (g, e);
 }
 
 /* The same as the above function, but it doesn't require a link parameter.  */
@@ -237,8 +237,8 @@ create_ddg_dep_no_link (ddg_ptr g, ddg_node_ptr from, 
ddg_node_ptr to,
   enum reg_note dep_kind;
   struct _dep _dep, *dep = &_dep;
 
-  gcc_assert (!DEBUG_INSN_P (to->insn) || d_t == ANTI_DEP);
-  gcc_assert (!DEBUG_INSN_P (from->insn) || d_t == ANTI_DEP);
+  gcc_assert (NONDEBUG_INSN_P (to->insn) || d_t == ANTI_DEP);
+  gcc_assert (NONDEBUG_INSN_P (from->insn) || DEBUG_INSN_P (to->insn));
 
   if (d_t == ANTI_DEP)
     dep_kind = REG_DEP_ANTI;
@@ -455,8 +455,12 @@ add_inter_loop_mem_dep (ddg_ptr g, ddg_node_ptr from, 
ddg_node_ptr to)
        return;
       else if (from->cuid != to->cuid)
        {
-         create_ddg_dep_no_link (g, from, to, ANTI_DEP, MEM_DEP, 1);
-         if (DEBUG_INSN_P (from->insn) || DEBUG_INSN_P (to->insn))
+         gcc_assert (NONDEBUG_INSN_P (to->insn));
+
+         if (NONDEBUG_INSN_P (from->insn))
+           create_ddg_dep_no_link (g, from, to, ANTI_DEP, MEM_DEP, 1);
+
+         if (DEBUG_INSN_P (from->insn))
            create_ddg_dep_no_link (g, to, from, ANTI_DEP, MEM_DEP, 1);
          else
            create_ddg_dep_no_link (g, to, from, TRUE_DEP, MEM_DEP, 1);
@@ -607,28 +611,34 @@ create_ddg (basic_block bb, int closing_branch_deps)
       if (! INSN_P (insn))
        {
          if (! first_note && NOTE_P (insn)
-             && NOTE_KIND (insn) !=  NOTE_INSN_BASIC_BLOCK)
+             && NOTE_KIND (insn) != NOTE_INSN_BASIC_BLOCK)
            first_note = insn;
          continue;
        }
+
       if (JUMP_P (insn))
        {
          gcc_assert (!g->closing_branch);
          g->closing_branch = &g->nodes[i];
        }
-      else if (GET_CODE (PATTERN (insn)) == USE)
-       {
-         if (! first_note)
-           first_note = insn;
-         continue;
-       }
+
+      if (! first_note)
+       first_note = insn;
+
+      if (GET_CODE (PATTERN (insn)) == USE)
+       continue;
 
       g->nodes[i].cuid = i;
       g->nodes[i].successors = sbitmap_alloc (num_nodes);
       bitmap_clear (g->nodes[i].successors);
       g->nodes[i].predecessors = sbitmap_alloc (num_nodes);
       bitmap_clear (g->nodes[i].predecessors);
-      g->nodes[i].first_note = (first_note ? first_note : insn);
+
+      if (! DEBUG_INSN_P (insn))
+       {
+         g->nodes[i].first_note = first_note;
+         first_note = NULL;
+       }
 
       g->nodes[i].aux.count = -1;
       g->nodes[i].max_dist = XCNEWVEC (int, num_nodes);
@@ -636,7 +646,6 @@ create_ddg (basic_block bb, int closing_branch_deps)
        g->nodes[i].max_dist[j] = -1;
 
       g->nodes[i++].insn = insn;
-      first_note = NULL;
     }
 
   /* We must have found a branch in DDG.  */
@@ -1002,13 +1011,19 @@ order_sccs (ddg_all_sccs_ptr g)
 static void
 check_sccs (ddg_all_sccs_ptr sccs, int num_nodes)
 {
-  int i = 0;
+  int i;
+  unsigned int j;
   auto_sbitmap tmp (num_nodes);
 
   bitmap_clear (tmp);
   for (i = 0; i < sccs->num_sccs; i++)
     {
+      sbitmap_iterator sbi;
       gcc_assert (!bitmap_empty_p (sccs->sccs[i]->nodes));
+      /* Debug insns should not be in found SCCs.  */
+      EXECUTE_IF_SET_IN_BITMAP (sccs->sccs[i]->nodes, 0, j, sbi)
+       gcc_assert (NONDEBUG_INSN_P (sccs->ddg->nodes[j].insn));
+
       /* Verify that every node in sccs is in exactly one strongly
          connected component.  */
       gcc_assert (!bitmap_intersect_p (tmp, sccs->sccs[i]->nodes));
diff --git a/gcc/modulo-sched.c b/gcc/modulo-sched.c
index e16e023243..3f6aa8c7c2 100644
--- a/gcc/modulo-sched.c
+++ b/gcc/modulo-sched.c
@@ -318,6 +318,7 @@ static rtx_insn *
 ps_first_note (partial_schedule_ptr ps, int id)
 {
   gcc_assert (id < ps->g->num_nodes);
+  gcc_assert (ps->g->nodes[id].first_note);
   return ps->g->nodes[id].first_note;
 }
 
diff --git a/gcc/testsuite/gcc.dg/pr42631-sms1.c 
b/gcc/testsuite/gcc.dg/pr42631-sms1.c
new file mode 100644
index 0000000000..0117276c73
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr42631-sms1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-g -O1 -funroll-loops -fmodulo-sched -fcompare-debug" } */
+/* { dg-xfail-if "" { powerpc-ibm-aix* } } */
+
+void foo()
+{
+  unsigned k;
+  while (--k > 0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr42631-sms2.c 
b/gcc/testsuite/gcc.dg/pr42631-sms2.c
new file mode 100644
index 0000000000..2555da4522
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr42631-sms2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-g -O1 -funroll-loops -fmodulo-sched 
-fmodulo-sched-allow-regmoves -fcompare-debug" } */
+/* { dg-xfail-if "" { powerpc-ibm-aix* } } */
+
+void foo()
+{
+  unsigned k;
+  while (--k > 0);
+}

Reply via email to