Patchwork gimple_val_nonnegative_real_p (PR46728 patch 7 of 7)

login
register
mail settings
Submitter William J. Schmidt
Date June 1, 2011, 7:27 p.m.
Message ID <1306956426.5243.18.camel@L3G5336.ibm.com>
Download mbox | patch
Permalink /patch/98247/
State New
Headers show

Comments

William J. Schmidt - June 1, 2011, 7:27 p.m.
This patch cleans up the FIXME logic in gimple_expand_builtin_pow by
introducing gimple_val_nonnegative_real_p for the same purpose that
tree_expr_nonnegative_p served in the expand logic.  This completes the
work for PR46728.

Bootstrapped/regtested on powerpc64-linux.


2011-06-01  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/46728
	* tree-ssa-math-opts.c (gimple_expand_builtin_pow): Change FIXME
	to use gimple_val_nonnegative_real_p.
	* gimple-fold.c (gimple_val_nonnegative_real_p): New function.
	* gimple.h (gimple_val_nonnegative_real_p): New declaration.
Richard Guenther - June 6, 2011, 11 a.m.
On Wed, Jun 1, 2011 at 9:27 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> This patch cleans up the FIXME logic in gimple_expand_builtin_pow by
> introducing gimple_val_nonnegative_real_p for the same purpose that
> tree_expr_nonnegative_p served in the expand logic.  This completes the
> work for PR46728.
>
> Bootstrapped/regtested on powerpc64-linux.
>
>
> 2011-06-01  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>        PR tree-optimization/46728
>        * tree-ssa-math-opts.c (gimple_expand_builtin_pow): Change FIXME
>        to use gimple_val_nonnegative_real_p.
>        * gimple-fold.c (gimple_val_nonnegative_real_p): New function.
>        * gimple.h (gimple_val_nonnegative_real_p): New declaration.
>
>
> Index: gcc/tree-ssa-math-opts.c
> ===================================================================
> --- gcc/tree-ssa-math-opts.c    (revision 174535)
> +++ gcc/tree-ssa-math-opts.c    (working copy)
> @@ -1172,13 +1172,7 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
>
>   if (flag_unsafe_math_optimizations
>       && cbrtfn
> -      /* FIXME: The following line was originally
> -        && (tree_expr_nonnegative_p (arg0) || !HONOR_NANS (mode)),
> -        but since arg0 is a gimple value, the first predicate
> -        will always return false.  It needs to be replaced with a
> -        call to a similar gimple_val_nonnegative_p function to be
> -         added in gimple-fold.c.  */
> -      && !HONOR_NANS (mode)
> +      && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
>       && REAL_VALUES_EQUAL (c, dconst1_3))
>     return build_and_insert_call (gsi, loc, &target, cbrtfn, arg0);
>
> @@ -1190,13 +1184,7 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
>   if (flag_unsafe_math_optimizations
>       && sqrtfn
>       && cbrtfn
> -      /* FIXME: The following line was originally
> -        && (tree_expr_nonnegative_p (arg0) || !HONOR_NANS (mode)),
> -        but since arg0 is a gimple value, the first predicate
> -        will always return false.  It needs to be replaced with a
> -        call to a similar gimple_val_nonnegative_p function to be
> -         added in gimple-fold.c.  */
> -      && !HONOR_NANS (mode)
> +      && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
>       && optimize_function_for_speed_p (cfun)
>       && hw_sqrt_exists
>       && REAL_VALUES_EQUAL (c, dconst1_6))
> @@ -1270,13 +1258,7 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
>
>   if (flag_unsafe_math_optimizations
>       && cbrtfn
> -      /* FIXME: The following line was originally
> -        && (tree_expr_nonnegative_p (arg0) || !HONOR_NANS (mode)),
> -        but since arg0 is a gimple value, the first predicate
> -        will always return false.  It needs to be replaced with a
> -        call to a similar gimple_val_nonnegative_p function to be
> -         added in gimple-fold.c.  */
> -      && !HONOR_NANS (mode)
> +      && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
>       && real_identical (&c2, &c)
>       && optimize_function_for_speed_p (cfun)
>       && powi_cost (n / 3) <= POWI_MAX_MULTS)
> Index: gcc/gimple-fold.c
> ===================================================================
> --- gcc/gimple-fold.c   (revision 174535)
> +++ gcc/gimple-fold.c   (working copy)
> @@ -3433,3 +3433,224 @@ fold_const_aggregate_ref (tree t)
>  {
>   return fold_const_aggregate_ref_1 (t, NULL);
>  }
> +
> +/* Return true iff VAL is a gimple expression that is known to be
> +   non-negative.  Restricted to floating-point inputs.  When changing
> +   this function, review fold-const.c:tree_expr_nonnegative_p to see
> +   whether similar changes are required.  */
> +
> +bool
> +gimple_val_nonnegative_real_p (tree val)
> +{
> +  gimple def_stmt;
> +
> +  /* Use existing logic for non-gimple trees.  */
> +  if (tree_expr_nonnegative_p (val))
> +    return true;
> +
> +  if (TREE_CODE (val) != SSA_NAME)
> +    return false;
> +
> +  def_stmt = SSA_NAME_DEF_STMT (val);
> +
> +  if (is_gimple_assign (def_stmt))
> +    {
> +      tree op0, op1;
> +
> +      /* If this is just a copy between SSA names, check the RHS.  */
> +      if (gimple_assign_ssa_name_copy_p (def_stmt))
> +       {
> +         op0 = gimple_assign_rhs1 (def_stmt);
> +         return gimple_val_nonnegative_real_p (op0);
> +       }

If handled then do so as SSA_NAME: case below.

> +      switch (gimple_assign_rhs_code (def_stmt))
> +       {
> +       case ABS_EXPR:
> +         /* Always true for floating-point operands.  */
> +         return true;

You don't verify anywhere that the input is FP.

As the depth of the expression we look at is unbound it is probably
easy to construct a testcase that exhibits quadratic compile-time
behavior like pow(pow(pow(pow(...,0.5), 0.5), 0.5), 0.5).  I originally
thought of just looking at the immediate defining statement but
never at its operands (simply return false when only the operand
would tell).  And I still think that is the way to go and should still
catch 99% of the useful cases.

As for the grand masterplan we probably should eventually drive
the builtin-folding by information provided by a SSA or DOM propagation
engine (see tree-complex.c for example).  That would avoid the
quadratic-ness.

So, please prune any recursion.

Thanks,
Richard.

> +       case NOP_EXPR:
> +       case CONVERT_EXPR:
> +         /* True if the first operand is a nonnegative real.  */
> +         op0 = gimple_assign_rhs1 (def_stmt);
> +         return (TREE_CODE (TREE_TYPE (op0)) == REAL_TYPE
> +                 && gimple_val_nonnegative_real_p (op0));
> +
> +       case PLUS_EXPR:
> +       case MIN_EXPR:
> +       case RDIV_EXPR:
> +         /* True if both operands are nonnegative.  */
> +         op0 = gimple_assign_rhs1 (def_stmt);
> +         op1 = gimple_assign_rhs2 (def_stmt);
> +         return (gimple_val_nonnegative_real_p (op0)
> +                 && gimple_val_nonnegative_real_p (op1));
> +
> +       case MAX_EXPR:
> +         /* True if either operand is nonnegative.  */
> +         op0 = gimple_assign_rhs1 (def_stmt);
> +         op1 = gimple_assign_rhs2 (def_stmt);
> +         return (gimple_val_nonnegative_real_p (op0)
> +                 || gimple_val_nonnegative_real_p (op1));
> +
> +       case MULT_EXPR:
> +         /* True if the two operands are identical (since we are
> +            restricted to floating-point inputs), or if both operands
> +            are nonnegative.  */
> +         op0 = gimple_assign_rhs1 (def_stmt);
> +         op1 = gimple_assign_rhs2 (def_stmt);
> +
> +         if (operand_equal_p (op0, op1, 0))
> +           return true;
> +
> +         if (TREE_CODE (op0) == SSA_NAME
> +             && TREE_CODE (op1) == SSA_NAME
> +             && SSA_NAME_VAR (op0) == SSA_NAME_VAR (op1)
> +             && SSA_NAME_VERSION (op0) == SSA_NAME_VERSION (op1))
> +           return true;

That case is covered by operand_equal_p already.

> +
> +         /* FIXME: Could we miss cases where op0 and op1 are different
> +            SSA_NAMES whose defining values are equivalent, or one is
> +            an SSA_NAME and the other is an equivalent non-SSA_NAME?
> +            These cases likely don't constitute a great loss in any
> +            event.  */

Well, we can surely assume value-numbering has done its job.

> +         return (gimple_val_nonnegative_real_p (op0)
> +                 && gimple_val_nonnegative_real_p (op1));
> +
> +       default:
> +         return false;
> +       }
> +    }
> +  else if (is_gimple_call (def_stmt))
> +    {
> +      tree fndecl = gimple_call_fndecl (def_stmt);
> +      if (fndecl
> +         && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> +       {
> +         tree arg0, arg1;
> +
> +         switch (DECL_FUNCTION_CODE (fndecl))
> +           {
> +           CASE_FLT_FN (BUILT_IN_ACOS):
> +           CASE_FLT_FN (BUILT_IN_ACOSH):
> +           CASE_FLT_FN (BUILT_IN_CABS):
> +           CASE_FLT_FN (BUILT_IN_COSH):
> +           CASE_FLT_FN (BUILT_IN_ERFC):
> +           CASE_FLT_FN (BUILT_IN_EXP):
> +           CASE_FLT_FN (BUILT_IN_EXP10):
> +           CASE_FLT_FN (BUILT_IN_EXP2):
> +           CASE_FLT_FN (BUILT_IN_FABS):
> +           CASE_FLT_FN (BUILT_IN_FDIM):
> +           CASE_FLT_FN (BUILT_IN_HYPOT):
> +           CASE_FLT_FN (BUILT_IN_POW10):
> +             return true;
> +
> +            CASE_FLT_FN (BUILT_IN_SQRT):
> +             /* sqrt(-0.0) is -0.0, and sqrt is not defined over other
> +                nonnegative inputs.  */
> +             if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (val))))
> +               return true;
> +
> +             arg0 = gimple_call_arg (def_stmt, 0);
> +             return gimple_val_nonnegative_real_p (arg0);
> +
> +           CASE_FLT_FN (BUILT_IN_ASINH):
> +           CASE_FLT_FN (BUILT_IN_ATAN):
> +           CASE_FLT_FN (BUILT_IN_ATANH):
> +           CASE_FLT_FN (BUILT_IN_CBRT):
> +           CASE_FLT_FN (BUILT_IN_CEIL):
> +           CASE_FLT_FN (BUILT_IN_ERF):
> +           CASE_FLT_FN (BUILT_IN_EXPM1):
> +           CASE_FLT_FN (BUILT_IN_FLOOR):
> +           CASE_FLT_FN (BUILT_IN_FMOD):
> +           CASE_FLT_FN (BUILT_IN_FREXP):
> +           CASE_FLT_FN (BUILT_IN_LCEIL):
> +           CASE_FLT_FN (BUILT_IN_LDEXP):
> +           CASE_FLT_FN (BUILT_IN_LFLOOR):
> +           CASE_FLT_FN (BUILT_IN_LLCEIL):
> +           CASE_FLT_FN (BUILT_IN_LLFLOOR):
> +           CASE_FLT_FN (BUILT_IN_LLRINT):
> +           CASE_FLT_FN (BUILT_IN_LLROUND):
> +           CASE_FLT_FN (BUILT_IN_LRINT):
> +           CASE_FLT_FN (BUILT_IN_LROUND):
> +           CASE_FLT_FN (BUILT_IN_MODF):
> +           CASE_FLT_FN (BUILT_IN_NEARBYINT):
> +           CASE_FLT_FN (BUILT_IN_RINT):
> +           CASE_FLT_FN (BUILT_IN_ROUND):
> +           CASE_FLT_FN (BUILT_IN_SCALB):
> +           CASE_FLT_FN (BUILT_IN_SCALBLN):
> +           CASE_FLT_FN (BUILT_IN_SCALBN):
> +           CASE_FLT_FN (BUILT_IN_SIGNBIT):
> +           CASE_FLT_FN (BUILT_IN_SIGNIFICAND):
> +           CASE_FLT_FN (BUILT_IN_SINH):
> +           CASE_FLT_FN (BUILT_IN_TANH):
> +           CASE_FLT_FN (BUILT_IN_TRUNC):
> +             /* True if the first argument is nonnegative.  */
> +             arg0 = gimple_call_arg (def_stmt, 0);
> +             return gimple_val_nonnegative_real_p (arg0);
> +
> +           CASE_FLT_FN (BUILT_IN_FMAX):
> +             /* True if either argument is nonnegative.  */
> +             arg0 = gimple_call_arg (def_stmt, 0);
> +             arg1 = gimple_call_arg (def_stmt, 1);
> +             return (gimple_val_nonnegative_real_p (arg0)
> +                     || gimple_val_nonnegative_real_p (arg1));
> +
> +           CASE_FLT_FN (BUILT_IN_FMIN):
> +             /* True if both arguments are nonnegative.  */
> +             arg0 = gimple_call_arg (def_stmt, 0);
> +             arg1 = gimple_call_arg (def_stmt, 1);
> +             return (gimple_val_nonnegative_real_p (arg0)
> +                     && gimple_val_nonnegative_real_p (arg1));
> +
> +           CASE_FLT_FN (BUILT_IN_COPYSIGN):
> +             /* True if the second argument is nonnegative.  */
> +             arg1 = gimple_call_arg (def_stmt, 1);
> +             return gimple_val_nonnegative_real_p (arg1);
> +
> +           CASE_FLT_FN (BUILT_IN_POWI):
> +             /* True if the first argument is nonnegative or the
> +                second argument is an even integer.  */
> +             arg0 = gimple_call_arg (def_stmt, 0);
> +             arg1 = gimple_call_arg (def_stmt, 1);
> +
> +             if (TREE_CODE (arg1) == INTEGER_CST
> +                 && (TREE_INT_CST_LOW (arg1) & 1) == 0)
> +               return true;
> +
> +             return gimple_val_nonnegative_real_p (arg0);
> +
> +           CASE_FLT_FN (BUILT_IN_POW):
> +             /* True if the first argument is nonnegative or the
> +                second argument is an even integer-valued real.  */
> +             arg0 = gimple_call_arg (def_stmt, 0);
> +             arg1 = gimple_call_arg (def_stmt, 1);
> +
> +             if (TREE_CODE (arg1) == REAL_CST)
> +               {
> +                 REAL_VALUE_TYPE c;
> +                 HOST_WIDE_INT n;
> +
> +                 c = TREE_REAL_CST (arg1);
> +                 n = real_to_integer (&c);
> +
> +                 if ((n & 1) == 0)
> +                   {
> +                     REAL_VALUE_TYPE cint;
> +                     real_from_integer (&cint, VOIDmode, n, n < 0 ? -1 : 0, 0);
> +                     if (real_identical (&c, &cint))
> +                       return true;
> +                   }
> +               }
> +
> +             return gimple_val_nonnegative_real_p (arg0);
> +
> +           default:
> +             return false;
> +           }
> +       }
> +    }
> +
> +  return false;
> +}
> Index: gcc/gimple.h
> ===================================================================
> --- gcc/gimple.h        (revision 174535)
> +++ gcc/gimple.h        (working copy)
> @@ -4988,4 +4988,5 @@ extern tree maybe_fold_and_comparisons (enum tree_
>  extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,
>                                       enum tree_code, tree, tree);
>
> +bool gimple_val_nonnegative_real_p (tree);
>  #endif  /* GCC_GIMPLE_H */
>
>
>
William J. Schmidt - June 6, 2011, 12:12 p.m.
On Mon, 2011-06-06 at 13:00 +0200, Richard Guenther wrote:
> On Wed, Jun 1, 2011 at 9:27 PM, William J. Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:

<snip>

> > +/* Return true iff VAL is a gimple expression that is known to be
> > +   non-negative.  Restricted to floating-point inputs.  When changing
> > +   this function, review fold-const.c:tree_expr_nonnegative_p to see
> > +   whether similar changes are required.  */
> > +
> > +bool
> > +gimple_val_nonnegative_real_p (tree val)
> > +{
> > +  gimple def_stmt;
> > +
> > +  /* Use existing logic for non-gimple trees.  */
> > +  if (tree_expr_nonnegative_p (val))
> > +    return true;
> > +
> > +  if (TREE_CODE (val) != SSA_NAME)
> > +    return false;
> > +
> > +  def_stmt = SSA_NAME_DEF_STMT (val);
> > +
> > +  if (is_gimple_assign (def_stmt))
> > +    {
> > +      tree op0, op1;
> > +
> > +      /* If this is just a copy between SSA names, check the RHS.  */
> > +      if (gimple_assign_ssa_name_copy_p (def_stmt))
> > +       {
> > +         op0 = gimple_assign_rhs1 (def_stmt);
> > +         return gimple_val_nonnegative_real_p (op0);
> > +       }
> 
> If handled then do so as SSA_NAME: case below.
> 
> > +      switch (gimple_assign_rhs_code (def_stmt))
> > +       {
> > +       case ABS_EXPR:
> > +         /* Always true for floating-point operands.  */
> > +         return true;
> 
> You don't verify anywhere that the input is FP.
> 
> As the depth of the expression we look at is unbound it is probably
> easy to construct a testcase that exhibits quadratic compile-time
> behavior like pow(pow(pow(pow(...,0.5), 0.5), 0.5), 0.5).  I originally
> thought of just looking at the immediate defining statement but
> never at its operands (simply return false when only the operand
> would tell).  And I still think that is the way to go and should still
> catch 99% of the useful cases.
> 
> As for the grand masterplan we probably should eventually drive
> the builtin-folding by information provided by a SSA or DOM propagation
> engine (see tree-complex.c for example).  That would avoid the
> quadratic-ness.
> 
> So, please prune any recursion.

OK.  I misunderstood your intent; I thought you had provided a skeleton
and wanted me to fill in the details to match the corresponding tree
interface.  I understand the concern and will remove the recursion.  If
we find we're missing cases, it would be simple enough to provide
limited-depth recursion.

> 
> Thanks,
> Richard.
> 
> > +       case NOP_EXPR:
> > +       case CONVERT_EXPR:
> > +         /* True if the first operand is a nonnegative real.  */
> > +         op0 = gimple_assign_rhs1 (def_stmt);
> > +         return (TREE_CODE (TREE_TYPE (op0)) == REAL_TYPE
> > +                 && gimple_val_nonnegative_real_p (op0));
> > +
> > +       case PLUS_EXPR:
> > +       case MIN_EXPR:
> > +       case RDIV_EXPR:
> > +         /* True if both operands are nonnegative.  */
> > +         op0 = gimple_assign_rhs1 (def_stmt);
> > +         op1 = gimple_assign_rhs2 (def_stmt);
> > +         return (gimple_val_nonnegative_real_p (op0)
> > +                 && gimple_val_nonnegative_real_p (op1));
> > +
> > +       case MAX_EXPR:
> > +         /* True if either operand is nonnegative.  */
> > +         op0 = gimple_assign_rhs1 (def_stmt);
> > +         op1 = gimple_assign_rhs2 (def_stmt);
> > +         return (gimple_val_nonnegative_real_p (op0)
> > +                 || gimple_val_nonnegative_real_p (op1));
> > +
> > +       case MULT_EXPR:
> > +         /* True if the two operands are identical (since we are
> > +            restricted to floating-point inputs), or if both operands
> > +            are nonnegative.  */
> > +         op0 = gimple_assign_rhs1 (def_stmt);
> > +         op1 = gimple_assign_rhs2 (def_stmt);
> > +
> > +         if (operand_equal_p (op0, op1, 0))
> > +           return true;
> > +
> > +         if (TREE_CODE (op0) == SSA_NAME
> > +             && TREE_CODE (op1) == SSA_NAME
> > +             && SSA_NAME_VAR (op0) == SSA_NAME_VAR (op1)
> > +             && SSA_NAME_VERSION (op0) == SSA_NAME_VERSION (op1))
> > +           return true;
> 
> That case is covered by operand_equal_p already.

I don't believe it is, though perhaps it should be.  I didn't see any
handling for SSA_NAME or tcc_exceptional, and the default just returns
false, so I added this logic.  Did I miss something subtle?

Thanks,
Bill
Richard Guenther - June 6, 2011, 2:26 p.m.
On Mon, Jun 6, 2011 at 2:12 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> On Mon, 2011-06-06 at 13:00 +0200, Richard Guenther wrote:
>> On Wed, Jun 1, 2011 at 9:27 PM, William J. Schmidt
>> <wschmidt@linux.vnet.ibm.com> wrote:
>
> <snip>
>
>> > +/* Return true iff VAL is a gimple expression that is known to be
>> > +   non-negative.  Restricted to floating-point inputs.  When changing
>> > +   this function, review fold-const.c:tree_expr_nonnegative_p to see
>> > +   whether similar changes are required.  */
>> > +
>> > +bool
>> > +gimple_val_nonnegative_real_p (tree val)
>> > +{
>> > +  gimple def_stmt;
>> > +
>> > +  /* Use existing logic for non-gimple trees.  */
>> > +  if (tree_expr_nonnegative_p (val))
>> > +    return true;
>> > +
>> > +  if (TREE_CODE (val) != SSA_NAME)
>> > +    return false;
>> > +
>> > +  def_stmt = SSA_NAME_DEF_STMT (val);
>> > +
>> > +  if (is_gimple_assign (def_stmt))
>> > +    {
>> > +      tree op0, op1;
>> > +
>> > +      /* If this is just a copy between SSA names, check the RHS.  */
>> > +      if (gimple_assign_ssa_name_copy_p (def_stmt))
>> > +       {
>> > +         op0 = gimple_assign_rhs1 (def_stmt);
>> > +         return gimple_val_nonnegative_real_p (op0);
>> > +       }
>>
>> If handled then do so as SSA_NAME: case below.
>>
>> > +      switch (gimple_assign_rhs_code (def_stmt))
>> > +       {
>> > +       case ABS_EXPR:
>> > +         /* Always true for floating-point operands.  */
>> > +         return true;
>>
>> You don't verify anywhere that the input is FP.
>>
>> As the depth of the expression we look at is unbound it is probably
>> easy to construct a testcase that exhibits quadratic compile-time
>> behavior like pow(pow(pow(pow(...,0.5), 0.5), 0.5), 0.5).  I originally
>> thought of just looking at the immediate defining statement but
>> never at its operands (simply return false when only the operand
>> would tell).  And I still think that is the way to go and should still
>> catch 99% of the useful cases.
>>
>> As for the grand masterplan we probably should eventually drive
>> the builtin-folding by information provided by a SSA or DOM propagation
>> engine (see tree-complex.c for example).  That would avoid the
>> quadratic-ness.
>>
>> So, please prune any recursion.
>
> OK.  I misunderstood your intent; I thought you had provided a skeleton
> and wanted me to fill in the details to match the corresponding tree
> interface.

Sorry for being unclear.

>  I understand the concern and will remove the recursion.  If
> we find we're missing cases, it would be simple enough to provide
> limited-depth recursion.

Indeed.

>>
>> Thanks,
>> Richard.
>>
>> > +       case NOP_EXPR:
>> > +       case CONVERT_EXPR:
>> > +         /* True if the first operand is a nonnegative real.  */
>> > +         op0 = gimple_assign_rhs1 (def_stmt);
>> > +         return (TREE_CODE (TREE_TYPE (op0)) == REAL_TYPE
>> > +                 && gimple_val_nonnegative_real_p (op0));
>> > +
>> > +       case PLUS_EXPR:
>> > +       case MIN_EXPR:
>> > +       case RDIV_EXPR:
>> > +         /* True if both operands are nonnegative.  */
>> > +         op0 = gimple_assign_rhs1 (def_stmt);
>> > +         op1 = gimple_assign_rhs2 (def_stmt);
>> > +         return (gimple_val_nonnegative_real_p (op0)
>> > +                 && gimple_val_nonnegative_real_p (op1));
>> > +
>> > +       case MAX_EXPR:
>> > +         /* True if either operand is nonnegative.  */
>> > +         op0 = gimple_assign_rhs1 (def_stmt);
>> > +         op1 = gimple_assign_rhs2 (def_stmt);
>> > +         return (gimple_val_nonnegative_real_p (op0)
>> > +                 || gimple_val_nonnegative_real_p (op1));
>> > +
>> > +       case MULT_EXPR:
>> > +         /* True if the two operands are identical (since we are
>> > +            restricted to floating-point inputs), or if both operands
>> > +            are nonnegative.  */
>> > +         op0 = gimple_assign_rhs1 (def_stmt);
>> > +         op1 = gimple_assign_rhs2 (def_stmt);
>> > +
>> > +         if (operand_equal_p (op0, op1, 0))
>> > +           return true;
>> > +
>> > +         if (TREE_CODE (op0) == SSA_NAME
>> > +             && TREE_CODE (op1) == SSA_NAME
>> > +             && SSA_NAME_VAR (op0) == SSA_NAME_VAR (op1)
>> > +             && SSA_NAME_VERSION (op0) == SSA_NAME_VERSION (op1))
>> > +           return true;
>>
>> That case is covered by operand_equal_p already.
>
> I don't believe it is, though perhaps it should be.  I didn't see any
> handling for SSA_NAME or tcc_exceptional, and the default just returns
> false, so I added this logic.  Did I miss something subtle?

  if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST)
      && (TREE_CODE (arg0) == SAVE_EXPR
          || (flags & OEP_CONSTANT_ADDRESS_OF)
          || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1))))
    return 1;

should return true.  SSA_NAMEs are shared trees, so your code
is, simplified

   if (op0 == op1)

which you could pre-pend to the operand-equal check to make it cheaper
in the common case.  Thus,

  if (op0 == op1
     || operand_equal_p (...))
    return true;

Richard.


> Thanks,
> Bill
>
>
>

Patch

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c	(revision 174535)
+++ gcc/tree-ssa-math-opts.c	(working copy)
@@ -1172,13 +1172,7 @@  gimple_expand_builtin_pow (gimple_stmt_iterator *g
 
   if (flag_unsafe_math_optimizations
       && cbrtfn
-      /* FIXME: The following line was originally
-	 && (tree_expr_nonnegative_p (arg0) || !HONOR_NANS (mode)),
-	 but since arg0 is a gimple value, the first predicate
-	 will always return false.  It needs to be replaced with a
-	 call to a similar gimple_val_nonnegative_p function to be
-         added in gimple-fold.c.  */
-      && !HONOR_NANS (mode)
+      && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
       && REAL_VALUES_EQUAL (c, dconst1_3))
     return build_and_insert_call (gsi, loc, &target, cbrtfn, arg0);
   
@@ -1190,13 +1184,7 @@  gimple_expand_builtin_pow (gimple_stmt_iterator *g
   if (flag_unsafe_math_optimizations
       && sqrtfn
       && cbrtfn
-      /* FIXME: The following line was originally
-	 && (tree_expr_nonnegative_p (arg0) || !HONOR_NANS (mode)),
-	 but since arg0 is a gimple value, the first predicate
-	 will always return false.  It needs to be replaced with a
-	 call to a similar gimple_val_nonnegative_p function to be
-         added in gimple-fold.c.  */
-      && !HONOR_NANS (mode)
+      && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
       && optimize_function_for_speed_p (cfun)
       && hw_sqrt_exists
       && REAL_VALUES_EQUAL (c, dconst1_6))
@@ -1270,13 +1258,7 @@  gimple_expand_builtin_pow (gimple_stmt_iterator *g
 
   if (flag_unsafe_math_optimizations
       && cbrtfn
-      /* FIXME: The following line was originally
-	 && (tree_expr_nonnegative_p (arg0) || !HONOR_NANS (mode)),
-	 but since arg0 is a gimple value, the first predicate
-	 will always return false.  It needs to be replaced with a
-	 call to a similar gimple_val_nonnegative_p function to be
-         added in gimple-fold.c.  */
-      && !HONOR_NANS (mode)
+      && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
       && real_identical (&c2, &c)
       && optimize_function_for_speed_p (cfun)
       && powi_cost (n / 3) <= POWI_MAX_MULTS)
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 174535)
+++ gcc/gimple-fold.c	(working copy)
@@ -3433,3 +3433,224 @@  fold_const_aggregate_ref (tree t)
 {
   return fold_const_aggregate_ref_1 (t, NULL);
 }
+
+/* Return true iff VAL is a gimple expression that is known to be
+   non-negative.  Restricted to floating-point inputs.  When changing
+   this function, review fold-const.c:tree_expr_nonnegative_p to see
+   whether similar changes are required.  */
+
+bool
+gimple_val_nonnegative_real_p (tree val)
+{
+  gimple def_stmt;
+
+  /* Use existing logic for non-gimple trees.  */
+  if (tree_expr_nonnegative_p (val))
+    return true;
+
+  if (TREE_CODE (val) != SSA_NAME)
+    return false;
+
+  def_stmt = SSA_NAME_DEF_STMT (val);
+
+  if (is_gimple_assign (def_stmt))
+    {
+      tree op0, op1;
+
+      /* If this is just a copy between SSA names, check the RHS.  */
+      if (gimple_assign_ssa_name_copy_p (def_stmt))
+	{
+	  op0 = gimple_assign_rhs1 (def_stmt);
+	  return gimple_val_nonnegative_real_p (op0);
+	}
+
+      switch (gimple_assign_rhs_code (def_stmt))
+	{
+	case ABS_EXPR:
+	  /* Always true for floating-point operands.  */
+	  return true;
+
+	case NOP_EXPR:
+	case CONVERT_EXPR:
+	  /* True if the first operand is a nonnegative real.  */
+	  op0 = gimple_assign_rhs1 (def_stmt);
+	  return (TREE_CODE (TREE_TYPE (op0)) == REAL_TYPE
+		  && gimple_val_nonnegative_real_p (op0));
+
+	case PLUS_EXPR:
+	case MIN_EXPR:
+	case RDIV_EXPR:
+	  /* True if both operands are nonnegative.  */
+	  op0 = gimple_assign_rhs1 (def_stmt);
+	  op1 = gimple_assign_rhs2 (def_stmt);
+	  return (gimple_val_nonnegative_real_p (op0)
+		  && gimple_val_nonnegative_real_p (op1));
+
+	case MAX_EXPR:
+	  /* True if either operand is nonnegative.  */
+	  op0 = gimple_assign_rhs1 (def_stmt);
+	  op1 = gimple_assign_rhs2 (def_stmt);
+	  return (gimple_val_nonnegative_real_p (op0)
+		  || gimple_val_nonnegative_real_p (op1));
+
+	case MULT_EXPR:
+	  /* True if the two operands are identical (since we are
+	     restricted to floating-point inputs), or if both operands
+	     are nonnegative.  */
+	  op0 = gimple_assign_rhs1 (def_stmt);
+	  op1 = gimple_assign_rhs2 (def_stmt);
+
+	  if (operand_equal_p (op0, op1, 0))
+	    return true;
+
+	  if (TREE_CODE (op0) == SSA_NAME
+	      && TREE_CODE (op1) == SSA_NAME
+	      && SSA_NAME_VAR (op0) == SSA_NAME_VAR (op1)
+	      && SSA_NAME_VERSION (op0) == SSA_NAME_VERSION (op1))
+	    return true;
+
+	  /* FIXME: Could we miss cases where op0 and op1 are different
+	     SSA_NAMES whose defining values are equivalent, or one is
+	     an SSA_NAME and the other is an equivalent non-SSA_NAME?
+	     These cases likely don't constitute a great loss in any
+	     event.  */
+
+	  return (gimple_val_nonnegative_real_p (op0)
+		  && gimple_val_nonnegative_real_p (op1));
+
+	default:
+	  return false;
+	}
+    }
+  else if (is_gimple_call (def_stmt))
+    {
+      tree fndecl = gimple_call_fndecl (def_stmt);
+      if (fndecl
+	  && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+	{
+	  tree arg0, arg1;
+
+	  switch (DECL_FUNCTION_CODE (fndecl))
+	    {
+	    CASE_FLT_FN (BUILT_IN_ACOS):
+	    CASE_FLT_FN (BUILT_IN_ACOSH):
+	    CASE_FLT_FN (BUILT_IN_CABS):
+	    CASE_FLT_FN (BUILT_IN_COSH):
+	    CASE_FLT_FN (BUILT_IN_ERFC):
+	    CASE_FLT_FN (BUILT_IN_EXP):
+	    CASE_FLT_FN (BUILT_IN_EXP10):
+	    CASE_FLT_FN (BUILT_IN_EXP2):
+	    CASE_FLT_FN (BUILT_IN_FABS):
+	    CASE_FLT_FN (BUILT_IN_FDIM):
+	    CASE_FLT_FN (BUILT_IN_HYPOT):
+	    CASE_FLT_FN (BUILT_IN_POW10):
+	      return true;
+
+            CASE_FLT_FN (BUILT_IN_SQRT):
+	      /* sqrt(-0.0) is -0.0, and sqrt is not defined over other
+		 nonnegative inputs.  */
+	      if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (val))))
+		return true;
+
+	      arg0 = gimple_call_arg (def_stmt, 0);
+	      return gimple_val_nonnegative_real_p (arg0);
+
+	    CASE_FLT_FN (BUILT_IN_ASINH):
+	    CASE_FLT_FN (BUILT_IN_ATAN):
+	    CASE_FLT_FN (BUILT_IN_ATANH):
+	    CASE_FLT_FN (BUILT_IN_CBRT):
+	    CASE_FLT_FN (BUILT_IN_CEIL):
+	    CASE_FLT_FN (BUILT_IN_ERF):
+	    CASE_FLT_FN (BUILT_IN_EXPM1):
+	    CASE_FLT_FN (BUILT_IN_FLOOR):
+	    CASE_FLT_FN (BUILT_IN_FMOD):
+	    CASE_FLT_FN (BUILT_IN_FREXP):
+	    CASE_FLT_FN (BUILT_IN_LCEIL):
+	    CASE_FLT_FN (BUILT_IN_LDEXP):
+	    CASE_FLT_FN (BUILT_IN_LFLOOR):
+	    CASE_FLT_FN (BUILT_IN_LLCEIL):
+	    CASE_FLT_FN (BUILT_IN_LLFLOOR):
+	    CASE_FLT_FN (BUILT_IN_LLRINT):
+	    CASE_FLT_FN (BUILT_IN_LLROUND):
+	    CASE_FLT_FN (BUILT_IN_LRINT):
+	    CASE_FLT_FN (BUILT_IN_LROUND):
+	    CASE_FLT_FN (BUILT_IN_MODF):
+	    CASE_FLT_FN (BUILT_IN_NEARBYINT):
+	    CASE_FLT_FN (BUILT_IN_RINT):
+	    CASE_FLT_FN (BUILT_IN_ROUND):
+	    CASE_FLT_FN (BUILT_IN_SCALB):
+	    CASE_FLT_FN (BUILT_IN_SCALBLN):
+	    CASE_FLT_FN (BUILT_IN_SCALBN):
+	    CASE_FLT_FN (BUILT_IN_SIGNBIT):
+	    CASE_FLT_FN (BUILT_IN_SIGNIFICAND):
+	    CASE_FLT_FN (BUILT_IN_SINH):
+	    CASE_FLT_FN (BUILT_IN_TANH):
+	    CASE_FLT_FN (BUILT_IN_TRUNC):
+	      /* True if the first argument is nonnegative.  */
+	      arg0 = gimple_call_arg (def_stmt, 0);
+	      return gimple_val_nonnegative_real_p (arg0);
+
+	    CASE_FLT_FN (BUILT_IN_FMAX):
+	      /* True if either argument is nonnegative.  */
+	      arg0 = gimple_call_arg (def_stmt, 0);
+	      arg1 = gimple_call_arg (def_stmt, 1);
+	      return (gimple_val_nonnegative_real_p (arg0)
+		      || gimple_val_nonnegative_real_p (arg1));
+
+	    CASE_FLT_FN (BUILT_IN_FMIN):
+	      /* True if both arguments are nonnegative.  */
+	      arg0 = gimple_call_arg (def_stmt, 0);
+	      arg1 = gimple_call_arg (def_stmt, 1);
+	      return (gimple_val_nonnegative_real_p (arg0)
+		      && gimple_val_nonnegative_real_p (arg1));
+
+	    CASE_FLT_FN (BUILT_IN_COPYSIGN):
+	      /* True if the second argument is nonnegative.  */
+	      arg1 = gimple_call_arg (def_stmt, 1);
+	      return gimple_val_nonnegative_real_p (arg1);
+
+	    CASE_FLT_FN (BUILT_IN_POWI):
+	      /* True if the first argument is nonnegative or the
+		 second argument is an even integer.  */
+	      arg0 = gimple_call_arg (def_stmt, 0);
+	      arg1 = gimple_call_arg (def_stmt, 1);
+
+	      if (TREE_CODE (arg1) == INTEGER_CST
+		  && (TREE_INT_CST_LOW (arg1) & 1) == 0)
+		return true;
+
+	      return gimple_val_nonnegative_real_p (arg0);
+	      
+	    CASE_FLT_FN (BUILT_IN_POW):
+	      /* True if the first argument is nonnegative or the
+		 second argument is an even integer-valued real.  */
+	      arg0 = gimple_call_arg (def_stmt, 0);
+	      arg1 = gimple_call_arg (def_stmt, 1);
+
+	      if (TREE_CODE (arg1) == REAL_CST)
+		{
+		  REAL_VALUE_TYPE c;
+		  HOST_WIDE_INT n;
+
+		  c = TREE_REAL_CST (arg1);
+		  n = real_to_integer (&c);
+
+		  if ((n & 1) == 0)
+		    {
+		      REAL_VALUE_TYPE cint;
+		      real_from_integer (&cint, VOIDmode, n, n < 0 ? -1 : 0, 0);
+		      if (real_identical (&c, &cint))
+			return true;
+		    }
+		}
+
+	      return gimple_val_nonnegative_real_p (arg0);
+
+	    default:
+	      return false;
+	    }
+	}
+    }
+
+  return false;
+}
Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h	(revision 174535)
+++ gcc/gimple.h	(working copy)
@@ -4988,4 +4988,5 @@  extern tree maybe_fold_and_comparisons (enum tree_
 extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,
 				       enum tree_code, tree, tree);
 
+bool gimple_val_nonnegative_real_p (tree);
 #endif  /* GCC_GIMPLE_H */