Hi!
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.
The following patch fixes it, by doing all the checks unconditionally.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-10-03 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/82381
* tree-ssa-reassoc.c (sort_by_operand_rank): Don't check
stmt_to_insert nor wheather SSA_NAMEs are default defs.
Return 1 or -1 if one of bba and bbb is NULL. If bb_rank is equal,
fallthrough into reassoc_stmt_dominates_stmt_p.
* gcc.c-torture/compile/pr82381.c: New test.
--- 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)
{
- /* As SSA_NAME_VERSION is assigned pretty randomly, because we reuse
- versions of removed SSA_NAMEs, so if possible, prefer to sort
- based on basic block and gimple_uid of the SSA_NAME_DEF_STMT.
- See PR60418. */
- if (!SSA_NAME_IS_DEFAULT_DEF (oea->op)
- && !SSA_NAME_IS_DEFAULT_DEF (oeb->op)
- && !oea->stmt_to_insert
- && !oeb->stmt_to_insert
- && SSA_NAME_VERSION (oeb->op) != SSA_NAME_VERSION (oea->op))
+ if (SSA_NAME_VERSION (oeb->op) != SSA_NAME_VERSION (oea->op))
{
+ /* As SSA_NAME_VERSION is assigned pretty randomly, because we reuse
+ versions of removed SSA_NAMEs, so if possible, prefer to sort
+ based on basic block and gimple_uid of the SSA_NAME_DEF_STMT.
+ See PR60418. */
gimple *stmta = SSA_NAME_DEF_STMT (oea->op);
gimple *stmtb = SSA_NAME_DEF_STMT (oeb->op);
basic_block bba = gimple_bb (stmta);
basic_block bbb = gimple_bb (stmtb);
if (bbb != bba)
{
+ /* One of the SSA_NAMEs can be defined in oeN->stmt_to_insert
+ but the other might not. */
+ if (!bba)
+ return 1;
+ if (!bbb)
+ return -1;
+ /* If neither is, compare bb_rank. */
if (bb_rank[bbb->index] != bb_rank[bba->index])
return bb_rank[bbb->index] - bb_rank[bba->index];
}
- else
- {
- bool da = reassoc_stmt_dominates_stmt_p (stmta, stmtb);
- bool db = reassoc_stmt_dominates_stmt_p (stmtb, stmta);
- if (da != db)
- return da ? 1 : -1;
- }
- }
- if (SSA_NAME_VERSION (oeb->op) != SSA_NAME_VERSION (oea->op))
- return SSA_NAME_VERSION (oeb->op) > SSA_NAME_VERSION (oea->op) ? 1 : -1;
+ bool da = reassoc_stmt_dominates_stmt_p (stmta, stmtb);
+ bool db = reassoc_stmt_dominates_stmt_p (stmtb, stmta);
+ if (da != db)
+ return da ? 1 : -1;
+
+ return (SSA_NAME_VERSION (oeb->op) > SSA_NAME_VERSION (oea->op)
+ ? 1 : -1);
+ }
else
return oeb->id > oea->id ? 1 : -1;
}
--- gcc/testsuite/gcc.c-torture/compile/pr82381.c.jj 2017-10-02
18:13:51.532715601 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr82381.c 2017-10-02
18:13:51.532715601 +0200
@@ -0,0 +1,18 @@
+/* PR tree-optimization/82381 */
+/* { dg-do compile } */
+
+signed char b, h;
+unsigned short c, e;
+short int d, f, g;
+
+void
+foo ()
+{
+ if (h)
+ {
+ short a = -(d + c - b);
+ f = e - a - -d;
+ }
+ if (c)
+ g = 0;
+}
Jakub
