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

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

[Richard Sandiford]

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?

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

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

[Richard Biener]

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
>