This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix reassoc very large TU handlng (PR tree-optimization/80072)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 22 Mar 2017 19:52:40 +0100
- Subject: [PATCH] Fix reassoc very large TU handlng (PR tree-optimization/80072)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jakub at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E0FEB81F01
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E0FEB81F01
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
On the huge testcase from the PR (not really reduceable) we run into
reassoc ranks bigger than INT_MAX (bb_rank is multiples of 65536,
so it might be enough to have just 32768+ basic blocks to reach that).
Rank is unsigned int, but we were returning (int) (oeb->rank - oea->rank);
from the comparison function, which means when unlucky enough it could sort
entries with huge rank e.g. after entries with 0 rank. Rest of reassoc
relies on the constants (rank 0) being sorted last though.
The patch also applies similar changes to other fields, that are far less
likely to be huge, but could be in theory.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-22 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/80072
* tree-ssa-reassoc.c (struct operand_entry): Change id field type
to unsigned int.
(next_operand_entry_id): Change type to unsigned int.
(sort_by_operand_rank): Make sure to return the right return value
even if unsigned fields are bigger than INT_MAX.
(struct oecount): Change cnt and id type to unsigned int.
(oecount_hasher::equal): Formatting fix.
(oecount_cmp): Make sure to return the right return value
even if unsigned fields are bigger than INT_MAX.
(undistribute_ops_list): Change next_oecount_id type to unsigned int.
--- gcc/tree-ssa-reassoc.c.jj 2017-02-10 09:46:50.000000000 +0100
+++ gcc/tree-ssa-reassoc.c 2017-03-22 13:57:00.476928949 +0100
@@ -193,7 +193,7 @@ static struct
struct operand_entry
{
unsigned int rank;
- int id;
+ unsigned int id;
tree op;
unsigned int count;
gimple *stmt_to_insert;
@@ -204,7 +204,7 @@ static object_allocator<operand_entry> o
/* This is used to assign a unique ID to each struct operand_entry
so that qsort results are identical on different hosts. */
-static int next_operand_entry_id;
+static unsigned int next_operand_entry_id;
/* Starting rank number for a given basic block, so that we can rank
operations using unmovable instructions in that BB based on the bb
@@ -505,12 +505,12 @@ sort_by_operand_rank (const void *pa, co
else
/* To make sorting result stable, we use unique IDs to determine
order. */
- return oeb->id - oea->id;
+ return oeb->id > oea->id ? 1 : -1;
}
/* Lastly, make sure the versions that are the same go next to each
other. */
- if ((oeb->rank - oea->rank == 0)
+ if (oeb->rank == oea->rank
&& TREE_CODE (oea->op) == SSA_NAME
&& TREE_CODE (oeb->op) == SSA_NAME)
{
@@ -543,15 +543,15 @@ sort_by_operand_rank (const void *pa, co
}
if (SSA_NAME_VERSION (oeb->op) != SSA_NAME_VERSION (oea->op))
- return SSA_NAME_VERSION (oeb->op) - SSA_NAME_VERSION (oea->op);
+ return SSA_NAME_VERSION (oeb->op) > SSA_NAME_VERSION (oea->op) ? 1 : -1;
else
- return oeb->id - oea->id;
+ return oeb->id > oea->id ? 1 : -1;
}
if (oeb->rank != oea->rank)
- return oeb->rank - oea->rank;
+ return oeb->rank > oea->rank ? 1 : -1;
else
- return oeb->id - oea->id;
+ return oeb->id > oea->id ? 1 : -1;
}
/* Add an operand entry to *OPS for the tree operand OP. */
@@ -1055,8 +1055,8 @@ static void linearize_expr_tree (vec<ope
/* Structure for tracking and counting operands. */
struct oecount {
- int cnt;
- int id;
+ unsigned int cnt;
+ unsigned int id;
enum tree_code oecode;
tree op;
};
@@ -1090,8 +1090,7 @@ oecount_hasher::equal (int p1, int p2)
{
const oecount *c1 = &cvec[p1 - 42];
const oecount *c2 = &cvec[p2 - 42];
- return (c1->oecode == c2->oecode
- && c1->op == c2->op);
+ return c1->oecode == c2->oecode && c1->op == c2->op;
}
/* Comparison function for qsort sorting oecount elements by count. */
@@ -1102,10 +1101,10 @@ oecount_cmp (const void *p1, const void
const oecount *c1 = (const oecount *)p1;
const oecount *c2 = (const oecount *)p2;
if (c1->cnt != c2->cnt)
- return c1->cnt - c2->cnt;
+ return c1->cnt > c2->cnt ? 1 : -1;
else
/* If counts are identical, use unique IDs to stabilize qsort. */
- return c1->id - c2->id;
+ return c1->id > c2->id ? 1 : -1;
}
/* Return TRUE iff STMT represents a builtin call that raises OP
@@ -1559,7 +1558,7 @@ undistribute_ops_list (enum tree_code op
sbitmap_iterator sbi0;
vec<operand_entry *> *subops;
bool changed = false;
- int next_oecount_id = 0;
+ unsigned int next_oecount_id = 0;
if (length <= 1
|| opcode != PLUS_EXPR)
Jakub