Created attachment 40988 [details] Reproducer. GCC fails with ICE with -O3 -march=broadwell/skylake-avx512 options. Error: >$ g++ -O3 -c -w -march=broadwell repr.cpp internal compiler error: in gimple_build_assign_1, at gimple.c:422 void foo () { ^~~ 0xabf24b gimple_build_assign_1 /home/vsevolod/workspace/gcc-dev/trunk/gcc/gimple.c:422 0xabf24b gimple_build_assign(tree_node*, tree_code, tree_node*, tree_node*) /home/vsevolod/workspace/gcc-dev/trunk/gcc/gimple.c:453 0xf1af6d rewrite_expr_tree /home/vsevolod/workspace/gcc-dev/trunk/gcc/tree-ssa-reassoc.c:4242 0xf1abb8 rewrite_expr_tree /home/vsevolod/workspace/gcc-dev/trunk/gcc/tree-ssa-reassoc.c:4287 (.........) GCC version: Using built-in specs. COLLECT_GCC=g++ COLLECT_LTO_WRAPPER=/home/vsevolod/workspace/gcc-dev/bin-trunk/libexec/gcc/x86_64-pc-linux-gnu/7.0.1/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /home/vsevolod/workspace/gcc-dev/trunk/configure --prefix=/home/vsevolod/workspace/gcc-dev/bin-trunk Thread model: posix gcc version 7.0.1 20170315 (experimental) (GCC) Test case is huge and creduce has failed to reduce it further.
Confirmed. #3 0x00000000013be796 in rewrite_expr_tree ( stmt=<gimple_assign 0x7fffec0963c0>, opindex=8, ops=..., changed=true) at /space/rguenther/src/svn/gcc-7-branch/gcc/tree-ssa-reassoc.c:4243 4243 oe1->op, oe2->op); (gdb) l 4238 gimple *insert_point 4239 = find_insert_point (stmt, oe1->op, oe2->op); 4240 lhs = make_ssa_name (TREE_TYPE (lhs)); 4241 stmt 4242 = gimple_build_assign (lhs, gimple_assign_rhs_code (stmt), 4243 oe1->op, oe2->op); (gdb) p gimple_assign_rhs_code (stmt) $1 = NOP_EXPR
I'll have a look.
This is quite impossible to reduce, after 16 hours of creduce I've reduced 22% from the original size. I've tried to build a testcase based on what I see in the reassociation, but void bar (int); unsigned long int vx, vx2, vx3; unsigned long int foo (unsigned long int _55062, unsigned long int _55063, unsigned long int _55172, unsigned long int _55171, int u, int v, int w, int x) { unsigned long int _55173 = _55171 * _55172; _Bool t1, t2; if (u == 35) t1 = 1; else if (u == 27) t1 = 0; else if (v == 12) { bar (4); t1 = 1; } else if (v == 24) { bar (5); t1 = 1; } else { bar (6); t1 = 1; } unsigned long int _55001 = vx2; unsigned long int _55053 = t1; unsigned long int _55054 = _55001 * _55053; unsigned long int _55064 = _55062 * _55063; unsigned long int _55060 = vx; unsigned long int _55065 = -_55064; unsigned long int _55066 = _55060 * _55065; unsigned long int _55067 = _55054 * _55066; if (w == 35) t2 = 1; else if (w == 27) t2 = 0; else if (x == 12) { bar (4); t2 = 1; } else if (x == 24) { bar (5); t2 = 1; } else { bar (6); t2 = 1; } unsigned long int _55119 = vx3; unsigned long int _55225 = t2; unsigned long int _55226 = _55173 * _55225; unsigned long int _55227 = _55119 * _55226; unsigned long int _55228 = _55067 * _55227; unsigned long int _55229 = _55228 * 9323891652267032265ULL; return _55229; } has different ranks and so while it gives the same ops order after linearize_expr_tree, the following sorting changes it.
On Wed, 22 Mar 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80072 > > --- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > This is quite impossible to reduce, after 16 hours of creduce I've reduced 22% > from the original size. > I've tried to build a testcase based on what I see in the reassociation, but > void bar (int); > unsigned long int vx, vx2, vx3; > > unsigned long int > foo (unsigned long int _55062, unsigned long int _55063, > unsigned long int _55172, unsigned long int _55171, > int u, int v, int w, int x) > { > unsigned long int _55173 = _55171 * _55172; > _Bool t1, t2; > if (u == 35) > t1 = 1; > else if (u == 27) > t1 = 0; > else if (v == 12) > { > bar (4); > t1 = 1; > } > else if (v == 24) > { > bar (5); > t1 = 1; > } > else > { > bar (6); > t1 = 1; > } > unsigned long int _55001 = vx2; > unsigned long int _55053 = t1; > unsigned long int _55054 = _55001 * _55053; > unsigned long int _55064 = _55062 * _55063; > unsigned long int _55060 = vx; > unsigned long int _55065 = -_55064; > unsigned long int _55066 = _55060 * _55065; > unsigned long int _55067 = _55054 * _55066; > if (w == 35) > t2 = 1; > else if (w == 27) > t2 = 0; > else if (x == 12) > { > bar (4); > t2 = 1; > } > else if (x == 24) > { > bar (5); > t2 = 1; > } > else > { > bar (6); > t2 = 1; > } > unsigned long int _55119 = vx3; > unsigned long int _55225 = t2; > unsigned long int _55226 = _55173 * _55225; > unsigned long int _55227 = _55119 * _55226; > unsigned long int _55228 = _55067 * _55227; > unsigned long int _55229 = _55228 * 9323891652267032265ULL; > return _55229; > } > > has different ranks and so while it gives the same ops order after > linearize_expr_tree, the following sorting changes it. Did you try -fdump-<pass-before-reassoc>-gimple and create a gimple testcase out of the affected function IL?
That function is way too large for anything. But I'll try to get the same ranks now.
(In reply to Jakub Jelinek from comment #5) > That function is way too large for anything. But I'll try to get the same > ranks now. If you have analyzed what happens you can maybe create an artificial gimple testcase reproducing it (given I guess only one or two reassoc chains are required in the end).
So, found a bug and its bugfix fixes the testcase, but still need to figure out what actually was going on, if it was just that we made (correct) assumptions that constants won't be weirdly sorted, or if there is some other (with this patch latent) bug. --- gcc/tree-ssa-reassoc.c.jj 2017-02-10 09:46:50.000000000 +0100 +++ gcc/tree-ssa-reassoc.c 2017-03-22 13:13:33.608745625 +0100 @@ -510,7 +510,7 @@ sort_by_operand_rank (const void *pa, co /* 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) { @@ -549,7 +549,7 @@ sort_by_operand_rank (const void *pa, co } if (oeb->rank != oea->rank) - return oeb->rank - oea->rank; + return oeb->rank > oea->rank ? 1 : -1; else return oeb->id - oea->id; } ->rank is unsigned int, and on the huge testcase it is over 0x7fffffff, so (int) (oeb->rank > oea->rank) doesn't do what it expects and we happily sort a SSA_NAME with extra high rank after constants, which e.g. means we can't merge them together.
On Wed, 22 Mar 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80072 > > --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > So, found a bug and its bugfix fixes the testcase, but still need to figure out > what actually was going on, if it was just that we made (correct) assumptions > that constants won't be weirdly sorted, or if there is some other (with this > patch latent) bug. > > --- gcc/tree-ssa-reassoc.c.jj 2017-02-10 09:46:50.000000000 +0100 > +++ gcc/tree-ssa-reassoc.c 2017-03-22 13:13:33.608745625 +0100 > @@ -510,7 +510,7 @@ sort_by_operand_rank (const void *pa, co > > /* 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) > { > @@ -549,7 +549,7 @@ sort_by_operand_rank (const void *pa, co > } > > if (oeb->rank != oea->rank) > - return oeb->rank - oea->rank; > + return oeb->rank > oea->rank ? 1 : -1; > else > return oeb->id - oea->id; > } > > ->rank is unsigned int, and on the huge testcase it is over 0x7fffffff, so > (int) (oeb->rank > oea->rank) doesn't do what it expects and we happily sort > a SSA_NAME with extra high rank after constants, which e.g. means we can't > merge them together. Ick ;) Patch looks obvious. For clarity the else return oeb->id - oea->id should probably be adjusted similarly (and id made unsigned...).
Created attachment 41020 [details] gcc7-pr80072.patch Full untested patch.
Comment on attachment 41020 [details] gcc7-pr80072.patch Looks good.
Author: jakub Date: Wed Mar 22 21:52:13 2017 New Revision: 246408 URL: https://gcc.gnu.org/viewcvs?rev=246408&root=gcc&view=rev Log: 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. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-ssa-reassoc.c
Fixed.