Fold comparisons between sqrt and zero

Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Oct 26, 2015 at 10:36 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> An upcoming patch adds a fold from hypot(x,x) to fabs(x)*sqrt(2).
>> This is unusual in that it could trigger in the gimplifier but would
>> require new SSA names to be created.  This patch makes sure that we
>> don't try to create new SSA names when we're not yet in SSA form.
>>
>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>> OK to install?
>
> Please use
>
>  if (gimple_in_ssa_p (cfun))
>   res = make_ssa_name (type);
>  else
>   res = create_tmp_reg (type);
>
> Ok with that change.

Thanks, committed in that form.  I retested the series on top of it and
the create_tmp_reg causes the later signbit patch (not yet committed)
to regress on:

  signbit(sqrt(x))

which is always 0 for -ffast-math.  The signbit fold first converts it to:

  sqrt(x) < 0

and whether we realise that this is false depends on a race between two
folders: the sqrt comparison folder, which wants to convert it to:

  x < 0*0

and the generic tree_expr_nonnegative_p rule for ... < 0, which would
give the hoped-for 0.

The sqrt code already handles comparisons with negative values specially,
so this patch simply extends that idea to comparisons with zero.

I don't really like patches like this since they'll probably help no
real code, but still...

Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
OK to install?

Thanks,
Richard


gcc/
	* match.pd: Handle sqrt(x) cmp 0 specially.

gcc/testsuite/
	* gcc.dg/torture/builtin-sqrt-cmp-1.c: New test.

On Tue, Oct 27, 2015 at 10:21 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, Oct 26, 2015 at 10:36 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> An upcoming patch adds a fold from hypot(x,x) to fabs(x)*sqrt(2).
>>> This is unusual in that it could trigger in the gimplifier but would
>>> require new SSA names to be created.  This patch makes sure that we
>>> don't try to create new SSA names when we're not yet in SSA form.
>>>
>>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>>> OK to install?
>>
>> Please use
>>
>>  if (gimple_in_ssa_p (cfun))
>>   res = make_ssa_name (type);
>>  else
>>   res = create_tmp_reg (type);
>>
>> Ok with that change.
>
> Thanks, committed in that form.  I retested the series on top of it and
> the create_tmp_reg causes the later signbit patch (not yet committed)
> to regress on:
>
>   signbit(sqrt(x))
>
> which is always 0 for -ffast-math.  The signbit fold first converts it to:
>
>   sqrt(x) < 0
>
> and whether we realise that this is false depends on a race between two
> folders: the sqrt comparison folder, which wants to convert it to:
>
>   x < 0*0
>
> and the generic tree_expr_nonnegative_p rule for ... < 0, which would
> give the hoped-for 0.
>
> The sqrt code already handles comparisons with negative values specially,
> so this patch simply extends that idea to comparisons with zero.
>
> I don't really like patches like this since they'll probably help no
> real code, but still...
>
> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
> OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * match.pd: Handle sqrt(x) cmp 0 specially.
>
> gcc/testsuite/
>         * gcc.dg/torture/builtin-sqrt-cmp-1.c: New test.
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 26491d2..b8e6b46 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1973,6 +1973,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         { constant_boolean_node (true, type); })
>         /* sqrt(x) > y is the same as x >= 0, if y is negative.  */
>         (ge @0 { build_real (TREE_TYPE (@0), dconst0); })))
> +     (if (real_equal (TREE_REAL_CST_PTR (@1), &dconst0))
> +      (switch
> +       /* sqrt(x) < 0 is always false.  */
> +       (if (cmp == LT_EXPR)
> +       { constant_boolean_node (false, type); })
> +       /* sqrt(x) >= 0 is always true if we don't care about NaNs.  */
> +       (if (cmp == GE_EXPR && !HONOR_NANS (@0))
> +       { constant_boolean_node (true, type); })
> +       /* sqrt(x) <= 0 -> x == 0.  */
> +       (if (cmp == LE_EXPR)
> +       (eq @0 @1))
> +       /* Otherwise sqrt(x) cmp 0 -> x cmp 0.  Here cmp can be >=, >,
> +          == or !=.  In the last case:
> +
> +           (sqrt(x) != 0) == (NaN != 0) == true == (x != 0)
> +
> +         if x is negative or NaN.  Due to -funsafe-math-optimizations,
> +         the results for other x follow from natural arithmetic.  */
> +       (cmp @0 @1)))
>       (if (cmp == GT_EXPR || cmp == GE_EXPR)
>        (with
>         {
> diff --git a/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c b/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c
> new file mode 100644
> index 0000000..3f4a708
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c
> @@ -0,0 +1,53 @@
> +/* { dg-do link } */
> +/* { dg-options "-ffast-math" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
> +
> +extern double sqrt (double);
> +extern float sqrtf (float);
> +extern long double sqrtl (long double);
> +
> +/* All references to link_error should go away at compile-time.  */
> +extern void link_error (void);
> +
> +#define TEST_ONE(SUFFIX, TYPE)                 \
> +  void __attribute__ ((noinline, noclone))     \
> +  test##SUFFIX (TYPE f, int *res)              \
> +  {                                            \
> +    TYPE sqrt_res = sqrt##SUFFIX (f);          \
> +    res[0] = sqrt_res < 0;                     \
> +    if (res[0])                                        \
> +      link_error ();                           \
> +    res[1] = sqrt_res <= 0;                    \
> +    if (res[1] != (f == 0))                    \
> +      link_error ();                           \
> +    res[2] = (sqrt_res == 0);                  \
> +    if (res[2] != (f == 0))                    \
> +      link_error ();                           \
> +    res[3] = (sqrt_res != 0);                  \
> +    if (res[3] != (f != 0))                    \
> +      link_error ();                           \
> +    res[4] = (sqrt_res > 0);                   \
> +    if (res[4] != (f > 0))                     \
> +      link_error ();                           \
> +    res[5] = (sqrt_res >= 0);                  \
> +    if (!res[5])                               \
> +      link_error ();                           \
> +  }
> +
> +volatile float f;
> +volatile double d;
> +volatile long double ld;
> +
> +TEST_ONE (f, float)
> +TEST_ONE (, double)
> +TEST_ONE (l, long double)
> +
> +int
> +main ()
> +{
> +  int res[6];
> +  testf (f, res);
> +  test (d, res);
> +  testl (ld, res);
> +  return 0;
> +}
>