| Message ID | 20160428155058.GT26501@tucnak.zalov.cz |
|---|---|
| State | New |
| Headers | show |
On 04/28/2016 09:50 AM, Jakub Jelinek wrote: > Hi! > > As reported in the PR and can be seen on this simplified testcase > everywhere, the FEs sometimes call operand_equal_p e.g. on a SAVE_EXPR > that contains a BIND_EXPR in it, and if arg0 == arg1, operand_equal_p > can return non-zero on it. > The ICE is because inchash::add_expr is unprepared to hash some trees, > it handles just tcc_declaration, selected specific trees and expressions of > all kinds, the last one usually by just recursing on all their operands. > For BIND_EXPR, the last operand is usually a BLOCK which we ICE on though, > and the middle argument usually a STATEMENT_LIST that we ICE on as well. > > The first hunk is just an optimization (but fixes the ICE anyway), > I think we really don't need to verify that a hash function for the same > argument always returns the same value. But I can imagine e.g. > a SAVE_EXPR of BIND_EXPR + var and var + the same SAVE_EXPR being compared > using operand_equal_p and there we wouldn't be equal at the top level and > still ICE. > The second hunk alone fixes the ICE too, by making sure we handle those > (just ignoring BLOCK and OMP_CLAUSE (the latter for now, if we find we want > to hash pre-OMP expansion trees too often we could adjust). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2016-04-28 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/70843 > * fold-const.c (operand_equal_p): Don't verify hash value equality > if arg0 == arg1. > * tree.c (inchash::add_expr): Handle STATEMENT_LIST. Ignore BLOCK > and OMP_CLAUSE. > > * gcc.dg/pr70843.c: New test. OK. Jeff
--- gcc/fold-const.c.jj 2016-04-27 15:29:05.000000000 +0200 +++ gcc/fold-const.c 2016-04-28 13:28:56.272276557 +0200 @@ -2756,12 +2756,15 @@ operand_equal_p (const_tree arg0, const_ { if (operand_equal_p (arg0, arg1, flags | OEP_NO_HASH_CHECK)) { - inchash::hash hstate0 (0), hstate1 (0); - inchash::add_expr (arg0, hstate0, flags); - inchash::add_expr (arg1, hstate1, flags); - hashval_t h0 = hstate0.end (); - hashval_t h1 = hstate1.end (); - gcc_assert (h0 == h1); + if (arg0 != arg1) + { + inchash::hash hstate0 (0), hstate1 (0); + inchash::add_expr (arg0, hstate0, flags); + inchash::add_expr (arg1, hstate1, flags); + hashval_t h0 = hstate0.end (); + hashval_t h1 = hstate1.end (); + gcc_assert (h0 == h1); + } return 1; } else --- gcc/tree.c.jj 2016-04-27 09:45:27.000000000 +0200 +++ gcc/tree.c 2016-04-28 13:24:01.770245254 +0200 @@ -7836,6 +7836,10 @@ add_expr (const_tree t, inchash::hash &h case PLACEHOLDER_EXPR: /* The node itself doesn't matter. */ return; + case BLOCK: + case OMP_CLAUSE: + /* Ignore. */ + return; case TREE_LIST: /* A list of expressions, for a CALL_EXPR or as the elements of a VECTOR_CST. */ @@ -7854,6 +7858,14 @@ add_expr (const_tree t, inchash::hash &h } return; } + case STATEMENT_LIST: + { + tree_stmt_iterator i; + for (i = tsi_start (CONST_CAST_TREE (t)); + !tsi_end_p (i); tsi_next (&i)) + inchash::add_expr (tsi_stmt (i), hstate, flags); + return; + } case FUNCTION_DECL: /* When referring to a built-in FUNCTION_DECL, use the __builtin__ form. Otherwise nodes that compare equal according to operand_equal_p might --- gcc/testsuite/gcc.dg/pr70843.c.jj 2016-04-28 13:37:54.596022706 +0200 +++ gcc/testsuite/gcc.dg/pr70843.c 2016-04-28 13:37:38.000000000 +0200 @@ -0,0 +1,9 @@ +/* PR middle-end/70843 */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +int +foo (int x, int y) +{ + return ({ int a = 5; a += x; a *= y; a; }) ? : 2; +}