RE: [PATCH 1/2]middle-end: Simplify subtract where both arguments are being bitwise inverted.

[Tamar Christina <Tamar.Christina@arm.com>]

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

> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Tuesday, June 21, 2022 8:43 AM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: Richard Sandiford <Richard.Sandiford@arm.com>; Richard Biener via Gcc-
> patches <gcc-patches@gcc.gnu.org>; Richard Guenther
> <rguenther@suse.de>; nd <nd@arm.com>
> Subject: Re: [PATCH 1/2]middle-end: Simplify subtract where both
> arguments are being bitwise inverted.
> 
> On Mon, Jun 20, 2022 at 10:49 AM Tamar Christina
> <Tamar.Christina@arm.com> wrote:
> >
> > > -----Original Message-----
> > > From: Richard Sandiford <richard.sandiford@arm.com>
> > > Sent: Monday, June 20, 2022 9:19 AM
> > > To: Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org>
> > > Cc: Tamar Christina <Tamar.Christina@arm.com>; Richard Biener
> > > <richard.guenther@gmail.com>; Richard Guenther
> <rguenther@suse.de>;
> > > nd <nd@arm.com>
> > > Subject: Re: [PATCH 1/2]middle-end: Simplify subtract where both
> > > arguments are being bitwise inverted.
> > >
> > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > > > On Thu, Jun 16, 2022 at 1:10 PM Tamar Christina via Gcc-patches
> > > > <gcc-patches@gcc.gnu.org> wrote:
> > > >>
> > > >> Hi All,
> > > >>
> > > >> This adds a match.pd rule that drops the bitwwise nots when both
> > > >> arguments to a subtract is inverted. i.e. for:
> > > >>
> > > >> float g(float a, float b)
> > > >> {
> > > >>   return ~(int)a - ~(int)b;
> > > >> }
> > > >>
> > > >> we instead generate
> > > >>
> > > >> float g(float a, float b)
> > > >> {
> > > >>   return (int)a - (int)b;
> > > >> }
> > > >>
> > > >> We already do a limited version of this from the fold_binary fold
> > > >> functions but this makes a more general version in match.pd that
> > > >> applies
> > > more often.
> > > >>
> > > >> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > > >>
> > > >> Ok for master?
> > > >>
> > > >> Thanks,
> > > >> Tamar
> > > >>
> > > >> gcc/ChangeLog:
> > > >>
> > > >>         * match.pd: New bit_not rule.
> > > >>
> > > >> gcc/testsuite/ChangeLog:
> > > >>
> > > >>         * gcc.dg/subnot.c: New test.
> > > >>
> > > >> --- inline copy of patch --
> > > >> diff --git a/gcc/match.pd b/gcc/match.pd index
> > > >>
> > >
> a59b6778f661cf9121dd3503f43472871e4da445..51b0a1b562409af535e53828a1
> > > 0
> > > >> c30b8a3e1ae2e 100644
> > > >> --- a/gcc/match.pd
> > > >> +++ b/gcc/match.pd
> > > >> @@ -1258,6 +1258,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN
> (RINT)
> > > >> (simplify
> > > >>   (bit_not (plus:c (bit_not @0) @1))
> > > >>   (minus @0 @1))
> > > >> +/* (~X - ~Y) -> X - Y.  */
> > > >> +(simplify
> > > >> + (minus (bit_not @0) (bit_not @1))  (minus @0 @1))
> > > >
> > > > It doesn't seem correct.
> > > >
> > > > (gdb) p/x ~-1 - ~0x80000000
> > > > $3 = 0x80000001
> > > > (gdb) p/x -1 - 0x80000000
> > > > $4 = 0x7fffffff
> > > >
> > > > where I was looking for a case exposing undefined integer overflow.
> > >
> > > Yeah, shouldn't it be folding to (minus @1 @0) instead?
> > >
> > >   ~X = (-X - 1)
> > >   -Y = (-Y - 1)
> > >
> > > so:
> > >
> > >   ~X - ~Y = (-X - 1) - (-Y - 1)
> > >           = -X - 1 + Y + 1
> > >           = Y - X
> > >
> >
> > You're right, sorry, I should have paid more attention when I wrote the
> patch.
> 
> You still need to watch out for undefined overflow cases in the result that
> were well-defined in the original expression I think.

The only special thing we do for signed numbers if to do the subtract as unsigned.  As I mentioned
before GCC already does this transformation as part of the fold machinery, but that only only happens
when a very simple tree is matched and only when single use. i.e. https://godbolt.org/z/EWsdhfrKj

I'm only attempting to make it apply more generally as the result is always beneficial.

I've respun the patch to the same as we already do.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	* match.pd: New bit_not rule.

gcc/testsuite/ChangeLog:

	* gcc.dg/subnot.c: New test.

--- inline copy of patch ---

diff --git a/gcc/match.pd b/gcc/match.pd
index 330c1db0c8e12b0fb010b1958729444672403866..00b3e07b2a5216b19ed58500923680d83c67d8cf 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1308,6 +1308,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
  (bit_not (plus:c (bit_not @0) @1))
  (minus @0 @1))
+/* (~X - ~Y) -> Y - X.  */
+(simplify
+ (minus (bit_not @0) (bit_not @1))
+  (with { tree utype = unsigned_type_for (type); }
+   (convert (minus (convert:utype @1) (convert:utype @0)))))
 
 /* ~(X - Y) -> ~X + Y.  */
 (simplify
diff --git a/gcc/testsuite/gcc.dg/subnot.c b/gcc/testsuite/gcc.dg/subnot.c
new file mode 100644
index 0000000000000000000000000000000000000000..d621bacd27bd3d19a010e4c9f831aa77d28bd02d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/subnot.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+float g(float a, float b)
+{
+  return ~(int)a - ~(int)b;
+}
+
+/* { dg-final { scan-tree-dump-not "~" "optimized" } } */

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

diff --git a/gcc/match.pd b/gcc/match.pd
index 330c1db0c8e12b0fb010b1958729444672403866..00b3e07b2a5216b19ed58500923680d83c67d8cf 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1308,6 +1308,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
  (bit_not (plus:c (bit_not @0) @1))
  (minus @0 @1))
+/* (~X - ~Y) -> Y - X.  */
+(simplify
+ (minus (bit_not @0) (bit_not @1))
+  (with { tree utype = unsigned_type_for (type); }
+   (convert (minus (convert:utype @1) (convert:utype @0)))))
 
 /* ~(X - Y) -> ~X + Y.  */
 (simplify
diff --git a/gcc/testsuite/gcc.dg/subnot.c b/gcc/testsuite/gcc.dg/subnot.c
new file mode 100644
index 0000000000000000000000000000000000000000..d621bacd27bd3d19a010e4c9f831aa77d28bd02d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/subnot.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+float g(float a, float b)
+{
+  return ~(int)a - ~(int)b;
+}
+
+/* { dg-final { scan-tree-dump-not "~" "optimized" } } */