This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fold comparisons between sqrt and zero
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, richard dot sandiford at arm dot com
- Date: Tue, 27 Oct 2015 12:28:18 +0100
- Subject: Re: Fold comparisons between sqrt and zero
- Authentication-results: sourceware.org; auth=none
- References: <871tcix97r dot fsf at e105548-lin dot cambridge dot arm dot com> <CAFiYyc3zfg6iJpxuLGOHXJ=1jqH3-WJxZ76GSQBoArsphkuCOQ at mail dot gmail dot com> <874mhcvf8f dot fsf_-_ at e105548-lin dot cambridge dot arm dot com>
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;
> +}
>