public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tamar Christina <Tamar.Christina@arm.com>
To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: nd <nd@arm.com>, "rguenther@suse.de" <rguenther@suse.de>
Subject: RE: [PATCH]middle-end convert negate + right shift into compare greater.
Date: Mon, 11 Oct 2021 11:36:19 +0000	[thread overview]
Message-ID: <VI1PR08MB53250021200C3D41ABB08536FFB59@VI1PR08MB5325.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <a9e8ab58-4015-0f88-e4f6-70cce9d78c66@foss.arm.com>

[-- Attachment #1: Type: text/plain, Size: 5166 bytes --]

Hi all,

Here's a new version of the patch.

> >>> " If an exceptional condition occurs during the evaluation of an
> >>> expression
> >> (that is, if the result is not mathematically defined or not in the
> >> range of representable values for its type), the behavior is undefined."
> >>>
> >>> So it should still be acceptable to do in this case.
> >>
> >> -fwrapv
> >
> > If I understand correctly, you're happy with this is I guard it on ! flag_wrapv ?
> 
> I did some more digging.  Right shift of a negative value is IMP_DEF (not
> UNDEF - this keeps catching me out).  So yes, wrapping this with !wrapv
> would address my concern.
> 
> I've not reviewed the patch itself, though.  I've never even written a patch
> for match.pd, so don't feel qualified to do that.

No problem, thanks for catching this! I'm sure one of the Richards will review it when
they have a chance.

Bootstrapped Regtested on aarch64-none-linux-gnu,
x86_64-pc-linux-gnu and no regressions.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	* match.pd: New negate+shift pattern.

gcc/testsuite/ChangeLog:

	* gcc.dg/signbit-2.c: New test.
	* gcc.dg/signbit-3.c: New test.
	* gcc.target/aarch64/signbit-1.c: New test.

--- inline copy of patch ---

diff --git a/gcc/match.pd b/gcc/match.pd
index 7d2a24dbc5e9644a09968f877e12a824d8ba1caa..3d48eda826f889483a83267409c3f278ee907b57 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -826,6 +826,38 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     { tree utype = unsigned_type_for (type); }
     (convert (rshift (lshift (convert:utype @0) @2) @3))))))
 
+/* Fold (-x >> C) into x > 0 where C = precision(type) - 1.  */
+(for cst (INTEGER_CST VECTOR_CST)
+ (simplify
+  (rshift (negate:s @0) cst@1)
+   (if (!flag_wrapv)
+    (with { tree ctype = TREE_TYPE (@0);
+	    tree stype = TREE_TYPE (@1);
+	    tree bt = truth_type_for (ctype); }
+     (switch
+      /* Handle scalar case.  */
+      (if (INTEGRAL_TYPE_P (ctype)
+	   && !VECTOR_TYPE_P (ctype)
+	   && !TYPE_UNSIGNED (ctype)
+	   && canonicalize_math_after_vectorization_p ()
+	   && wi::eq_p (wi::to_wide (@1), TYPE_PRECISION (stype) - 1))
+       (convert:bt (gt:bt @0 { build_zero_cst (stype); })))
+      /* Handle vector case with a scalar immediate.  */
+      (if (VECTOR_INTEGER_TYPE_P (ctype)
+	   && !VECTOR_TYPE_P (stype)
+	   && !TYPE_UNSIGNED (ctype)
+           && wi::eq_p (wi::to_wide (@1), TYPE_PRECISION (stype) - 1))
+       (convert:bt (gt:bt @0 { build_zero_cst (ctype); })))
+      /* Handle vector case with a vector immediate.   */
+      (if (VECTOR_INTEGER_TYPE_P (ctype)
+	   && VECTOR_TYPE_P (stype)
+	   && !TYPE_UNSIGNED (ctype)
+	   && uniform_vector_p (@1))
+       (with { tree cst = vector_cst_elt (@1, 0);
+	       tree t = TREE_TYPE (cst); }
+        (if (wi::eq_p (wi::to_wide (cst), TYPE_PRECISION (t) - 1))
+         (convert:bt (gt:bt @0 { build_zero_cst (ctype); }))))))))))
+
 /* Fold (C1/X)*C2 into (C1*C2)/X.  */
 (simplify
  (mult (rdiv@3 REAL_CST@0 @1) REAL_CST@2)
diff --git a/gcc/testsuite/gcc.dg/signbit-2.c b/gcc/testsuite/gcc.dg/signbit-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..fc0157cbc5c7996b481f2998bc30176c96a669bb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/signbit-2.c
@@ -0,0 +1,19 @@
+/* { dg-do assemble } */
+/* { dg-options "-O3 --save-temps -fdump-tree-optimized" } */
+
+#include <stdint.h>
+
+void fun1(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 31;
+}
+
+void fun2(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 30;
+}
+
+/* { dg-final { scan-tree-dump-times {\s+>\s+\{ 0, 0, 0, 0 \}} 1 optimized } } */
+/* { dg-final { scan-tree-dump-not {\s+>>\s+31} optimized } } */
diff --git a/gcc/testsuite/gcc.dg/signbit-3.c b/gcc/testsuite/gcc.dg/signbit-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..19e9c06c349b3287610f817628f00938ece60bf7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/signbit-3.c
@@ -0,0 +1,13 @@
+/* { dg-do assemble } */
+/* { dg-options "-O1 --save-temps -fdump-tree-optimized" } */
+
+#include <stdint.h>
+
+void fun1(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 31;
+}
+
+/* { dg-final { scan-tree-dump-times {\s+>\s+0;} 1 optimized } } */
+/* { dg-final { scan-tree-dump-not {\s+>>\s+31} optimized } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/signbit-1.c b/gcc/testsuite/gcc.target/aarch64/signbit-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3ebfb0586f37de29cf58635b27fe48503714447e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/signbit-1.c
@@ -0,0 +1,18 @@
+/* { dg-do assemble } */
+/* { dg-options "-O3 --save-temps" } */
+
+#include <stdint.h>
+
+void fun1(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 31;
+}
+
+void fun2(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 30;
+}
+
+/* { dg-final { scan-assembler-times {\tcmgt\t} 1 } } */

[-- Attachment #2: rb14918.patch --]
[-- Type: application/octet-stream, Size: 3783 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 7d2a24dbc5e9644a09968f877e12a824d8ba1caa..3d48eda826f889483a83267409c3f278ee907b57 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -826,6 +826,38 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     { tree utype = unsigned_type_for (type); }
     (convert (rshift (lshift (convert:utype @0) @2) @3))))))
 
+/* Fold (-x >> C) into x > 0 where C = precision(type) - 1.  */
+(for cst (INTEGER_CST VECTOR_CST)
+ (simplify
+  (rshift (negate:s @0) cst@1)
+   (if (!flag_wrapv)
+    (with { tree ctype = TREE_TYPE (@0);
+	    tree stype = TREE_TYPE (@1);
+	    tree bt = truth_type_for (ctype); }
+     (switch
+      /* Handle scalar case.  */
+      (if (INTEGRAL_TYPE_P (ctype)
+	   && !VECTOR_TYPE_P (ctype)
+	   && !TYPE_UNSIGNED (ctype)
+	   && canonicalize_math_after_vectorization_p ()
+	   && wi::eq_p (wi::to_wide (@1), TYPE_PRECISION (stype) - 1))
+       (convert:bt (gt:bt @0 { build_zero_cst (stype); })))
+      /* Handle vector case with a scalar immediate.  */
+      (if (VECTOR_INTEGER_TYPE_P (ctype)
+	   && !VECTOR_TYPE_P (stype)
+	   && !TYPE_UNSIGNED (ctype)
+           && wi::eq_p (wi::to_wide (@1), TYPE_PRECISION (stype) - 1))
+       (convert:bt (gt:bt @0 { build_zero_cst (ctype); })))
+      /* Handle vector case with a vector immediate.   */
+      (if (VECTOR_INTEGER_TYPE_P (ctype)
+	   && VECTOR_TYPE_P (stype)
+	   && !TYPE_UNSIGNED (ctype)
+	   && uniform_vector_p (@1))
+       (with { tree cst = vector_cst_elt (@1, 0);
+	       tree t = TREE_TYPE (cst); }
+        (if (wi::eq_p (wi::to_wide (cst), TYPE_PRECISION (t) - 1))
+         (convert:bt (gt:bt @0 { build_zero_cst (ctype); }))))))))))
+
 /* Fold (C1/X)*C2 into (C1*C2)/X.  */
 (simplify
  (mult (rdiv@3 REAL_CST@0 @1) REAL_CST@2)
diff --git a/gcc/testsuite/gcc.dg/signbit-2.c b/gcc/testsuite/gcc.dg/signbit-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..fc0157cbc5c7996b481f2998bc30176c96a669bb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/signbit-2.c
@@ -0,0 +1,19 @@
+/* { dg-do assemble } */
+/* { dg-options "-O3 --save-temps -fdump-tree-optimized" } */
+
+#include <stdint.h>
+
+void fun1(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 31;
+}
+
+void fun2(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 30;
+}
+
+/* { dg-final { scan-tree-dump-times {\s+>\s+\{ 0, 0, 0, 0 \}} 1 optimized } } */
+/* { dg-final { scan-tree-dump-not {\s+>>\s+31} optimized } } */
diff --git a/gcc/testsuite/gcc.dg/signbit-3.c b/gcc/testsuite/gcc.dg/signbit-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..19e9c06c349b3287610f817628f00938ece60bf7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/signbit-3.c
@@ -0,0 +1,13 @@
+/* { dg-do assemble } */
+/* { dg-options "-O1 --save-temps -fdump-tree-optimized" } */
+
+#include <stdint.h>
+
+void fun1(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 31;
+}
+
+/* { dg-final { scan-tree-dump-times {\s+>\s+0;} 1 optimized } } */
+/* { dg-final { scan-tree-dump-not {\s+>>\s+31} optimized } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/signbit-1.c b/gcc/testsuite/gcc.target/aarch64/signbit-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3ebfb0586f37de29cf58635b27fe48503714447e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/signbit-1.c
@@ -0,0 +1,18 @@
+/* { dg-do assemble } */
+/* { dg-options "-O3 --save-temps" } */
+
+#include <stdint.h>
+
+void fun1(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 31;
+}
+
+void fun2(int32_t *x, int n)
+{
+    for (int i = 0; i < (n & -16); i++)
+      x[i] = (-x[i]) >> 30;
+}
+
+/* { dg-final { scan-assembler-times {\tcmgt\t} 1 } } */

  reply	other threads:[~2021-10-11 11:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05 12:50 Tamar Christina
2021-10-05 12:56 ` Richard Earnshaw
2021-10-05 13:30   ` Tamar Christina
2021-10-05 13:34     ` Richard Earnshaw
2021-10-05 13:49       ` Tamar Christina
2021-10-05 13:51         ` Richard Earnshaw
2021-10-05 13:56           ` Tamar Christina
2021-10-07 12:46             ` Richard Earnshaw
2021-10-11 11:36               ` Tamar Christina [this message]
2021-10-13 12:11                 ` Richard Biener
2021-10-15  7:48                   ` Tamar Christina
2021-10-15  9:06                     ` Richard Biener
2021-10-15 10:36                       ` Richard Earnshaw
2021-10-15 10:57                         ` Richard Biener
2021-10-15 11:55                       ` Tamar Christina
     [not found]                         ` <VI1PR08MB5325D87574F2D09568EF5C40FF849@VI1PR08MB5325.eurprd08.prod.outlook.com>
     [not found]                           ` <34p8433-751p-2n5s-qp50-r8rss490npop@fhfr.qr>
2021-11-03 13:21                             ` Tamar Christina
2021-11-04 13:06                               ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR08MB53250021200C3D41ABB08536FFB59@VI1PR08MB5325.eurprd08.prod.outlook.com \
    --to=tamar.christina@arm.com \
    --cc=Richard.Earnshaw@foss.arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nd@arm.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).