On Tue, Jul 04, 2017 at 01:46:25PM +0200, Richard Biener wrote:
> > 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;
>
> so this will now ICE if b->e is NULL, did you forget the && b->e == NULL
> above?
That was intentional. If a->e != NULL, then we know that b->e != NULL,
because we have
else if (a->e != NULL && b->e == NULL)
return -1;
earlier. Similarly, if a->e == NULL, then we know that b-> == NULL, because
we have:
if (a->e == NULL && b->e != NULL)
return 1;
earlier.
> > @@ -6506,11 +6547,12 @@ process_assert_insertions (void)
> > }
> > }
> >
> > + asserts.qsort (compare_assert_loc_stable);
>
> please add a comment here why we re-sort.
Ok, will do.
> > 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);
>
> Otherwise looks ok to me. I wonder if we should merge the two
> sorting functions and change behavior with a global var or a
> template parameter instead (to reduce source duplication). Does
>
> vec.qsort (function_template<true>);
>
> work?
Let me try that.
Jakub
