[v3] MATCH: Simplify `(trunc)copysign ((extend)x, CST)` to `copysign (x, -1.0/1.0)` [PR112472]

This patch simplify `(trunc)copysign ((extend)x, CST)` to `copysign (x, -1.0/1.0)`
depending on the sign of CST. Previously, it was simplified to `copysign (x, CST)`.
It can be optimized as the sign of the CST matters, not the value.

The patch also simplify `(trunc)abs (extend x)` to `abs (x)`.

	PR tree-optimization/112472

gcc/ChangeLog:

	* match.pd ((trunc)copysign ((extend)x, -CST) --> copysign (x, -1.0)): New pattern.
	((trunc)abs (extend x) --> abs (x)): New pattern.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/pr112472.c: New test.

Signed-off-by: Eikansh Gupta <quic_eikagupt@quicinc.com>
---
 gcc/match.pd                             | 25 +++++++++++++++++++-----
 gcc/testsuite/gcc.dg/tree-ssa/pr112472.c | 22 +++++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr112472.c

On Thu, Nov 14, 2024 at 11:59 AM Eikansh Gupta
<quic_eikagupt@quicinc.com> wrote:
>
> This patch simplify `(trunc)copysign ((extend)x, CST)` to `copysign (x, -1.0/1.0)`
> depending on the sign of CST. Previously, it was simplified to `copysign (x, CST)`.
> It can be optimized as the sign of the CST matters, not the value.
>
> The patch also simplify `(trunc)abs (extend x)` to `abs (x)`.

Please do not mix two different changes.

>         PR tree-optimization/112472
>
> gcc/ChangeLog:
>
>         * match.pd ((trunc)copysign ((extend)x, -CST) --> copysign (x, -1.0)): New pattern.
>         ((trunc)abs (extend x) --> abs (x)): New pattern.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/pr112472.c: New test.
>
> Signed-off-by: Eikansh Gupta <quic_eikagupt@quicinc.com>
> ---
>  gcc/match.pd                             | 25 +++++++++++++++++++-----
>  gcc/testsuite/gcc.dg/tree-ssa/pr112472.c | 22 +++++++++++++++++++++
>  2 files changed, 42 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr112472.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 00988241348..5b930beb418 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -8854,19 +8854,34 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>                                              type, OPTIMIZE_FOR_BOTH))
>         (tos @0))))
>
> -/* Simplify (trunc)copysign ((extend)x, (extend)y) to copysignf (x, y),
> -   x,y is float value, similar for _Float16/double.  */
> +/* Simplify (trunc)copysign ((extend)x, (extend)y) to copysignf (x, y) and
> +   simplify (trunc)copysign ((extend)x, CST) to copysign (x, -1.0/1.0).
> +   x,y is float value, similar for _Float16/double. */
>  (for copysigns (COPYSIGN_ALL)
>   (simplify
> -  (convert (copysigns (convert@2 @0) (convert @1)))
> +  (convert (copysigns (convert@2 @0) (convert2? @1)))

You want to capture convert2? with @3

>     (if (optimize
>         && !HONOR_SNANS (@2)
>         && types_match (type, TREE_TYPE (@0))
> -       && types_match (type, TREE_TYPE (@1))
>         && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@2))
>         && direct_internal_fn_supported_p (IFN_COPYSIGN,
>                                           type, OPTIMIZE_FOR_BOTH))
> -    (IFN_COPYSIGN @0 @1))))
> +     (if (TREE_CODE (@1) == REAL_CST)

and check TREE_CODE (@3) == REAL_CST, we might not always
fold a conversion of a FP constant.

> +      (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
> +       (IFN_COPYSIGN @0 { build_minus_one_cst (type); })
> +       (IFN_COPYSIGN @0 { build_one_cst (type); }))
> +      (if (types_match (type, TREE_TYPE (@1)))
> +       (IFN_COPYSIGN @0 @1))))))
> +
> +/* (trunc)abs (extend x) --> abs (x)
> +   x is a float value */
> +(simplify
> + (convert (abs (convert@1 @0)))
> +  (if (optimize
> +      && !HONOR_SNANS (@1)
> +      && types_match (type, TREE_TYPE (@0))
> +      && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@1)))
> +   (abs @0)))

This one is OK, but I don't see a testcase?  Please split it out to a
separate patch
and add one.

Richard.

>  (for froms (BUILT_IN_FMAF BUILT_IN_FMA BUILT_IN_FMAL)
>       tos (IFN_FMA IFN_FMA IFN_FMA)
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr112472.c b/gcc/testsuite/gcc.dg/tree-ssa/pr112472.c
> new file mode 100644
> index 00000000000..8f97278ffe8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr112472.c
> @@ -0,0 +1,22 @@
> +/* PR tree-optimization/109878 */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-optimized" } */
> +
> +/* Optimized to .COPYSIGN(a, -1.0e+0) */
> +float f(float a)
> +{
> +  return (float)__builtin_copysign(a, -3.0);
> +}
> +
> +/* This gets converted to (float) abs((double) a)
> +   With the patch it is optimized to abs(a) */
> +float f2(float a)
> +{
> +  return (float)__builtin_copysign(a, 5.0);
> +}
> +
> +/* { dg-final { scan-tree-dump-not "= __builtin_copysign" "optimized" } } */
> +/* { dg-final { scan-tree-dump-not " double " "optimized" { target ifn_copysign } } } */
> +/* { dg-final { scan-tree-dump-times ".COPYSIGN" 1 "optimized" { target ifn_copysign } } } */
> +/* { dg-final { scan-tree-dump-times "-1.0e\\+0" 1 "optimized" { target ifn_copysign } } } */
> +/* { dg-final { scan-tree-dump-times " ABS_EXPR " 1 "optimized" { target ifn_copysign } } } */
> --
> 2.17.1
>

> On Thu, Nov 14, 2024 at 11:59 AM Eikansh Gupta
> <quic_eikagupt@quicinc.com> wrote:
> >
> > This patch simplify `(trunc)copysign ((extend)x, CST)` to `copysign
> > (x, -1.0/1.0)` depending on the sign of CST. Previously, it was simplified to
> `copysign (x, CST)`.
> > It can be optimized as the sign of the CST matters, not the value.
> >
> > The patch also simplify `(trunc)abs (extend x)` to `abs (x)`.
> 
> Please do not mix two different changes.

The second change is needed because at match.pd:1198, copysign(x, +ve CST) is
converted to abs(x).

> 
> >         PR tree-optimization/112472
> >
> > gcc/ChangeLog:
> >
> >         * match.pd ((trunc)copysign ((extend)x, -CST) --> copysign (x, -1.0)):
> New pattern.
> >         ((trunc)abs (extend x) --> abs (x)): New pattern.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/tree-ssa/pr112472.c: New test.
> >
> > Signed-off-by: Eikansh Gupta <quic_eikagupt@quicinc.com>
> > ---
> >  gcc/match.pd                             | 25 +++++++++++++++++++-----
> >  gcc/testsuite/gcc.dg/tree-ssa/pr112472.c | 22 +++++++++++++++++++++
> >  2 files changed, 42 insertions(+), 5 deletions(-)  create mode 100644
> > gcc/testsuite/gcc.dg/tree-ssa/pr112472.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd index
> > 00988241348..5b930beb418 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -8854,19 +8854,34 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >                                              type, OPTIMIZE_FOR_BOTH))
> >         (tos @0))))
> >
> > -/* Simplify (trunc)copysign ((extend)x, (extend)y) to copysignf (x, y),
> > -   x,y is float value, similar for _Float16/double.  */
> > +/* Simplify (trunc)copysign ((extend)x, (extend)y) to copysignf (x, y) and
> > +   simplify (trunc)copysign ((extend)x, CST) to copysign (x, -1.0/1.0).
> > +   x,y is float value, similar for _Float16/double. */
> >  (for copysigns (COPYSIGN_ALL)
> >   (simplify
> > -  (convert (copysigns (convert@2 @0) (convert @1)))
> > +  (convert (copysigns (convert@2 @0) (convert2? @1)))
> 
> You want to capture convert2? with @3
> 
> >     (if (optimize
> >         && !HONOR_SNANS (@2)
> >         && types_match (type, TREE_TYPE (@0))
> > -       && types_match (type, TREE_TYPE (@1))
> >         && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@2))
> >         && direct_internal_fn_supported_p (IFN_COPYSIGN,
> >                                           type, OPTIMIZE_FOR_BOTH))
> > -    (IFN_COPYSIGN @0 @1))))
> > +     (if (TREE_CODE (@1) == REAL_CST)
> 
> and check TREE_CODE (@3) == REAL_CST, we might not always fold a
> conversion of a FP constant.

A basic question here. When do we not fold this type of conversion?

> > +      (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
> > +       (IFN_COPYSIGN @0 { build_minus_one_cst (type); })
> > +       (IFN_COPYSIGN @0 { build_one_cst (type); }))

Should I remove the above line as copysign(x, +ve CST) is converted to abs(x)?

> > +      (if (types_match (type, TREE_TYPE (@1)))
> > +       (IFN_COPYSIGN @0 @1))))))
> > +
> > +/* (trunc)abs (extend x) --> abs (x)
> > +   x is a float value */
> > +(simplify
> > + (convert (abs (convert@1 @0)))
> > +  (if (optimize
> > +      && !HONOR_SNANS (@1)
> > +      && types_match (type, TREE_TYPE (@0))
> > +      && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@1)))
> > +   (abs @0)))
> 
> This one is OK, but I don't see a testcase?  Please split it out to a separate patch
> and add one.

The second testcase is for this change. I will split it into separate patch.

> 
> Richard.

Regards,
Eikansh