Move sqrt and cbrt simplifications to match.pd

Richard Sandiford <richard.sandiford@arm.com> writes:
> Christophe Lyon <christophe.lyon@linaro.org> writes:
>> On 8 October 2015 at 18:55, Richard Sandiford
>> <richard.sandiford@arm.com> wrote:
>>> Marc Glisse <marc.glisse@inria.fr> writes:
>>>> On Mon, 5 Oct 2015, Richard Sandiford wrote:
>>>>
>>>>> +  /* cbrt(sqrt(x)) -> pow(x,1/6).  */
>>>>> +  (simplify
>>>>> +   (sqrts (cbrts @0))
>>>>> +   (pows @0 { build_real_truncate (type, dconst<1, 6> ()); }))
>>>>> +  /* sqrt(cbrt(x)) -> pow(x,1/6).  */
>>>>> +  (simplify
>>>>> +   (cbrts (sqrts @0))
>>>>> +   (pows @0 { build_real_truncate (type, dconst<1, 6> ()); }))
>>>>
>>>> I think you swapped the comments (not that it matters).
>>>
>>> Thanks, fixed in the committed version.
>>>
>>> Richard
>>>
>> Hi Richard,
>>
>> Since you committed this patch, I've noticed that gcc.dg/builtins-10.c fails
>> on arm-none-linux-gnueabi targets (as opposed to arm-none-linux-gnueabihf).
>>
>> gcc.log shows:
>> /cchfHDHc.o: In function `test':
>> builtins-10.c:(.text+0x60): undefined reference to `link_error'
>> collect2: error: ld returned 1 exit status
>
> Looks like this is the same fold_strip_sign_ops problem that I was seeing
> with some WIP follow-on patches.  We don't fold pow(abs(x), 4) to pow(x, 4).

Here's the patch I'm testing.

Thanks,
Richard


gcc/
	* real.h (real_isinteger): Declare.
	* real.c (real_isinteger): New function.
	* match.pd: Simplify pow(|x|,y) and pow(-x,y) to pow(x,y)
	if y is an even integer.

On Fri, Oct 9, 2015 at 6:17 PM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> Richard Sandiford <richard.sandiford@arm.com> writes:
>> Christophe Lyon <christophe.lyon@linaro.org> writes:
>>> On 8 October 2015 at 18:55, Richard Sandiford
>>> <richard.sandiford@arm.com> wrote:
>>>> Marc Glisse <marc.glisse@inria.fr> writes:
>>>>> On Mon, 5 Oct 2015, Richard Sandiford wrote:
>>>>>
>>>>>> +  /* cbrt(sqrt(x)) -> pow(x,1/6).  */
>>>>>> +  (simplify
>>>>>> +   (sqrts (cbrts @0))
>>>>>> +   (pows @0 { build_real_truncate (type, dconst<1, 6> ()); }))
>>>>>> +  /* sqrt(cbrt(x)) -> pow(x,1/6).  */
>>>>>> +  (simplify
>>>>>> +   (cbrts (sqrts @0))
>>>>>> +   (pows @0 { build_real_truncate (type, dconst<1, 6> ()); }))
>>>>>
>>>>> I think you swapped the comments (not that it matters).
>>>>
>>>> Thanks, fixed in the committed version.
>>>>
>>>> Richard
>>>>
>>> Hi Richard,
>>>
>>> Since you committed this patch, I've noticed that gcc.dg/builtins-10.c fails
>>> on arm-none-linux-gnueabi targets (as opposed to arm-none-linux-gnueabihf).
>>>
>>> gcc.log shows:
>>> /cchfHDHc.o: In function `test':
>>> builtins-10.c:(.text+0x60): undefined reference to `link_error'
>>> collect2: error: ld returned 1 exit status
>>
>> Looks like this is the same fold_strip_sign_ops problem that I was seeing
>> with some WIP follow-on patches.  We don't fold pow(abs(x), 4) to pow(x, 4).
>
> Here's the patch I'm testing.

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * real.h (real_isinteger): Declare.
>         * real.c (real_isinteger): New function.
>         * match.pd: Simplify pow(|x|,y) and pow(-x,y) to pow(x,y)
>         if y is an even integer.
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index b87c436..67f9d54 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -309,12 +309,19 @@ along with GCC; see the file COPYING3.  If not see
>         && TYPE_OVERFLOW_UNDEFINED (type))
>     @0)))
>
> -/* Simplify cos (-x) -> cos (x).  */
>  (for op (negate abs)
> -(for coss (COS COSH)
> - (simplify
> -  (coss (op @0))
> -   (coss @0))))
> + /* Simplify cos(-x) and cos(|x|) -> cos(x).  Similarly for cosh.  */
> + (for coss (COS COSH)
> +  (simplify
> +   (coss (op @0))
> +    (coss @0)))
> + /* Simplify pow(-x, y) and pow(|x|,y) -> pow(x,y) if y is an even integer.  */
> + (for pows (POW)
> +  (simplify
> +   (pows (op @0) REAL_CST@1)
> +   (with { HOST_WIDE_INT n; }
> +    (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
> +     (pows @0 @1))))))
>
>  /* X % Y is smaller than Y.  */
>  (for cmp (lt ge)
> diff --git a/gcc/real.c b/gcc/real.c
> index f633ffd..85ac83d 100644
> --- a/gcc/real.c
> +++ b/gcc/real.c
> @@ -4997,6 +4997,24 @@ real_isinteger (const REAL_VALUE_TYPE *c, machine_mode mode)
>    return real_identical (c, &cint);
>  }
>
> +/* Check whether C is an integer that fits in a HOST_WIDE_INT,
> +   storing it in *INT_OUT if so.  */
> +
> +bool
> +real_isinteger (const REAL_VALUE_TYPE *c, HOST_WIDE_INT *int_out)
> +{
> +  REAL_VALUE_TYPE cint;
> +
> +  HOST_WIDE_INT n = real_to_integer (c);
> +  real_from_integer (&cint, VOIDmode, n, SIGNED);
> +  if (real_identical (c, &cint))
> +    {
> +      *int_out = n;
> +      return true;
> +    }
> +  return false;
> +}
> +
>  /* Write into BUF the maximum representable finite floating-point
>     number, (1 - b**-p) * b**emax for a given FP format FMT as a hex
>     float string.  LEN is the size of BUF, and the buffer must be large
> diff --git a/gcc/real.h b/gcc/real.h
> index 706859b..e65b526 100644
> --- a/gcc/real.h
> +++ b/gcc/real.h
> @@ -467,7 +467,8 @@ extern void real_round (REAL_VALUE_TYPE *, machine_mode,
>  extern void real_copysign (REAL_VALUE_TYPE *, const REAL_VALUE_TYPE *);
>
>  /* Check whether the real constant value given is an integer.  */
> -extern bool real_isinteger (const REAL_VALUE_TYPE *c, machine_mode mode);
> +extern bool real_isinteger (const REAL_VALUE_TYPE *, machine_mode);
> +extern bool real_isinteger (const REAL_VALUE_TYPE *, HOST_WIDE_INT *);
>
>  /* Write into BUF the maximum representable finite floating-point
>     number, (1 - b**-p) * b**emax for a given FP format FMT as a hex
>

On 9 October 2015 at 18:17, Richard Sandiford <richard.sandiford@arm.com> wrote:
> Richard Sandiford <richard.sandiford@arm.com> writes:
>> Christophe Lyon <christophe.lyon@linaro.org> writes:
>>> On 8 October 2015 at 18:55, Richard Sandiford
>>> <richard.sandiford@arm.com> wrote:
>>>> Marc Glisse <marc.glisse@inria.fr> writes:
>>>>> On Mon, 5 Oct 2015, Richard Sandiford wrote:
>>>>>
>>>>>> +  /* cbrt(sqrt(x)) -> pow(x,1/6).  */
>>>>>> +  (simplify
>>>>>> +   (sqrts (cbrts @0))
>>>>>> +   (pows @0 { build_real_truncate (type, dconst<1, 6> ()); }))
>>>>>> +  /* sqrt(cbrt(x)) -> pow(x,1/6).  */
>>>>>> +  (simplify
>>>>>> +   (cbrts (sqrts @0))
>>>>>> +   (pows @0 { build_real_truncate (type, dconst<1, 6> ()); }))
>>>>>
>>>>> I think you swapped the comments (not that it matters).
>>>>
>>>> Thanks, fixed in the committed version.
>>>>
>>>> Richard
>>>>
>>> Hi Richard,
>>>
>>> Since you committed this patch, I've noticed that gcc.dg/builtins-10.c fails
>>> on arm-none-linux-gnueabi targets (as opposed to arm-none-linux-gnueabihf).
>>>
>>> gcc.log shows:
>>> /cchfHDHc.o: In function `test':
>>> builtins-10.c:(.text+0x60): undefined reference to `link_error'
>>> collect2: error: ld returned 1 exit status
>>
>> Looks like this is the same fold_strip_sign_ops problem that I was seeing
>> with some WIP follow-on patches.  We don't fold pow(abs(x), 4) to pow(x, 4).
>
> Here's the patch I'm testing.
>
> Thanks,
> Richard
>
>
> gcc/
>         * real.h (real_isinteger): Declare.
>         * real.c (real_isinteger): New function.
>         * match.pd: Simplify pow(|x|,y) and pow(-x,y) to pow(x,y)
>         if y is an even integer.
>

This makes sense indeed. I was wondering why I didn't notice
regressions on arm-*hf targets:
are such optimizations caught in later passes for some targets?

> diff --git a/gcc/match.pd b/gcc/match.pd
> index b87c436..67f9d54 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -309,12 +309,19 @@ along with GCC; see the file COPYING3.  If not see
>         && TYPE_OVERFLOW_UNDEFINED (type))
>     @0)))
>
> -/* Simplify cos (-x) -> cos (x).  */
>  (for op (negate abs)
> -(for coss (COS COSH)
> - (simplify
> -  (coss (op @0))
> -   (coss @0))))
> + /* Simplify cos(-x) and cos(|x|) -> cos(x).  Similarly for cosh.  */
> + (for coss (COS COSH)
> +  (simplify
> +   (coss (op @0))
> +    (coss @0)))
> + /* Simplify pow(-x, y) and pow(|x|,y) -> pow(x,y) if y is an even integer.  */
> + (for pows (POW)
> +  (simplify
> +   (pows (op @0) REAL_CST@1)
> +   (with { HOST_WIDE_INT n; }
> +    (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
> +     (pows @0 @1))))))
>
>  /* X % Y is smaller than Y.  */
>  (for cmp (lt ge)
> diff --git a/gcc/real.c b/gcc/real.c
> index f633ffd..85ac83d 100644
> --- a/gcc/real.c
> +++ b/gcc/real.c
> @@ -4997,6 +4997,24 @@ real_isinteger (const REAL_VALUE_TYPE *c, machine_mode mode)
>    return real_identical (c, &cint);
>  }
>
> +/* Check whether C is an integer that fits in a HOST_WIDE_INT,
> +   storing it in *INT_OUT if so.  */
> +
> +bool
> +real_isinteger (const REAL_VALUE_TYPE *c, HOST_WIDE_INT *int_out)
> +{
> +  REAL_VALUE_TYPE cint;
> +
> +  HOST_WIDE_INT n = real_to_integer (c);
> +  real_from_integer (&cint, VOIDmode, n, SIGNED);
> +  if (real_identical (c, &cint))
> +    {
> +      *int_out = n;
> +      return true;
> +    }
> +  return false;
> +}
> +
>  /* Write into BUF the maximum representable finite floating-point
>     number, (1 - b**-p) * b**emax for a given FP format FMT as a hex
>     float string.  LEN is the size of BUF, and the buffer must be large
> diff --git a/gcc/real.h b/gcc/real.h
> index 706859b..e65b526 100644
> --- a/gcc/real.h
> +++ b/gcc/real.h
> @@ -467,7 +467,8 @@ extern void real_round (REAL_VALUE_TYPE *, machine_mode,
>  extern void real_copysign (REAL_VALUE_TYPE *, const REAL_VALUE_TYPE *);
>
>  /* Check whether the real constant value given is an integer.  */
> -extern bool real_isinteger (const REAL_VALUE_TYPE *c, machine_mode mode);
> +extern bool real_isinteger (const REAL_VALUE_TYPE *, machine_mode);
> +extern bool real_isinteger (const REAL_VALUE_TYPE *, HOST_WIDE_INT *);
>
>  /* Write into BUF the maximum representable finite floating-point
>     number, (1 - b**-p) * b**emax for a given FP format FMT as a hex
>