Re: [PATCH] Fix PR42108

[Richard Biener <rguenther at suse dot de>]

On Tue, 9 Dec 2014, Richard Biener wrote:

> 
> The following finally fixes PR42108 (well, hopefully...) by using
> range-information on SSA names to allow the integer divisions introduced
> by Fortran array lowering being hoisted out of loops, thus detecting
> them as not trapping.
> 
> I chose to enhance tree_single_nonzero_warnv_p for this and adjusted
> operation_could_trap_helper_p to use this helper.

Unfortunately it can't be done this way as range-information is
dependent on the location of the definition - but if we change
predicates the way I tried code motion optimizations (like LIM)
will happily move that definition before dominating conditions
which may make the range information invalid.

The patch caused

                === acats tests ===
FAIL:   c450001
FAIL:   cxg2023
FAIL:   cxg2024

in testing (not investigated).

> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Richard.
> 
> 2014-12-09  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/42108
> 	* fold-const.c (tree_single_nonzero_warnv_p): Use range
> 	information associated with SSA names.
> 	* tree-eh.c (operation_could_trap_helper_p): Use
> 	tree_single_nonzero_warnv_p to check for trapping non-fp
> 	operation.
> 	* tree-vrp.c (remove_range_assertions): Remove bogus assert.
> 
> 	* gfortran.dg/pr42108.f90: Adjust testcase.
> 
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c	(revision 218479)
> +++ gcc/fold-const.c	(working copy)
> @@ -83,6 +83,8 @@ along with GCC; see the file COPYING3.
>  #include "cgraph.h"
>  #include "generic-match.h"
>  #include "optabs.h"
> +#include "stringpool.h"
> +#include "tree-ssanames.h"
>  
>  /* Nonzero if we are folding constants inside an initializer; zero
>     otherwise.  */
> @@ -15362,6 +15381,26 @@ tree_single_nonzero_warnv_p (tree t, boo
>  	}
>        break;
>  
> +    case SSA_NAME:
> +      if (INTEGRAL_TYPE_P (TREE_TYPE (t)))
> +	{
> +	  wide_int minv, maxv;
> +	  enum value_range_type rtype = get_range_info (t, &minv, &maxv);
> +	  if (rtype == VR_RANGE)
> +	    {
> +	      if (wi::lt_p (maxv, 0, TYPE_SIGN (TREE_TYPE (t)))
> +		  || wi::gt_p (minv, 0, TYPE_SIGN (TREE_TYPE (t))))
> +		return true;
> +	    }
> +	  else if (rtype == VR_ANTI_RANGE)
> +	    {
> +	      if (wi::le_p (minv, 0, TYPE_SIGN (TREE_TYPE (t)))
> +		  && wi::ge_p (maxv, 0, TYPE_SIGN (TREE_TYPE (t))))
> +		return true;
> +	    }
> +	}
> +      break;
> +
>      default:
>        break;
>      }
> Index: gcc/tree-eh.c
> ===================================================================
> --- gcc/tree-eh.c	(revision 218479)
> +++ gcc/tree-eh.c	(working copy)
> @@ -2440,13 +2440,16 @@ operation_could_trap_helper_p (enum tree
>      case ROUND_MOD_EXPR:
>      case TRUNC_MOD_EXPR:
>      case RDIV_EXPR:
> -      if (honor_snans || honor_trapv)
> -	return true;
> -      if (fp_operation)
> -	return flag_trapping_math;
> -      if (!TREE_CONSTANT (divisor) || integer_zerop (divisor))
> -        return true;
> -      return false;
> +      {
> +	if (honor_snans || honor_trapv)
> +	  return true;
> +	if (fp_operation)
> +	  return flag_trapping_math;
> +	bool sop;
> +	if (!tree_single_nonzero_warnv_p (divisor, &sop))
> +	  return true;
> +	return false;
> +      }
>  
>      case LT_EXPR:
>      case LE_EXPR:
> Index: gcc/testsuite/gfortran.dg/pr42108.f90
> ===================================================================
> --- gcc/testsuite/gfortran.dg/pr42108.f90	(revision 218479)
> +++ gcc/testsuite/gfortran.dg/pr42108.f90	(working copy)
> @@ -1,5 +1,5 @@
>  ! { dg-do compile }
> -! { dg-options "-O2 -fdump-tree-fre1" }
> +! { dg-options "-O2 -fdump-tree-fre1 -fdump-tree-lim1-details" }
>  
>  subroutine  eval(foo1,foo2,foo3,foo4,x,n,nnd)
>    implicit real*8 (a-h,o-z)
> @@ -21,7 +21,10 @@ subroutine  eval(foo1,foo2,foo3,foo4,x,n
>    end do
>  end subroutine eval
>  
> -! There should be only one load from n left
> +! There should be only one load from n left and the division should
> +! be hoisted out of the loop
>  
>  ! { dg-final { scan-tree-dump-times "\\*n_" 1 "fre1" } }
> +! { dg-final { scan-tree-dump-times "Moving statement" 5 "lim1" } }
>  ! { dg-final { cleanup-tree-dump "fre1" } }
> +! { dg-final { cleanup-tree-dump "lim1" } }
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c	(revision 218479)
> +++ gcc/tree-vrp.c	(working copy)
> @@ -6866,12 +6866,9 @@ remove_range_assertions (void)
>  	    tree lhs = gimple_assign_lhs (stmt);
>  	    tree rhs = gimple_assign_rhs1 (stmt);
>  	    tree var;
> -	    tree cond = fold (ASSERT_EXPR_COND (rhs));
>  	    use_operand_p use_p;
>  	    imm_use_iterator iter;
>  
> -	    gcc_assert (cond != boolean_false_node);
> -
>  	    var = ASSERT_EXPR_VAR (rhs);
>  	    gcc_assert (TREE_CODE (var) == SSA_NAME);
>  
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany