diff mbox series

MATCH: Add pattern for `signbit(x) ? x : -x` into abs (and swapped)

Message ID 20230514031258.1542461-1-apinski@marvell.com
State Accepted
Headers
Series MATCH: Add pattern for `signbit(x) ? x : -x` into abs (and swapped) |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski May 14, 2023, 3:12 a.m. UTC 
  This adds a simple pattern to match.pd for `signbit(x) ? x : -x`
into abs<x>. This can be done for all types even ones that honor
signed zeros and NaNs because both signbit and - are considered
only looking at/touching the sign bit of those types and does
not trap either.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

	PR tree-optimization/109829

gcc/ChangeLog:

	* match.pd: Add pattern for `signbit(x) !=/== 0 ? x : -x`.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/abs-3.c: New test.
	* gcc.dg/tree-ssa/abs-4.c: New test.
---
 gcc/match.pd                          | 10 ++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/abs-3.c | 13 +++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/abs-4.c | 13 +++++++++++++
 3 files changed, 36 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/abs-3.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/abs-4.c

Comments

Alexander Monakov May 14, 2023, 7:16 a.m. UTC | #1 
On Sat, 13 May 2023, Andrew Pinski via Gcc-patches wrote:

> +/* signbit(x) != 0 ? -x : x -> abs(x)
> +   signbit(x) == 0 ? -x : x -> -abs(x) */
> +(for sign (SIGNBIT)

Surprised to see a dummy iterator here. Was this meant to include
float and long double versions of the builtin too (SIGNBITF and SIGNBITL)?

> + (for neeq (ne eq)
> +  (simplify
> +   (cond (neeq (sign @0) integer_zerop) (negate @0) @0)
> +    (if (neeq == NE_EXPR)
> +     (abs @0)
> +     (negate (abs @0))))))
> +
>  (simplify
>   /* signbit(x) -> 0 if x is nonnegative.  */
>   (SIGNBIT tree_expr_nonnegative_p@0)

Thanks.
Alexander
Alexander Monakov May 14, 2023, 7:43 a.m. UTC | #2 
On Sun, 14 May 2023, Alexander Monakov wrote:

> On Sat, 13 May 2023, Andrew Pinski via Gcc-patches wrote:
> 
> > +/* signbit(x) != 0 ? -x : x -> abs(x)
> > +   signbit(x) == 0 ? -x : x -> -abs(x) */
> > +(for sign (SIGNBIT)
> 
> Surprised to see a dummy iterator here. Was this meant to include
> float and long double versions of the builtin too (SIGNBITF and SIGNBITL)?

On the other hand, the following clauses both use SIGNBIT directly, and
it would be nice to be consistent.

> > + (for neeq (ne eq)
> > +  (simplify
> > +   (cond (neeq (sign @0) integer_zerop) (negate @0) @0)
> > +    (if (neeq == NE_EXPR)
> > +     (abs @0)
> > +     (negate (abs @0))))))
> > +
> >  (simplify
> >   /* signbit(x) -> 0 if x is nonnegative.  */
> >   (SIGNBIT tree_expr_nonnegative_p@0)
> 
> Thanks.
> Alexander
>
Andrew Pinski May 14, 2023, 2:07 p.m. UTC | #3 
On Sun, May 14, 2023 at 12:44 AM Alexander Monakov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> On Sun, 14 May 2023, Alexander Monakov wrote:
>
> > On Sat, 13 May 2023, Andrew Pinski via Gcc-patches wrote:
> >
> > > +/* signbit(x) != 0 ? -x : x -> abs(x)
> > > +   signbit(x) == 0 ? -x : x -> -abs(x) */
> > > +(for sign (SIGNBIT)
> >
> > Surprised to see a dummy iterator here. Was this meant to include
> > float and long double versions of the builtin too (SIGNBITF and SIGNBITL)?

It is NOT a dummy iterator. SIGNBIT is a operator list that expands to
"BUILT_IN_SIGNBITF  BUILT_IN_SIGNBIT BUILT_IN_SIGNBITL IFN_SIGNBIT".

>
> On the other hand, the following clauses both use SIGNBIT directly, and
> it would be nice to be consistent.

You cannot use the operator list directly if you have a for loop
expansion too. So it is internally consistent already.

Thanks,
Andrew Pinski

>
> > > + (for neeq (ne eq)
> > > +  (simplify
> > > +   (cond (neeq (sign @0) integer_zerop) (negate @0) @0)
> > > +    (if (neeq == NE_EXPR)
> > > +     (abs @0)
> > > +     (negate (abs @0))))))
> > > +
> > >  (simplify
> > >   /* signbit(x) -> 0 if x is nonnegative.  */
> > >   (SIGNBIT tree_expr_nonnegative_p@0)
> >
> > Thanks.
> > Alexander
> >
Alexander Monakov May 14, 2023, 3:01 p.m. UTC | #4 
On Sun, 14 May 2023, Andrew Pinski wrote:

> It is NOT a dummy iterator. SIGNBIT is a operator list that expands to
> "BUILT_IN_SIGNBITF  BUILT_IN_SIGNBIT BUILT_IN_SIGNBITL IFN_SIGNBIT".

Ah, it's in cfn-operators.pd in the build tree, not the source tree.

> > On the other hand, the following clauses both use SIGNBIT directly, and
> > it would be nice to be consistent.
> 
> You cannot use the operator list directly if you have a for loop
> expansion too. So it is internally consistent already.

I see. Wasn't aware of the limitation.

Thanks.
Alexander
Jeff Law May 14, 2023, 11:02 p.m. UTC | #5 
On 5/13/23 21:12, Andrew Pinski via Gcc-patches wrote:
> This adds a simple pattern to match.pd for `signbit(x) ? x : -x`
> into abs<x>. This can be done for all types even ones that honor
> signed zeros and NaNs because both signbit and - are considered
> only looking at/touching the sign bit of those types and does
> not trap either.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> 	PR tree-optimization/109829
> 
> gcc/ChangeLog:
> 
> 	* match.pd: Add pattern for `signbit(x) !=/== 0 ? x : -x`.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/tree-ssa/abs-3.c: New test.
> 	* gcc.dg/tree-ssa/abs-4.c: New test.

OK.
jeff
diff mbox series

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 26395880047..b025fb8facf 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -7439,6 +7439,16 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        && TREE_CODE (@0) != INTEGER_CST)
    (op @0 (ext @1 @2)))))
 
+/* signbit(x) != 0 ? -x : x -> abs(x)
+   signbit(x) == 0 ? -x : x -> -abs(x) */
+(for sign (SIGNBIT)
+ (for neeq (ne eq)
+  (simplify
+   (cond (neeq (sign @0) integer_zerop) (negate @0) @0)
+    (if (neeq == NE_EXPR)
+     (abs @0)
+     (negate (abs @0))))))
+
 (simplify
  /* signbit(x) -> 0 if x is nonnegative.  */
  (SIGNBIT tree_expr_nonnegative_p@0)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/abs-3.c b/gcc/testsuite/gcc.dg/tree-ssa/abs-3.c
new file mode 100644
index 00000000000..d2638e8f4c5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/abs-3.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+/* PR tree-optimization/109829 */
+
+float abs_f(float x) { return __builtin_signbit(x) ? -x : x; }
+double abs_d(double x) { return __builtin_signbit(x) ? -x : x; }
+long double abs_ld(long double x) { return __builtin_signbit(x) ? -x : x; }
+
+
+/* __builtin_signbit(x) ? -x : x. Should be convert into ABS_EXP<x> */
+/* { dg-final { scan-tree-dump-not "signbit" "optimized"} } */
+/* { dg-final { scan-tree-dump-not "= -" "optimized"} } */
+/* { dg-final { scan-tree-dump-times "= ABS_EXPR" 3 "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/abs-4.c b/gcc/testsuite/gcc.dg/tree-ssa/abs-4.c
new file mode 100644
index 00000000000..6197519faf7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/abs-4.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+/* PR tree-optimization/109829 */
+
+float abs_f(float x) { return __builtin_signbit(x) ? x : -x; }
+double abs_d(double x) { return __builtin_signbit(x) ? x : -x; }
+long double abs_ld(long double x) { return __builtin_signbit(x) ? x : -x; }
+
+
+/* __builtin_signbit(x) ? x : -x. Should be convert into - ABS_EXP<x> */
+/* { dg-final { scan-tree-dump-not "signbit" "optimized"} } */
+/* { dg-final { scan-tree-dump-times "= ABS_EXPR" 3 "optimized"} } */
+/* { dg-final { scan-tree-dump-times "= -" 3 "optimized"} } */