[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 Jeffrey A. Law changed: What|Removed |Added Status|NEW |RESOLVED CC||law at redhat dot com Resolution|--- |FIXED --- Comment #8 from Jeffrey A. Law --- Fixed by Wilco's patch on the trunk.
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 --- Comment #7 from Jeffrey A. Law --- Author: law Date: Wed Dec 20 23:55:42 2017 New Revision: 255906 URL: https://gcc.gnu.org/viewcvs?rev=255906=gcc=rev Log: PR tree-optimization/83491 * tree-ssa-math-opts.c (execute_cse_reciprocals_1): Check for SSA_NAME before walking uses. Improve coding style and comments. PR tree-optimization/83491 * gcc.dg/pr83491.c: Add new test. Added: trunk/gcc/testsuite/gcc.dg/pr83491.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-math-opts.c
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 --- Comment #6 from Wilco --- Patch: https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01357.html
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 --- Comment #5 from Jakub Jelinek --- And "in to account" should be "into account".
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 --- Comment #4 from Jakub Jelinek --- Thanks. One more nit: /* If this is a square (x * x), we should check whether there are any enough divisions by x on it's own to warrant waiting for that pass. */ Either whether there are any divisions, or whether there are enough divisions, any enough doesn't make sense to me.
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491
--- Comment #3 from Wilco ---
(In reply to Jakub Jelinek from comment #2)
> There are multiple bugs:
> 1) the callers of execute_cse_reciprocals_1 ensure that def is SSA_NAME, so
> using:
> /* If this is a square (x * x), we should check whether there are any
> enough divisions by x on it's own to warrant waiting for that pass. */
> if (TREE_CODE (def) == SSA_NAME)
> {
> gimple *def_stmt = SSA_NAME_DEF_STMT (def);
> ...
> }
>
> FOR_EACH_IMM_USE_FAST (use_p, use_iter, def)
> {
> gimple *use_stmt = USE_STMT (use_p);
> is misleading. The thing is, the FOR_EACH_IMM_USE_FAST would fail miserably
> if
> def isn't SSA_NAME. Please just add gcc_checking_assert (TREE_CODE (def) ==
> SSA_NAME); or gcc_assert, and reindent the body of the useless first if.
>
> 2)
> if (is_gimple_assign (def_stmt)
> && gimple_assign_rhs_code (def_stmt) == MULT_EXPR
> && gimple_assign_rhs1 (def_stmt) == gimple_assign_rhs2 (def_stmt))
> This of course needs to also verify that TREE_CODE (gimple_assign_rhs1
> (def_stmt)) == SSA_NAME. The equality comparison afterwards is ok, as
> SSA_NAMEs are unique and can be compared with pointer comparisons.
>
> 3) sqrt_recip_count ++;
> formatting (no space in between variable name and ++). Happens several
> times in the function.
>
> 4)
> /* Do the expensive part only if we can hope to optimize something. */
> if (count + square_recip_count >= threshold
> && count >= 1)
> This condition should have been on a single line, it is short enough to fit.
Yes, I've got a fix for (2), and can easily clean up the coding style issues.
Interestingly it's possible to trigger the failure much more often with
-frounding-math (where you end up with def = const*const).
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek ---
There are multiple bugs:
1) the callers of execute_cse_reciprocals_1 ensure that def is SSA_NAME, so
using:
/* If this is a square (x * x), we should check whether there are any
enough divisions by x on it's own to warrant waiting for that pass. */
if (TREE_CODE (def) == SSA_NAME)
{
gimple *def_stmt = SSA_NAME_DEF_STMT (def);
...
}
FOR_EACH_IMM_USE_FAST (use_p, use_iter, def)
{
gimple *use_stmt = USE_STMT (use_p);
is misleading. The thing is, the FOR_EACH_IMM_USE_FAST would fail miserably if
def isn't SSA_NAME. Please just add gcc_checking_assert (TREE_CODE (def) ==
SSA_NAME); or gcc_assert, and reindent the body of the useless first if.
2)
if (is_gimple_assign (def_stmt)
&& gimple_assign_rhs_code (def_stmt) == MULT_EXPR
&& gimple_assign_rhs1 (def_stmt) == gimple_assign_rhs2 (def_stmt))
This of course needs to also verify that TREE_CODE (gimple_assign_rhs1
(def_stmt)) == SSA_NAME. The equality comparison afterwards is ok, as
SSA_NAMEs are unique and can be compared with pointer comparisons.
3) sqrt_recip_count ++;
formatting (no space in between variable name and ++). Happens several times
in the function.
4)
/* Do the expensive part only if we can hope to optimize something. */
if (count + square_recip_count >= threshold
&& count >= 1)
This condition should have been on a single line, it is short enough to fit.
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 Richard Biener changed: What|Removed |Added Version|unknown |8.0 Target Milestone|--- |8.0
[Bug tree-optimization/83491] [8 Regression] ICE in execute_cse_reciprocals_1 at gcc/tree-ssa-math-opts.c:585
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491 Wilco changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2017-12-19 Ever confirmed|0 |1 --- Comment #1 from Wilco --- Confirmed, I'll have a look
