[ABSU_EXPR] Add some of the missing patterns in match,pd

Message ID CAELXzTP2ZZPEt_qL2Y4_LcwGrPA9-crDyDFHx0oi1g2iCUfeDg@mail.gmail.com
State New
Headers show
Series
  • [ABSU_EXPR] Add some of the missing patterns in match,pd
Related show

Commit Message

Kugan Vivekanandarajah June 28, 2018, 12:25 a.m.
Hi,

This patch adds some of the missing patterns in match.pd for ABSU_EXPR.

Bootstrap and regression testing on x86_64-linux-gnu. Is this OK if no
regressions.

Thanks,
Kugan

gcc/ChangeLog:

2018-06-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

    * match.pd (absu(x)*absu(x) -> x*x): Handle.
        (absu(absu(X)) -> absu(X)): Likewise.
        (absu(-X) -> absu(X)): Likewise.
        (absu(X)  where X is nonnegative -> X): Likewise.

Comments

Jeff Law June 28, 2018, 3:54 a.m. | #1
On 06/27/2018 06:25 PM, Kugan Vivekanandarajah wrote:
> Hi,

> 

> This patch adds some of the missing patterns in match.pd for ABSU_EXPR.

> 

> Bootstrap and regression testing on x86_64-linux-gnu. Is this OK if no

> regressions.

> 

> Thanks,

> Kugan

> 

> gcc/ChangeLog:

> 

> 2018-06-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

> 

>     * match.pd (absu(x)*absu(x) -> x*x): Handle.

>         (absu(absu(X)) -> absu(X)): Likewise.

>         (absu(-X) -> absu(X)): Likewise.

>         (absu(X)  where X is nonnegative -> X): Likewise.

> 

OK once testing is complete.

Thanks,
jeff
Marc Glisse June 28, 2018, 4:11 a.m. | #2
(why is there no mention of ABSU_EXPR in doc/* ?)

--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -571,10 +571,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     (copysigns (op @0) @1)
     (copysigns @0 @1))))

-/* abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
-(simplify
- (mult (abs@1 @0) @1)
- (mult @0 @0))
+/* abs(x)*abs(x) -> x*x.  Should be valid for all types.
+   also for absu(x)*absu(x) -> x*x.  */
+(for op (abs absu)
+ (simplify
+  (mult (op@1 @0) @1)
+  (mult @0 @0)))

1) the types are wrong, it should be (convert (mult @0 @0)), or maybe 
view_convert if vectors also use absu.
2) this is only valid with -fwrapv (TYPE_OVERFLOW_WRAPS(TREE_TYPE(@0))), 
otherwise you are possibly replacing a multiplication on unsigned with a 
multiplication on signed that may overflow. So maybe it is actually 
supposed to be
(mult (convert @0) (convert @0))

  /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
  (for coss (COS COSH)
@@ -1013,15 +1015,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        && tree_nop_conversion_p (type, TREE_TYPE (@2)))
    (bit_xor (convert @1) (convert @2))))

-(simplify
- (abs (abs@1 @0))
- @1)
-(simplify
- (abs (negate @0))
- (abs @0))
-(simplify
- (abs tree_expr_nonnegative_p@0)
- @0)
+/* Convert abs (abs (X)) into abs (X).
+   also absu (absu (X)) into absu (X).  */
+(for op (abs absu)
+ (simplify
+  (op (op@1 @0))
+  @1))

You cannot have (absu (absu @0)) since absu takes a signed integer and 
returns an unsigned one. (absu (abs @0)) may indeed be equivalent to 
(convert (abs @0)). Possibly (op (convert (absu @0))) could also be 
simplified if convert is a NOP.

+/* Convert abs[u] (-X) -> abs[u] (X).  */
+(for op (abs absu)
+ (simplify
+  (op (negate @0))
+  (op @0)))
+
+/* Convert abs[u] (X)  where X is nonnegative -> (X).  */
+(for op (abs absu)
+ (simplify
+  (op tree_expr_nonnegative_p@0)
+  @0))

Missing convert again.

Where are the testcases?

> Bootstrap and regression testing on x86_64-linux-gnu. Is this OK if no 

> regressions.


Does it mean you have run the tests or intend to run them in the future? 
It is confusing.

-- 
Marc Glisse
Jeff Law June 28, 2018, 4:14 a.m. | #3
On 06/27/2018 10:11 PM, Marc Glisse wrote:
> (why is there no mention of ABSU_EXPR in doc/* ?)

[ ... ]
Kugan,

Please address Marc's comments and repost.

Marc, thanks for chiming in.

jeff
Kugan Vivekanandarajah June 29, 2018, 2:38 a.m. | #4
Hi Marc,

Thanks for the review.

On 28 June 2018 at 14:11, Marc Glisse <marc.glisse@inria.fr> wrote:
> (why is there no mention of ABSU_EXPR in doc/* ?)


I will fix this in a separate patch.
>

> --- a/gcc/match.pd

> +++ b/gcc/match.pd

> @@ -571,10 +571,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>     (copysigns (op @0) @1)

>     (copysigns @0 @1))))

>

> -/* abs(x)*abs(x) -> x*x.  Should be valid for all types.  */

> -(simplify

> - (mult (abs@1 @0) @1)

> - (mult @0 @0))

> +/* abs(x)*abs(x) -> x*x.  Should be valid for all types.

> +   also for absu(x)*absu(x) -> x*x.  */

> +(for op (abs absu)

> + (simplify

> +  (mult (op@1 @0) @1)

> +  (mult @0 @0)))

>

> 1) the types are wrong, it should be (convert (mult @0 @0)), or maybe

> view_convert if vectors also use absu.

> 2) this is only valid with -fwrapv (TYPE_OVERFLOW_WRAPS(TREE_TYPE(@0))),

> otherwise you are possibly replacing a multiplication on unsigned with a

> multiplication on signed that may overflow. So maybe it is actually supposed

> to be

> (mult (convert @0) (convert @0))

Indeed, I missed it. I have changed it in the attached patch.
>

>  /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */

>  (for coss (COS COSH)

> @@ -1013,15 +1015,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>        && tree_nop_conversion_p (type, TREE_TYPE (@2)))

>    (bit_xor (convert @1) (convert @2))))

>

> -(simplify

> - (abs (abs@1 @0))

> - @1)

> -(simplify

> - (abs (negate @0))

> - (abs @0))

> -(simplify

> - (abs tree_expr_nonnegative_p@0)

> - @0)

> +/* Convert abs (abs (X)) into abs (X).

> +   also absu (absu (X)) into absu (X).  */

> +(for op (abs absu)

> + (simplify

> +  (op (op@1 @0))

> +  @1))

>

> You cannot have (absu (absu @0)) since absu takes a signed integer and

> returns an unsigned one. (absu (abs @0)) may indeed be equivalent to

> (convert (abs @0)). Possibly (op (convert (absu @0))) could also be

> simplified if convert is a NOP.

>

> +/* Convert abs[u] (-X) -> abs[u] (X).  */

> +(for op (abs absu)

> + (simplify

> +  (op (negate @0))

> +  (op @0)))

> +

> +/* Convert abs[u] (X)  where X is nonnegative -> (X).  */

> +(for op (abs absu)

> + (simplify

> +  (op tree_expr_nonnegative_p@0)

> +  @0))

>

> Missing convert again.

>

> Where are the testcases?

I have fixed the above and added test-cases.

>

>> Bootstrap and regression testing on x86_64-linux-gnu. Is this OK if no

>> regressions.

>

>

> Does it mean you have run the tests or intend to run them in the future? It

> is confusing.

Sorry for not being clear.

I have bootstrapped and regression tested with no new regression.

Is this OK?

Thanks,
Kugan

>

> --

> Marc Glisse
From a2606af5d9ed814cbbbce19dafbb780405ca8a06 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 29 Jun 2018 10:52:46 +1000
Subject: [PATCH] add absu patterns V2

Change-Id: I002312bf23a5fb225b7ff18e98ad22822baa6bef
---
 gcc/match.pd                       | 23 +++++++++++++++++++++++
 gcc/testsuite/gcc.dg/gimplefe-30.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.dg/gimplefe-31.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.dg/gimplefe-32.c | 14 ++++++++++++++
 gcc/testsuite/gcc.dg/gimplefe-33.c | 16 ++++++++++++++++
 5 files changed, 85 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-30.c
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-31.c
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-32.c
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-33.c

diff --git a/gcc/match.pd b/gcc/match.pd
index c1e0963..7959787 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -576,6 +576,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (mult (abs@1 @0) @1)
  (mult @0 @0))
 
+/* Convert absu(x)*absu(x) -> x*x.  */
+(simplify
+ (mult (absu@1 @0) @1)
+ (mult (convert @0) (convert @0)))
+
 /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
 (for coss (COS COSH)
      copysigns (COPYSIGN)
@@ -1013,16 +1018,34 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
       && tree_nop_conversion_p (type, TREE_TYPE (@2)))
   (bit_xor (convert @1) (convert @2))))
 
+/* Convert abs (abs (X)) into abs (X).
+   also absu (absu (X)) into absu (X).  */
 (simplify
  (abs (abs@1 @0))
  @1)
+
+(simplify
+ (absu (nop_convert (absu@1 @0)))
+ @1)
+
+/* Convert abs[u] (-X) -> abs[u] (X).  */
 (simplify
  (abs (negate @0))
  (abs @0))
+
+(simplify
+ (absu (negate @0))
+ (absu @0))
+
+/* Convert abs[u] (X)  where X is nonnegative -> (X).  */
 (simplify
  (abs tree_expr_nonnegative_p@0)
  @0)
 
+(simplify
+ (absu tree_expr_nonnegative_p@0)
+ (convert @0))
+
 /* A few cases of fold-const.c negate_expr_p predicate.  */
 (match negate_expr_p
  INTEGER_CST
diff --git a/gcc/testsuite/gcc.dg/gimplefe-30.c b/gcc/testsuite/gcc.dg/gimplefe-30.c
new file mode 100644
index 0000000..6c25106
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-30.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-optimized" } */
+
+unsigned int __GIMPLE() f(int a)
+{
+  unsigned int t0;
+  unsigned int t1;
+  unsigned int t2;
+  t0 = __ABSU a;
+  t1 = __ABSU a;
+  t2 = t0 * t1;
+  return t2;
+}
+
+
+/* { dg-final { scan-tree-dump-times "ABSU" 0 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/gimplefe-31.c b/gcc/testsuite/gcc.dg/gimplefe-31.c
new file mode 100644
index 0000000..a97d0dd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-31.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-optimized" } */
+
+
+unsigned int __GIMPLE() f(int a)
+{
+  unsigned int t0;
+  int t1;
+  unsigned int t2;
+  t0 = __ABSU a;
+  t1 = (int) t0;
+  t2 = __ABSU t1;
+  return t2;
+}
+
+/* { dg-final { scan-tree-dump-times "ABSU" 1 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/gimplefe-32.c b/gcc/testsuite/gcc.dg/gimplefe-32.c
new file mode 100644
index 0000000..9b3963c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-32.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-optimized" } */
+
+unsigned int __GIMPLE() f(int a)
+{
+  int t0;
+  unsigned int t1;
+  t0 = -a;
+  t1 = __ABSU a;
+  return t1;
+}
+
+
+/* { dg-final { scan-tree-dump-times "= -" 0 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/gimplefe-33.c b/gcc/testsuite/gcc.dg/gimplefe-33.c
new file mode 100644
index 0000000..4e49822
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-33.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-optimized" } */
+
+int __GIMPLE() f(int c)
+{
+  int D;
+  int _1;
+  unsigned int _2;
+  _1 = __ABS c;
+  _2 = __ABSU _1;
+  D = (int) _2;
+  return D;
+}
+
+
+/* { dg-final { scan-tree-dump-times "ABSU" 0 "optimized" } } */
Richard Biener July 2, 2018, 1:55 p.m. | #5
On Fri, Jun 29, 2018 at 4:38 AM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>

> Hi Marc,

>

> Thanks for the review.

>

> On 28 June 2018 at 14:11, Marc Glisse <marc.glisse@inria.fr> wrote:

> > (why is there no mention of ABSU_EXPR in doc/* ?)

>

> I will fix this in a separate patch.

> >

> > --- a/gcc/match.pd

> > +++ b/gcc/match.pd

> > @@ -571,10 +571,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

> >     (copysigns (op @0) @1)

> >     (copysigns @0 @1))))

> >

> > -/* abs(x)*abs(x) -> x*x.  Should be valid for all types.  */

> > -(simplify

> > - (mult (abs@1 @0) @1)

> > - (mult @0 @0))

> > +/* abs(x)*abs(x) -> x*x.  Should be valid for all types.

> > +   also for absu(x)*absu(x) -> x*x.  */

> > +(for op (abs absu)

> > + (simplify

> > +  (mult (op@1 @0) @1)

> > +  (mult @0 @0)))

> >

> > 1) the types are wrong, it should be (convert (mult @0 @0)), or maybe

> > view_convert if vectors also use absu.

> > 2) this is only valid with -fwrapv (TYPE_OVERFLOW_WRAPS(TREE_TYPE(@0))),

> > otherwise you are possibly replacing a multiplication on unsigned with a

> > multiplication on signed that may overflow. So maybe it is actually supposed

> > to be

> > (mult (convert @0) (convert @0))

> Indeed, I missed it. I have changed it in the attached patch.

> >

> >  /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */

> >  (for coss (COS COSH)

> > @@ -1013,15 +1015,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

> >        && tree_nop_conversion_p (type, TREE_TYPE (@2)))

> >    (bit_xor (convert @1) (convert @2))))

> >

> > -(simplify

> > - (abs (abs@1 @0))

> > - @1)

> > -(simplify

> > - (abs (negate @0))

> > - (abs @0))

> > -(simplify

> > - (abs tree_expr_nonnegative_p@0)

> > - @0)

> > +/* Convert abs (abs (X)) into abs (X).

> > +   also absu (absu (X)) into absu (X).  */

> > +(for op (abs absu)

> > + (simplify

> > +  (op (op@1 @0))

> > +  @1))

> >

> > You cannot have (absu (absu @0)) since absu takes a signed integer and

> > returns an unsigned one. (absu (abs @0)) may indeed be equivalent to

> > (convert (abs @0)). Possibly (op (convert (absu @0))) could also be

> > simplified if convert is a NOP.

> >

> > +/* Convert abs[u] (-X) -> abs[u] (X).  */

> > +(for op (abs absu)

> > + (simplify

> > +  (op (negate @0))

> > +  (op @0)))

> > +

> > +/* Convert abs[u] (X)  where X is nonnegative -> (X).  */

> > +(for op (abs absu)

> > + (simplify

> > +  (op tree_expr_nonnegative_p@0)

> > +  @0))

> >

> > Missing convert again.

> >

> > Where are the testcases?

> I have fixed the above and added test-cases.

>

> >

> >> Bootstrap and regression testing on x86_64-linux-gnu. Is this OK if no

> >> regressions.

> >

> >

> > Does it mean you have run the tests or intend to run them in the future? It

> > is confusing.

> Sorry for not being clear.

>

> I have bootstrapped and regression tested with no new regression.

>

> Is this OK?


+/* Convert absu(x)*absu(x) -> x*x.  */
+(simplify
+ (mult (absu@1 @0) @1)
+ (mult (convert @0) (convert @0)))

(mult (convert@2 @0) @2)

+/* Convert abs (abs (X)) into abs (X).
+   also absu (absu (X)) into absu (X).  */
 (simplify
  (abs (abs@1 @0))
  @1)
+
+(simplify
+ (absu (nop_convert (absu@1 @0)))
+ @1)

please use

  (absu (convert@2 (absu@1 @0))
  (if (tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@1)))
   @1))

as Marc said we can have (absu (abs @0))
which should be equivalent to (convert (abs @0))

Otherwise OK.

Richard.

> Thanks,

> Kugan

>

> >

> > --

> > Marc Glisse

Patch

From 374ee7928039c16cb091bd02d5efd4c493aab86e Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Mon, 18 Jun 2018 10:51:06 +1000
Subject: [PATCH] add absu patterns

Change-Id: Ied504be83f00041a6c815d23e16a394b71445f27
---
 gcc/match.pd | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/gcc/match.pd b/gcc/match.pd
index c1e0963..a356a92 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -571,10 +571,12 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    (copysigns (op @0) @1)
    (copysigns @0 @1))))
 
-/* abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
-(simplify
- (mult (abs@1 @0) @1)
- (mult @0 @0))
+/* abs(x)*abs(x) -> x*x.  Should be valid for all types.
+   also for absu(x)*absu(x) -> x*x.  */
+(for op (abs absu)
+ (simplify
+  (mult (op@1 @0) @1)
+  (mult @0 @0)))
 
 /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
 (for coss (COS COSH)
@@ -1013,15 +1015,24 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
       && tree_nop_conversion_p (type, TREE_TYPE (@2)))
   (bit_xor (convert @1) (convert @2))))
 
-(simplify
- (abs (abs@1 @0))
- @1)
-(simplify
- (abs (negate @0))
- (abs @0))
-(simplify
- (abs tree_expr_nonnegative_p@0)
- @0)
+/* Convert abs (abs (X)) into abs (X).
+   also absu (absu (X)) into absu (X).  */
+(for op (abs absu)
+ (simplify
+  (op (op@1 @0))
+  @1))
+
+/* Convert abs[u] (-X) -> abs[u] (X).  */
+(for op (abs absu)
+ (simplify
+  (op (negate @0))
+  (op @0)))
+
+/* Convert abs[u] (X)  where X is nonnegative -> (X).  */
+(for op (abs absu)
+ (simplify
+  (op tree_expr_nonnegative_p@0)
+  @0))
 
 /* A few cases of fold-const.c negate_expr_p predicate.  */
 (match negate_expr_p
-- 
2.7.4