Hi Marc,

Thanks for the review.

On 28 June 2018 at 14:11, Marc Glisse <marc.gli...@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.vivekanandara...@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" } } */
-- 
2.7.4

Reply via email to