Re: PR68146: Check for null SSA_NAME_DEF_STMTs in fold-const.c

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Dec 2, 2015 at 9:33 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> The problem in the testcase was that tree-complex.c was trying
> to fold ABS_EXPRs of SSA names that didn't yet have a definition
> (because the SSA names were real and imaginary parts of a complex
> SSA name whose definition hadn't yet been visited by the pass).
> tree-complex.c uses a straightforward walk in index order:
>
>   /* ??? Ideally we'd traverse the blocks in breadth-first order.  */
>   old_last_basic_block = last_basic_block_for_fn (cfun);
>   FOR_EACH_BB_FN (bb, cfun)
>     {
>
> and in the testcase, we have a block A with a single successor B that
> comes before it.  B has no other predecessor and has a complex division
> that uses an SSA name X defined in A, so we split the components of X
> before we reach the definition of X.  (I imagine cfgcleanup would
> clean this up by joining A and B.)
>
> Tested on x86_64-linux-gnu.  OK to install?

I think the new checks are just bogus because all SSA names in the IL ought
to have a def-stmt (if only a GIMPLE_NOP).

So I think what tree-complex.c does it just wrong(TM) and worked by
accident only.

So I suggest instead compute a proper CFG order and process basic blocks in
that order instead.  (a domwalk doesn't work as explained in the PR)

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         PR tree-optimization/68146
>         * fold-const.c (tree_binary_nonnegative_warnv_p): Check for null
>         SSA_NAME_DEF_STMTs.
>         (integer_valued_real_call_p): Likewise.
>
> gcc/testsuite/
>         * gfortran.fortran-torture/compile/pr68146.f90: New test.
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 16bff5f..c99e78e 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -12867,6 +12867,8 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0,
>  bool
>  tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
>  {
> +  gimple *stmt;
> +
>    if (TYPE_UNSIGNED (TREE_TYPE (t)))
>      return true;
>
> @@ -12892,8 +12894,9 @@ tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
>          to provide it through dataflow propagation.  */
>        return (!name_registered_for_update_p (t)
>               && depth < PARAM_VALUE (PARAM_MAX_SSA_NAME_QUERY_DEPTH)
> -             && gimple_stmt_nonnegative_warnv_p (SSA_NAME_DEF_STMT (t),
> -                                                 strict_overflow_p, depth));
> +             && (stmt = SSA_NAME_DEF_STMT (t))
> +             && gimple_stmt_nonnegative_warnv_p (stmt, strict_overflow_p,
> +                                                 depth));
>
>      default:
>        return tree_simple_nonnegative_warnv_p (TREE_CODE (t), TREE_TYPE (t));
> @@ -13508,6 +13511,7 @@ integer_valued_real_call_p (combined_fn fn, tree arg0, tree arg1, int depth)
>  bool
>  integer_valued_real_single_p (tree t, int depth)
>  {
> +  gimple *stmt;
>    switch (TREE_CODE (t))
>      {
>      case REAL_CST:
> @@ -13524,8 +13528,8 @@ integer_valued_real_single_p (tree t, int depth)
>          to provide it through dataflow propagation.  */
>        return (!name_registered_for_update_p (t)
>               && depth < PARAM_VALUE (PARAM_MAX_SSA_NAME_QUERY_DEPTH)
> -             && gimple_stmt_integer_valued_real_p (SSA_NAME_DEF_STMT (t),
> -                                                   depth));
> +             && (stmt = SSA_NAME_DEF_STMT (t))
> +             && gimple_stmt_integer_valued_real_p (stmt, depth));
>
>      default:
>        break;
> diff --git a/gcc/testsuite/gfortran.fortran-torture/compile/pr68146.f90 b/gcc/testsuite/gfortran.fortran-torture/compile/pr68146.f90
> new file mode 100644
> index 0000000..7f75ec0
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.fortran-torture/compile/pr68146.f90
> @@ -0,0 +1,11 @@
> +subroutine foo(a1, a2, s1, s2, n)
> +  integer :: n
> +  complex(kind=8) :: a1(n), a2(n), s1, s2
> +  do i = 1, n
> +     a1(i) = i
> +  end do
> +  s1 = 20.0 / s2
> +  do i = 1, n
> +     a2(i) = i / s2
> +  end do
> +end subroutine foo
>