This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix sort_by_operand_rank with qsort checking (PR tree-optimization/82381)


On Tue, 3 Oct 2017, Jakub Jelinek wrote:
> The qsort cmp transitivity checks may fail with the sort_by_operand_rank
> comparator, because if one of the operands has stmt_to_insert and the
> other doesn't (but likely also if one SSA_NAME is (D) and the other is not),
> we fallthrough into SSA_NAME_VERSION comparison, but if both aren't (D) and
> both don't have stmt_to_insert, we use different comparison rules.

When looking at the code I've noticed another issue, so the fix seems
incomplete (but I don't know if the other problem can/should be fixed
independently), see below:


> --- gcc/tree-ssa-reassoc.c.jj	2017-10-02 17:33:14.270282226 +0200
> +++ gcc/tree-ssa-reassoc.c	2017-10-02 18:18:07.328611132 +0200
> @@ -514,36 +514,37 @@ sort_by_operand_rank (const void *pa, co
>        && TREE_CODE (oea->op) == SSA_NAME
>        && TREE_CODE (oeb->op) == SSA_NAME)
>      {

The containing if statement condition reads:

  if (oeb->rank == oea->rank
      && TREE_CODE (oea->op) == SSA_NAME
      && TREE_CODE (oeb->op) == SSA_NAME)

so as far as I can tell it's possible to have items A, B, C such that
all have same rank, A and C correspond to SSA_NAMEs and compare C < A,
but B does not, so comparisons of A or C against B fall down to 

    return oeb->id > oea->id ? 1 : -1;

and ultimately result in A < B < C < A.

Alexander


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]