https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83491

--- Comment #3 from Wilco <wilco at gcc dot gnu.org> ---
(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).

Reply via email to