[patch, aarch64] additional bics patterns

[patch, aarch64] additional bics patterns

[Sandra Loosemore]

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

This patch to the AArch64 back end adds a couple of additional bics 
patterns to match code of the form

   if ((x & y) == x) ...;

This is testing whether the bits set in x are a subset of the bits set 
in y; or, that no bits in x are set that are not set in y.  So, it is 
equivalent to

   if ((x & ~y) == 0) ...;

Presently this generates code like
   and     x21, x21, x20
   cmp     x21, x20
   b.eq    c0 <main+0xc0>

and this patch allows it to be written more concisely as:
   bics     x21, x20, x21
   b.eq     c0 <main+0xc0>

Since the bics instruction sets the condition codes itself, no explicit 
comparison is required and the result of the bics computation can be 
discarded.

Regression-tested on aarch64-linux-gnu.  OK to commit?

-Sandra


2014-11-12  Sandra Loosemore  <sandra@codesourcery.com>
	    Chris Jones <chrisj@nvidia.com>

	gcc/
	* config/aarch64/aarch64.md (*and<mode>3_compare_op1): New.
	(*and<mode>3_compare_op2): New.

	gcc/testsuite/
	* gcc.target/aarch64/bics_3.c: New.



[-- Attachment #2: aarch64-bics.patch --]
[-- Type: text/x-patch, Size: 2989 bytes --]

Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 217322)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -2894,6 +2894,32 @@
   [(set_attr "type" "logics_shift_imm")]
 )
 
+;; ((a & b) == a) ==> ((a & ~b) == 0)
+(define_insn "*and<mode>3_compare_op1"
+  [(set (reg:CC CC_REGNUM)
+        (compare:CC
+          (and:GPI (match_operand:GPI 1 "register_operand" "r")
+		   (match_operand:GPI 2 "register_operand" "r"))
+	  (match_dup 1)))
+   (clobber (match_scratch:GPI 0 "=r"))]
+  ""
+  "bics\\t%<w>0, %<w>1, %<w>2"
+  [(set_attr "type" "logics_reg")]
+)
+
+;; ((a & b) == b) ==> ((b & ~a) == 0)
+(define_insn "*and<mode>3_compare_op2"
+  [(set (reg:CC CC_REGNUM)
+	(compare:CC
+	  (and:GPI (match_operand:GPI 1 "register_operand" "r")
+		   (match_operand:GPI 2 "register_operand" "r"))
+	  (match_dup 2)))
+   (clobber (match_scratch:GPI 0 "=r"))]
+  ""
+  "bics\\t%<w>0, %<w>2, %<w>1"
+  [(set_attr "type" "logics_reg")]
+)
+
 (define_insn "clz<mode>2"
   [(set (match_operand:GPI 0 "register_operand" "=r")
 	(clz:GPI (match_operand:GPI 1 "register_operand" "r")))]
Index: gcc/testsuite/gcc.target/aarch64/bics_3.c
===================================================================
--- gcc/testsuite/gcc.target/aarch64/bics_3.c	(revision 0)
+++ gcc/testsuite/gcc.target/aarch64/bics_3.c	(revision 0)
@@ -0,0 +1,87 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps -fno-inline" } */
+
+extern void abort (void);
+
+int
+bics_si_test1 (int a, int b, int c)
+{
+  if ((a & b) == a)
+    return a;
+  else
+    return c;
+}
+
+int
+bics_si_test2 (int a, int b, int c)
+{
+  if ((a & b) == b)
+    return b;
+  else
+    return c;
+}
+
+typedef long long s64;
+
+s64
+bics_di_test1 (s64 a, s64 b, s64 c)
+{
+  if ((a & b) == a)
+    return a;
+  else
+    return c;
+}
+
+s64
+bics_di_test2 (s64 a, s64 b, s64 c)
+{
+  if ((a & b) == b)
+    return b;
+  else
+    return c;
+}
+
+int
+main ()
+{
+  int x;
+  s64 y;
+
+  x = bics_si_test1 (0xf00d, 0xf11f, 0);
+  if (x != 0xf00d)
+    abort ();
+
+  x = bics_si_test1 (0xf11f, 0xf00d, 0);
+  if (x != 0)
+    abort ();
+
+  x = bics_si_test2 (0xf00d, 0xf11f, 0);
+  if (x != 0)
+    abort ();
+
+  x = bics_si_test2 (0xf11f, 0xf00d, 0);
+  if (x != 0xf00d)
+    abort ();
+
+  y = bics_di_test1 (0x10001000f00dll, 0x12341000f00dll, 0ll);
+  if (y != 0x10001000f00dll)
+    abort ();
+
+  y = bics_di_test1 (0x12341000f00dll, 0x10001000f00dll, 0ll);
+  if (y != 0)
+    abort ();
+
+  y = bics_di_test2 (0x10001000f00dll, 0x12341000f00dll, 0ll);
+  if (y != 0)
+    abort ();
+
+  y = bics_di_test2 (0x12341000f00dll, 0x10001000f00dll, 0ll);
+  if (y != 0x10001000f00dll)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times "bics\tw\[0-9\]+, w\[0-9\]+, w\[0-9\]+" 2 } } */
+/* { dg-final { scan-assembler-times "bics\tx\[0-9\]+, x\[0-9\]+, x\[0-9\]+" 2 } } */
+/* { dg-final { cleanup-saved-temps } } */

Re: [patch, aarch64] additional bics patterns

[Ramana Radhakrishnan]

On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> This patch to the AArch64 back end adds a couple of additional bics patterns
> to match code of the form
>
>   if ((x & y) == x) ...;
>
> This is testing whether the bits set in x are a subset of the bits set in y;
> or, that no bits in x are set that are not set in y.  So, it is equivalent
> to
>
>   if ((x & ~y) == 0) ...;
>
> Presently this generates code like
>   and     x21, x21, x20
>   cmp     x21, x20
>   b.eq    c0 <main+0xc0>
>
> and this patch allows it to be written more concisely as:
>   bics     x21, x20, x21
>   b.eq     c0 <main+0xc0>
>
> Since the bics instruction sets the condition codes itself, no explicit
> comparison is required and the result of the bics computation can be
> discarded.
>
> Regression-tested on aarch64-linux-gnu.  OK to commit?

Is this not a duplicate of
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?

regards
Ramana

>
> -Sandra
>
>
> 2014-11-12  Sandra Loosemore  <sandra@codesourcery.com>
>             Chris Jones <chrisj@nvidia.com>
>
>         gcc/
>         * config/aarch64/aarch64.md (*and<mode>3_compare_op1): New.
>         (*and<mode>3_compare_op2): New.
>
>         gcc/testsuite/
>         * gcc.target/aarch64/bics_3.c: New.
>
>

Re: [patch, aarch64] additional bics patterns

[Richard Earnshaw]

On 13/11/14 17:05, Ramana Radhakrishnan wrote:
> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
> <sandra@codesourcery.com> wrote:
>> This patch to the AArch64 back end adds a couple of additional bics patterns
>> to match code of the form
>>
>>   if ((x & y) == x) ...;
>>
>> This is testing whether the bits set in x are a subset of the bits set in y;
>> or, that no bits in x are set that are not set in y.  So, it is equivalent
>> to
>>
>>   if ((x & ~y) == 0) ...;
>>
>> Presently this generates code like
>>   and     x21, x21, x20
>>   cmp     x21, x20
>>   b.eq    c0 <main+0xc0>
>>
>> and this patch allows it to be written more concisely as:
>>   bics     x21, x20, x21
>>   b.eq     c0 <main+0xc0>
>>
>> Since the bics instruction sets the condition codes itself, no explicit
>> comparison is required and the result of the bics computation can be
>> discarded.
>>
>> Regression-tested on aarch64-linux-gnu.  OK to commit?
> 
> Is this not a duplicate of
> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
> 
> regards
> Ramana
> 
>>
>> -Sandra
>>
>>
>> 2014-11-12  Sandra Loosemore  <sandra@codesourcery.com>
>>             Chris Jones <chrisj@nvidia.com>
>>
>>         gcc/
>>         * config/aarch64/aarch64.md (*and<mode>3_compare_op1): New.
>>         (*and<mode>3_compare_op2): New.
>>
>>         gcc/testsuite/
>>         * gcc.target/aarch64/bics_3.c: New.
>>
>>
> 
I don't think so.  However, I think it is something that should be
caught in generic simplification code

ie map  ((a & b) == b) ==> ((~a & b) == 0), etc

Bit-clear operations are not that uncommon.  Furthermore, A may be a
constant.

R.

Re: [patch, aarch64] additional bics patterns

[Sandra Loosemore]

On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>> <sandra@codesourcery.com> wrote:
>>> This patch to the AArch64 back end adds a couple of additional bics patterns
>>> to match code of the form
>>>
>>>    if ((x & y) == x) ...;
>>>
>>> This is testing whether the bits set in x are a subset of the bits set in y;
>>> or, that no bits in x are set that are not set in y.  So, it is equivalent
>>> to
>>>
>>>    if ((x & ~y) == 0) ...;
>>>
>>> Presently this generates code like
>>>    and     x21, x21, x20
>>>    cmp     x21, x20
>>>    b.eq    c0 <main+0xc0>
>>>
>>> and this patch allows it to be written more concisely as:
>>>    bics     x21, x20, x21
>>>    b.eq     c0 <main+0xc0>
>>>
>>> Since the bics instruction sets the condition codes itself, no explicit
>>> comparison is required and the result of the bics computation can be
>>> discarded.
>>>
>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>
>> Is this not a duplicate of
>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>
>>
> I don't think so.  However, I think it is something that should be
> caught in generic simplification code
>
> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>
> Bit-clear operations are not that uncommon.  Furthermore, A may be a
> constant.

Alex posted his patch when I already had Chris's in my regression test 
queue, but I've just confirmed that it does not fix the test case I 
included.

I already thought a little about making this a generic simplification, 
but it seemed to me like it was only useful on targets that have a 
bit-clear instruction that happens to set condition codes, and that it 
would pessimize code on targets that don't have a bit-clear instruction 
at all (by inserting the extra complement operation).  So to me it 
seemed reasonable to do it in the back end.

-Sandra

Re: [patch, aarch64] additional bics patterns

[Andrew Pinski]

On Thu, Nov 13, 2014 at 9:42 AM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
>>
>> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>>>
>>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>>> <sandra@codesourcery.com> wrote:
>>>>
>>>> This patch to the AArch64 back end adds a couple of additional bics
>>>> patterns
>>>> to match code of the form
>>>>
>>>>    if ((x & y) == x) ...;
>>>>
>>>> This is testing whether the bits set in x are a subset of the bits set
>>>> in y;
>>>> or, that no bits in x are set that are not set in y.  So, it is
>>>> equivalent
>>>> to
>>>>
>>>>    if ((x & ~y) == 0) ...;
>>>>
>>>> Presently this generates code like
>>>>    and     x21, x21, x20
>>>>    cmp     x21, x20
>>>>    b.eq    c0 <main+0xc0>
>>>>
>>>> and this patch allows it to be written more concisely as:
>>>>    bics     x21, x20, x21
>>>>    b.eq     c0 <main+0xc0>
>>>>
>>>> Since the bics instruction sets the condition codes itself, no explicit
>>>> comparison is required and the result of the bics computation can be
>>>> discarded.
>>>>
>>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>>
>>>
>>> Is this not a duplicate of
>>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>>
>>>
>> I don't think so.  However, I think it is something that should be
>> caught in generic simplification code
>>
>> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>>
>> Bit-clear operations are not that uncommon.  Furthermore, A may be a
>> constant.
>
>
> Alex posted his patch when I already had Chris's in my regression test
> queue, but I've just confirmed that it does not fix the test case I
> included.
>
> I already thought a little about making this a generic simplification, but
> it seemed to me like it was only useful on targets that have a bit-clear
> instruction that happens to set condition codes, and that it would pessimize
> code on targets that don't have a bit-clear instruction at all (by inserting
> the extra complement operation).  So to me it seemed reasonable to do it in
> the back end.

But can't you do this in simplify-rtx.c and allow for the cost model
to do the correct thing?

Thanks,
Andrew

>
> -Sandra
>

Re: [patch, aarch64] additional bics patterns

[Sandra Loosemore]

On 11/13/2014 10:47 AM, Andrew Pinski wrote:
> On Thu, Nov 13, 2014 at 9:42 AM, Sandra Loosemore
> <sandra@codesourcery.com> wrote:
>> On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
>>>
>>> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>>>>
>>>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>>>> <sandra@codesourcery.com> wrote:
>>>>>
>>>>> This patch to the AArch64 back end adds a couple of additional bics
>>>>> patterns
>>>>> to match code of the form
>>>>>
>>>>>     if ((x & y) == x) ...;
>>>>>
>>>>> This is testing whether the bits set in x are a subset of the bits set
>>>>> in y;
>>>>> or, that no bits in x are set that are not set in y.  So, it is
>>>>> equivalent
>>>>> to
>>>>>
>>>>>     if ((x & ~y) == 0) ...;
>>>>>
>>>>> Presently this generates code like
>>>>>     and     x21, x21, x20
>>>>>     cmp     x21, x20
>>>>>     b.eq    c0 <main+0xc0>
>>>>>
>>>>> and this patch allows it to be written more concisely as:
>>>>>     bics     x21, x20, x21
>>>>>     b.eq     c0 <main+0xc0>
>>>>>
>>>>> Since the bics instruction sets the condition codes itself, no explicit
>>>>> comparison is required and the result of the bics computation can be
>>>>> discarded.
>>>>>
>>>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>>>
>>>>
>>>> Is this not a duplicate of
>>>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>>>
>>>>
>>> I don't think so.  However, I think it is something that should be
>>> caught in generic simplification code
>>>
>>> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>>>
>>> Bit-clear operations are not that uncommon.  Furthermore, A may be a
>>> constant.
>>
>>
>> Alex posted his patch when I already had Chris's in my regression test
>> queue, but I've just confirmed that it does not fix the test case I
>> included.
>>
>> I already thought a little about making this a generic simplification, but
>> it seemed to me like it was only useful on targets that have a bit-clear
>> instruction that happens to set condition codes, and that it would pessimize
>> code on targets that don't have a bit-clear instruction at all (by inserting
>> the extra complement operation).  So to me it seemed reasonable to do it in
>> the back end.
>
> But can't you do this in simplify-rtx.c and allow for the cost model
> to do the correct thing?

I could give that a shot, but it seems unlikely that I will be able to 
complete the patch rewrite and testing before we are in Stage 3.  If I 
have something ready next week, will it be too late for consideration in 
GCC 5?

-Sandra

Re: [patch, aarch64] additional bics patterns

[Richard Earnshaw]

On 13/11/14 17:42, Sandra Loosemore wrote:
> On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
>> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>>> <sandra@codesourcery.com> wrote:
>>>> This patch to the AArch64 back end adds a couple of additional bics patterns
>>>> to match code of the form
>>>>
>>>>    if ((x & y) == x) ...;
>>>>
>>>> This is testing whether the bits set in x are a subset of the bits set in y;
>>>> or, that no bits in x are set that are not set in y.  So, it is equivalent
>>>> to
>>>>
>>>>    if ((x & ~y) == 0) ...;
>>>>
>>>> Presently this generates code like
>>>>    and     x21, x21, x20
>>>>    cmp     x21, x20
>>>>    b.eq    c0 <main+0xc0>
>>>>
>>>> and this patch allows it to be written more concisely as:
>>>>    bics     x21, x20, x21
>>>>    b.eq     c0 <main+0xc0>
>>>>
>>>> Since the bics instruction sets the condition codes itself, no explicit
>>>> comparison is required and the result of the bics computation can be
>>>> discarded.
>>>>
>>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>>
>>> Is this not a duplicate of
>>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>>
>>>
>> I don't think so.  However, I think it is something that should be
>> caught in generic simplification code
>>
>> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>>
>> Bit-clear operations are not that uncommon.  Furthermore, A may be a
>> constant.
> 
> Alex posted his patch when I already had Chris's in my regression test 
> queue, but I've just confirmed that it does not fix the test case I 
> included.

Alex's patch is adding a proper pattern for the BICS instruction.  I
wouldn't expect it to directly achieve what you are trying to do - but I
do think the compiler should transform what you have into the form that
Alex has just added the patterns for.

> 
> I already thought a little about making this a generic simplification, 
> but it seemed to me like it was only useful on targets that have a 
> bit-clear instruction that happens to set condition codes, and that it 
> would pessimize code on targets that don't have a bit-clear instruction 
> at all (by inserting the extra complement operation).  So to me it 
> seemed reasonable to do it in the back end.

I doubt that.  I'd be surprised if any target had a direct ((A & B) ==
A) type comparison, so all you're doing is canonicalizing something that
no target is likely to have into something some targets have.  And
ulitimately, if the pattern doesn't match, combine will just undo all
the changes.  Furthermore, as I previously said, if B is a constant then
you're going to get a real optimization for that case that is widely
applicable.  Eg

	A & ~1 == A

will simplify into

	A & 1 == 0

Many targets will have a flag-setting AND operation.

R.

Re: [patch, aarch64] additional bics patterns

[Richard Earnshaw]

On 13/11/14 22:44, Sandra Loosemore wrote:
> On 11/13/2014 10:47 AM, Andrew Pinski wrote:
>> On Thu, Nov 13, 2014 at 9:42 AM, Sandra Loosemore
>> <sandra@codesourcery.com> wrote:
>>> On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
>>>>
>>>> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>>>>>
>>>>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>>>>> <sandra@codesourcery.com> wrote:
>>>>>>
>>>>>> This patch to the AArch64 back end adds a couple of additional bics
>>>>>> patterns
>>>>>> to match code of the form
>>>>>>
>>>>>>     if ((x & y) == x) ...;
>>>>>>
>>>>>> This is testing whether the bits set in x are a subset of the bits set
>>>>>> in y;
>>>>>> or, that no bits in x are set that are not set in y.  So, it is
>>>>>> equivalent
>>>>>> to
>>>>>>
>>>>>>     if ((x & ~y) == 0) ...;
>>>>>>
>>>>>> Presently this generates code like
>>>>>>     and     x21, x21, x20
>>>>>>     cmp     x21, x20
>>>>>>     b.eq    c0 <main+0xc0>
>>>>>>
>>>>>> and this patch allows it to be written more concisely as:
>>>>>>     bics     x21, x20, x21
>>>>>>     b.eq     c0 <main+0xc0>
>>>>>>
>>>>>> Since the bics instruction sets the condition codes itself, no explicit
>>>>>> comparison is required and the result of the bics computation can be
>>>>>> discarded.
>>>>>>
>>>>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>>>>
>>>>>
>>>>> Is this not a duplicate of
>>>>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>>>>
>>>>>
>>>> I don't think so.  However, I think it is something that should be
>>>> caught in generic simplification code
>>>>
>>>> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>>>>
>>>> Bit-clear operations are not that uncommon.  Furthermore, A may be a
>>>> constant.
>>>
>>>
>>> Alex posted his patch when I already had Chris's in my regression test
>>> queue, but I've just confirmed that it does not fix the test case I
>>> included.
>>>
>>> I already thought a little about making this a generic simplification, but
>>> it seemed to me like it was only useful on targets that have a bit-clear
>>> instruction that happens to set condition codes, and that it would pessimize
>>> code on targets that don't have a bit-clear instruction at all (by inserting
>>> the extra complement operation).  So to me it seemed reasonable to do it in
>>> the back end.
>>
>> But can't you do this in simplify-rtx.c and allow for the cost model
>> to do the correct thing?
> 
> I could give that a shot, but it seems unlikely that I will be able to 
> complete the patch rewrite and testing before we are in Stage 3.  If I 
> have something ready next week, will it be too late for consideration in 
> GCC 5?
> 

You're following up on a previously submitted patch and you're not
rewriting large chunks of the compiler.  I don't think this is really
that complicated so I'd be surprised if anyone strongly objected because
it wasn't in final form before the end of stage1.

I would recommend you try to get it at least re-submitted before the end
of the year, though.

R.

Re: [patch v2, aarch64] additional bics patterns

[Sandra Loosemore]

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

On 11/13/2014 10:47 AM, Andrew Pinski wrote:
> On Thu, Nov 13, 2014 at 9:42 AM, Sandra Loosemore
> <sandra@codesourcery.com> wrote:
>> On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
>>>
>>> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>>>>
>>>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>>>> <sandra@codesourcery.com> wrote:
>>>>>
>>>>> This patch to the AArch64 back end adds a couple of additional bics
>>>>> patterns
>>>>> to match code of the form
>>>>>
>>>>>     if ((x & y) == x) ...;
>>>>>
>>>>> This is testing whether the bits set in x are a subset of the bits set
>>>>> in y;
>>>>> or, that no bits in x are set that are not set in y.  So, it is
>>>>> equivalent
>>>>> to
>>>>>
>>>>>     if ((x & ~y) == 0) ...;
>>>>>
>>>>> Presently this generates code like
>>>>>     and     x21, x21, x20
>>>>>     cmp     x21, x20
>>>>>     b.eq    c0 <main+0xc0>
>>>>>
>>>>> and this patch allows it to be written more concisely as:
>>>>>     bics     x21, x20, x21
>>>>>     b.eq     c0 <main+0xc0>
>>>>>
>>>>> Since the bics instruction sets the condition codes itself, no explicit
>>>>> comparison is required and the result of the bics computation can be
>>>>> discarded.
>>>>>
>>>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>>>
>>>>
>>>> Is this not a duplicate of
>>>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>>>
>>>>
>>> I don't think so.  However, I think it is something that should be
>>> caught in generic simplification code
>>>
>>> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>>>
>>> Bit-clear operations are not that uncommon.  Furthermore, A may be a
>>> constant.
>>
>>
>> Alex posted his patch when I already had Chris's in my regression test
>> queue, but I've just confirmed that it does not fix the test case I
>> included.
>>
>> I already thought a little about making this a generic simplification, but
>> it seemed to me like it was only useful on targets that have a bit-clear
>> instruction that happens to set condition codes, and that it would pessimize
>> code on targets that don't have a bit-clear instruction at all (by inserting
>> the extra complement operation).  So to me it seemed reasonable to do it in
>> the back end.
>
> But can't you do this in simplify-rtx.c and allow for the cost model
> to do the correct thing?

OK, here is a revised patch to apply the identity there.  This version 
depends on Alex's aarch64 BICS patch for the included test case to pass, 
though.

In addition to the aarch64 testing, I bootstrapped and regression-tested 
the target-inspecific part of the patch on x86_64-linux-gnu.  Is this 
OK?  Should I hold off on committing it until Alex's patch is in?

-Sandra


2014-11-19  Sandra Loosemore  <sandra@codesourcery.com>

	gcc/
	* simplify-rtx.c (simplify_relational_operation_1): Handle
	simplification identities for BICS patterns.

	gcc/testsuite/
	* gcc.target/aarch64/bics_4.c: New.



[-- Attachment #2: bics2.patch --]
[-- Type: text/x-patch, Size: 3280 bytes --]

Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	(revision 217322)
+++ gcc/simplify-rtx.c	(working copy)
@@ -4551,6 +4551,32 @@ simplify_relational_operation_1 (enum rt
 				    simplify_gen_binary (XOR, cmp_mode,
 							 XEXP (op0, 1), op1));
 
+  /* (eq/ne (and x y) x) simplifies to (eq/ne (and (not y) x) 0), which
+     can be implemented with a BICS instruction on some targets, or
+     constant-folded if y is a constant.  */
+  if ((code == EQ || code == NE)
+      && op0code == AND
+      && rtx_equal_p (XEXP (op0, 0), op1)
+      && !side_effects_p (op1))
+    {
+      rtx not_y = simplify_gen_unary (NOT, cmp_mode, XEXP (op0, 1), cmp_mode);
+      rtx lhs = simplify_gen_binary (AND, cmp_mode, not_y, XEXP (op0, 0));
+      
+      return simplify_gen_relational (code, mode, cmp_mode, lhs, const0_rtx);
+    }
+
+  /* Likewise for (eq/ne (and x y) y).  */
+  if ((code == EQ || code == NE)
+      && op0code == AND
+      && rtx_equal_p (XEXP (op0, 1), op1)
+      && !side_effects_p (op1))
+    {
+      rtx not_x = simplify_gen_unary (NOT, cmp_mode, XEXP (op0, 0), cmp_mode);
+      rtx lhs = simplify_gen_binary (AND, cmp_mode, not_x, XEXP (op0, 1));
+      
+      return simplify_gen_relational (code, mode, cmp_mode, lhs, const0_rtx);
+    }
+
   /* (eq/ne (bswap x) C1) simplifies to (eq/ne x C2) with C2 swapped.  */
   if ((code == EQ || code == NE)
       && GET_CODE (op0) == BSWAP
Index: gcc/testsuite/gcc.target/aarch64/bics_4.c
===================================================================
--- gcc/testsuite/gcc.target/aarch64/bics_4.c	(revision 0)
+++ gcc/testsuite/gcc.target/aarch64/bics_4.c	(revision 0)
@@ -0,0 +1,87 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps -fno-inline" } */
+
+extern void abort (void);
+
+int
+bics_si_test1 (int a, int b, int c)
+{
+  if ((a & b) == a)
+    return a;
+  else
+    return c;
+}
+
+int
+bics_si_test2 (int a, int b, int c)
+{
+  if ((a & b) == b)
+    return b;
+  else
+    return c;
+}
+
+typedef long long s64;
+
+s64
+bics_di_test1 (s64 a, s64 b, s64 c)
+{
+  if ((a & b) == a)
+    return a;
+  else
+    return c;
+}
+
+s64
+bics_di_test2 (s64 a, s64 b, s64 c)
+{
+  if ((a & b) == b)
+    return b;
+  else
+    return c;
+}
+
+int
+main ()
+{
+  int x;
+  s64 y;
+
+  x = bics_si_test1 (0xf00d, 0xf11f, 0);
+  if (x != 0xf00d)
+    abort ();
+
+  x = bics_si_test1 (0xf11f, 0xf00d, 0);
+  if (x != 0)
+    abort ();
+
+  x = bics_si_test2 (0xf00d, 0xf11f, 0);
+  if (x != 0)
+    abort ();
+
+  x = bics_si_test2 (0xf11f, 0xf00d, 0);
+  if (x != 0xf00d)
+    abort ();
+
+  y = bics_di_test1 (0x10001000f00dll, 0x12341000f00dll, 0ll);
+  if (y != 0x10001000f00dll)
+    abort ();
+
+  y = bics_di_test1 (0x12341000f00dll, 0x10001000f00dll, 0ll);
+  if (y != 0)
+    abort ();
+
+  y = bics_di_test2 (0x10001000f00dll, 0x12341000f00dll, 0ll);
+  if (y != 0)
+    abort ();
+
+  y = bics_di_test2 (0x12341000f00dll, 0x10001000f00dll, 0ll);
+  if (y != 0x10001000f00dll)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times "bics\twzr, w\[0-9\]+, w\[0-9\]+" 2 } } */
+/* { dg-final { scan-assembler-times "bics\txzr, x\[0-9\]+, x\[0-9\]+" 2 } } */
+/* { dg-final { cleanup-saved-temps } } */

Re: [patch v2, aarch64] additional bics patterns

[Richard Earnshaw]

On 19/11/14 18:22, Sandra Loosemore wrote:
> On 11/13/2014 10:47 AM, Andrew Pinski wrote:
>> On Thu, Nov 13, 2014 at 9:42 AM, Sandra Loosemore
>> <sandra@codesourcery.com> wrote:
>>> On 11/13/2014 10:27 AM, Richard Earnshaw wrote:
>>>>
>>>> On 13/11/14 17:05, Ramana Radhakrishnan wrote:
>>>>>
>>>>> On Thu, Nov 13, 2014 at 4:55 PM, Sandra Loosemore
>>>>> <sandra@codesourcery.com> wrote:
>>>>>>
>>>>>> This patch to the AArch64 back end adds a couple of additional bics
>>>>>> patterns
>>>>>> to match code of the form
>>>>>>
>>>>>>     if ((x & y) == x) ...;
>>>>>>
>>>>>> This is testing whether the bits set in x are a subset of the bits set
>>>>>> in y;
>>>>>> or, that no bits in x are set that are not set in y.  So, it is
>>>>>> equivalent
>>>>>> to
>>>>>>
>>>>>>     if ((x & ~y) == 0) ...;
>>>>>>
>>>>>> Presently this generates code like
>>>>>>     and     x21, x21, x20
>>>>>>     cmp     x21, x20
>>>>>>     b.eq    c0 <main+0xc0>
>>>>>>
>>>>>> and this patch allows it to be written more concisely as:
>>>>>>     bics     x21, x20, x21
>>>>>>     b.eq     c0 <main+0xc0>
>>>>>>
>>>>>> Since the bics instruction sets the condition codes itself, no explicit
>>>>>> comparison is required and the result of the bics computation can be
>>>>>> discarded.
>>>>>>
>>>>>> Regression-tested on aarch64-linux-gnu.  OK to commit?
>>>>>
>>>>>
>>>>> Is this not a duplicate of
>>>>> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html ?
>>>>>
>>>>>
>>>> I don't think so.  However, I think it is something that should be
>>>> caught in generic simplification code
>>>>
>>>> ie map  ((a & b) == b) ==> ((~a & b) == 0), etc
>>>>
>>>> Bit-clear operations are not that uncommon.  Furthermore, A may be a
>>>> constant.
>>>
>>>
>>> Alex posted his patch when I already had Chris's in my regression test
>>> queue, but I've just confirmed that it does not fix the test case I
>>> included.
>>>
>>> I already thought a little about making this a generic simplification, but
>>> it seemed to me like it was only useful on targets that have a bit-clear
>>> instruction that happens to set condition codes, and that it would pessimize
>>> code on targets that don't have a bit-clear instruction at all (by inserting
>>> the extra complement operation).  So to me it seemed reasonable to do it in
>>> the back end.
>>
>> But can't you do this in simplify-rtx.c and allow for the cost model
>> to do the correct thing?
> 
> OK, here is a revised patch to apply the identity there.  This version 
> depends on Alex's aarch64 BICS patch for the included test case to pass, 
> though.
> 
> In addition to the aarch64 testing, I bootstrapped and regression-tested 
> the target-inspecific part of the patch on x86_64-linux-gnu.  Is this 
> OK?  Should I hold off on committing it until Alex's patch is in?
> 
> -Sandra
> 
> 
> 2014-11-19  Sandra Loosemore  <sandra@codesourcery.com>
> 
> 	gcc/
> 	* simplify-rtx.c (simplify_relational_operation_1): Handle
> 	simplification identities for BICS patterns.
> 
> 	gcc/testsuite/
> 	* gcc.target/aarch64/bics_4.c: New.
> 

Looks sensible to me.  Eric, are you happy?

R.

> 
> bics2.patch
> 
> 
> Index: gcc/simplify-rtx.c
> ===================================================================
> --- gcc/simplify-rtx.c	(revision 217322)
> +++ gcc/simplify-rtx.c	(working copy)
> @@ -4551,6 +4551,32 @@ simplify_relational_operation_1 (enum rt
>  				    simplify_gen_binary (XOR, cmp_mode,
>  							 XEXP (op0, 1), op1));
>  
> +  /* (eq/ne (and x y) x) simplifies to (eq/ne (and (not y) x) 0), which
> +     can be implemented with a BICS instruction on some targets, or
> +     constant-folded if y is a constant.  */
> +  if ((code == EQ || code == NE)
> +      && op0code == AND
> +      && rtx_equal_p (XEXP (op0, 0), op1)
> +      && !side_effects_p (op1))
> +    {
> +      rtx not_y = simplify_gen_unary (NOT, cmp_mode, XEXP (op0, 1), cmp_mode);
> +      rtx lhs = simplify_gen_binary (AND, cmp_mode, not_y, XEXP (op0, 0));
> +      
> +      return simplify_gen_relational (code, mode, cmp_mode, lhs, const0_rtx);
> +    }
> +
> +  /* Likewise for (eq/ne (and x y) y).  */
> +  if ((code == EQ || code == NE)
> +      && op0code == AND
> +      && rtx_equal_p (XEXP (op0, 1), op1)
> +      && !side_effects_p (op1))
> +    {
> +      rtx not_x = simplify_gen_unary (NOT, cmp_mode, XEXP (op0, 0), cmp_mode);
> +      rtx lhs = simplify_gen_binary (AND, cmp_mode, not_x, XEXP (op0, 1));
> +      
> +      return simplify_gen_relational (code, mode, cmp_mode, lhs, const0_rtx);
> +    }
> +
>    /* (eq/ne (bswap x) C1) simplifies to (eq/ne x C2) with C2 swapped.  */
>    if ((code == EQ || code == NE)
>        && GET_CODE (op0) == BSWAP
> Index: gcc/testsuite/gcc.target/aarch64/bics_4.c
> ===================================================================
> --- gcc/testsuite/gcc.target/aarch64/bics_4.c	(revision 0)
> +++ gcc/testsuite/gcc.target/aarch64/bics_4.c	(revision 0)
> @@ -0,0 +1,87 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 --save-temps -fno-inline" } */
> +
> +extern void abort (void);
> +
> +int
> +bics_si_test1 (int a, int b, int c)
> +{
> +  if ((a & b) == a)
> +    return a;
> +  else
> +    return c;
> +}
> +
> +int
> +bics_si_test2 (int a, int b, int c)
> +{
> +  if ((a & b) == b)
> +    return b;
> +  else
> +    return c;
> +}
> +
> +typedef long long s64;
> +
> +s64
> +bics_di_test1 (s64 a, s64 b, s64 c)
> +{
> +  if ((a & b) == a)
> +    return a;
> +  else
> +    return c;
> +}
> +
> +s64
> +bics_di_test2 (s64 a, s64 b, s64 c)
> +{
> +  if ((a & b) == b)
> +    return b;
> +  else
> +    return c;
> +}
> +
> +int
> +main ()
> +{
> +  int x;
> +  s64 y;
> +
> +  x = bics_si_test1 (0xf00d, 0xf11f, 0);
> +  if (x != 0xf00d)
> +    abort ();
> +
> +  x = bics_si_test1 (0xf11f, 0xf00d, 0);
> +  if (x != 0)
> +    abort ();
> +
> +  x = bics_si_test2 (0xf00d, 0xf11f, 0);
> +  if (x != 0)
> +    abort ();
> +
> +  x = bics_si_test2 (0xf11f, 0xf00d, 0);
> +  if (x != 0xf00d)
> +    abort ();
> +
> +  y = bics_di_test1 (0x10001000f00dll, 0x12341000f00dll, 0ll);
> +  if (y != 0x10001000f00dll)
> +    abort ();
> +
> +  y = bics_di_test1 (0x12341000f00dll, 0x10001000f00dll, 0ll);
> +  if (y != 0)
> +    abort ();
> +
> +  y = bics_di_test2 (0x10001000f00dll, 0x12341000f00dll, 0ll);
> +  if (y != 0)
> +    abort ();
> +
> +  y = bics_di_test2 (0x12341000f00dll, 0x10001000f00dll, 0ll);
> +  if (y != 0x10001000f00dll)
> +    abort ();
> +
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "bics\twzr, w\[0-9\]+, w\[0-9\]+" 2 } } */
> +/* { dg-final { scan-assembler-times "bics\txzr, x\[0-9\]+, x\[0-9\]+" 2 } } */
> +/* { dg-final { cleanup-saved-temps } } */
> 

Re: [patch v2, aarch64] additional bics patterns

[Eric Botcazou]

> 2014-11-19  Sandra Loosemore  <sandra@codesourcery.com>
> 
> 	gcc/
> 	* simplify-rtx.c (simplify_relational_operation_1): Handle
> 	simplification identities for BICS patterns.
> 
> 	gcc/testsuite/
> 	* gcc.target/aarch64/bics_4.c: New.

OK for mainline, but there are trailing spaces in the patch.

-- 
Eric Botcazou

Re: [patch v2, aarch64] additional bics patterns

[Sandra Loosemore]

On 12/04/2014 09:06 AM, Eric Botcazou wrote:
>> 2014-11-19  Sandra Loosemore  <sandra@codesourcery.com>
>>
>> 	gcc/
>> 	* simplify-rtx.c (simplify_relational_operation_1): Handle
>> 	simplification identities for BICS patterns.
>>
>> 	gcc/testsuite/
>> 	* gcc.target/aarch64/bics_4.c: New.
>
> OK for mainline, but there are trailing spaces in the patch.

I'll fix that, but I don't see any point in committing this until Alex's 
AArch64 BICS patch is in.  It doesn't look like it's been reviewed yet 
-- ping?

https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00943.html

-Sandra