Hi!

The compare_assert_loc added recently to sort assert exprs that could
operand_equal_p the expressions/values in there unfortunately broke
-fcompare-debug.  The problem is that DECL_UIDs don't have to be the same
between -g and -g0, and thus what iterative_hash_expr returns might not be
the same.  For the removal of duplicate assert exprs or moving assert expr
to the dominator if present on all edges this doesn't matter, because
all we care about there are the adjacent vector entries and code generation
is not affected by the traversal order, but when we actually
process_assert_insertions_for afterwards, the order matters a lot for
code generation (different SSA_NAME_VERSION on the ASSERT_EXPR lhs will
mean different order of release_ssa_name afterwards and might result
in different SSA_NAME versions created later on in other passes and that
in many cases affects code generation.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?  No testcase, as the reduced testcase doesn't reproduce it (at least
for me) and the original is way too large).

2017-07-04  Jakub Jelinek  <ja...@redhat.com>

        PR debug/81278
        * tree-vrp.c (compare_assert_loc): Only test if a->e is NULL,
        !a->e == !b->e has been verified already.  Use e == NULL or
        e != NULL instead of e or ! e tests.
        (compare_assert_loc_stable): New function.
        (process_assert_insertions): Sort using compare_assert_loc_stable
        before calling process_assert_insertions_for in a loop.  Use break
        instead of continue once seen NULL pointer.

--- gcc/tree-vrp.c.jj   2017-07-03 19:03:23.817824263 +0200
+++ gcc/tree-vrp.c      2017-07-04 10:30:36.403204757 +0200
@@ -6400,13 +6400,13 @@ compare_assert_loc (const void *pa, cons
 {
   assert_locus * const a = *(assert_locus * const *)pa;
   assert_locus * const b = *(assert_locus * const *)pb;
-  if (! a->e && b->e)
+  if (a->e == NULL && b->e != NULL)
     return 1;
-  else if (a->e && ! b->e)
+  else if (a->e != NULL && b->e == NULL)
     return -1;
 
   /* Sort after destination index.  */
-  if (! a->e && ! b->e)
+  if (a->e == NULL)
     ;
   else if (a->e->dest->index > b->e->dest->index)
     return 1;
@@ -6423,12 +6423,53 @@ compare_assert_loc (const void *pa, cons
   hashval_t ha = iterative_hash_expr (a->expr, iterative_hash_expr (a->val, 
0));
   hashval_t hb = iterative_hash_expr (b->expr, iterative_hash_expr (b->val, 
0));
   if (ha == hb)
-    return (a->e && b->e
+    return (a->e != NULL
            ? a->e->src->index - b->e->src->index
            : a->bb->index - b->bb->index);
   return ha - hb;
 }
 
+/* Qsort helper for sorting assert locations.  Like the above, but
+   don't use expression hashes for the sorting to make the sorting
+   stable for -fcompare-debug.  Some assert_locus pointers could
+   be NULL, sort those last.  */
+
+static int
+compare_assert_loc_stable (const void *pa, const void *pb)
+{
+  assert_locus * const a = *(assert_locus * const *)pa;
+  assert_locus * const b = *(assert_locus * const *)pb;
+
+  if (a == NULL)
+    return b != NULL;
+  else if (b == NULL)
+    return -1;
+
+  if (a->e == NULL && b->e != NULL)
+    return 1;
+  else if (a->e != NULL && b->e == NULL)
+    return -1;
+
+  /* Sort after destination index.  */
+  if (a->e == NULL)
+    ;
+  else if (a->e->dest->index > b->e->dest->index)
+    return 1;
+  else if (a->e->dest->index < b->e->dest->index)
+    return -1;
+
+  /* Sort after comp_code.  */
+  if (a->comp_code > b->comp_code)
+    return 1;
+  else if (a->comp_code < b->comp_code)
+    return -1;
+
+  /* Break the tie using source/bb index.  */
+  return (a->e != NULL
+         ? a->e->src->index - b->e->src->index
+         : a->bb->index - b->bb->index);
+}
+
 /* Process all the insertions registered for every name N_i registered
    in NEED_ASSERT_FOR.  The list of assertions to be inserted are
    found in ASSERTS_FOR[i].  */
@@ -6506,11 +6547,12 @@ process_assert_insertions (void)
            }
        }
 
+      asserts.qsort (compare_assert_loc_stable);
       for (unsigned j = 0; j < asserts.length (); ++j)
        {
          loc = asserts[j];
          if (! loc)
-           continue;
+           break;
          update_edges_p |= process_assert_insertions_for (ssa_name (i), loc);
          num_asserts++;
          free (loc);

        Jakub

Reply via email to