[PATCH][PR 67328] Improve bitfield testing

[PATCH][PR 67328] Improve bitfield testing

[Yuri Gribov]

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

Hi all,

This fixes inefficient bitfield code reported in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67328

Bootstrapped and regtested on x86_64.

Ok for trunk?

-I

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

2017-01-21  Yury Gribov  <tetra2005@gmail.com>

	PR tree-optimization/67328

gcc/testsuite/
	* c-c++-common/fold-masked-cmp-1.c: New test.
	* c-c++-common/fold-masked-cmp-2.c: New test.
gcc/
	* match.pd: New pattern.

diff -rupN gcc-master/gcc/match.pd gcc-master-67328/gcc/match.pd
--- a/gcc/match.pd	2017-01-19 06:49:25.723020999 +0000
+++ b/gcc/match.pd	2017-01-20 09:15:10.852018999 +0000
@@ -2632,6 +2632,38 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        || VECTOR_INTEGER_TYPE_P (TREE_TYPE (@0)))
    { constant_boolean_node (false, type); })))
 
+/* A & (2**N - 1) <= 2**K - 1 -> ~(A & (2**N - 2**K)
+   A & (2**N - 1) <  2**K     -> ~(A & (2**N - 2**K)
+   A & (2**N - 1) >= 2**K     -> A & (2**N - 2**K)
+   A & (2**N - 1) >  2**K - 1 -> A & (2**N - 2**K)
+ */
+
+(for cmp (lt le gt ge)
+ (simplify
+  (cmp (bit_and@0 @1 INTEGER_CST@2) INTEGER_CST@3)
+  (if (TYPE_UNSIGNED (TREE_TYPE (@0)) && tree_fits_uhwi_p (@2) && tree_fits_uhwi_p (@3))
+   (with
+    {
+     unsigned HOST_WIDE_INT mask = tree_to_uhwi (@2);
+     bool mask_all_ones_p = !(mask & (mask + 1));
+     unsigned HOST_WIDE_INT rhs = tree_to_uhwi (@3);
+    }
+    (if (mask_all_ones_p && rhs > 0)
+     (with
+      {
+       tree ty = TREE_TYPE (@0);
+       enum tree_code code = cmp == LT_EXPR ? --rhs, LE_EXPR :
+         cmp == GE_EXPR ? --rhs, GT_EXPR :
+         cmp;
+       bool rhs_all_ones_p = !(rhs & (rhs + 1));
+       unsigned HOST_WIDE_INT hi_bits = mask - rhs;
+      }
+      (switch
+       (if (code == LE_EXPR && rhs_all_ones_p && mask >= rhs)
+        (eq:type (bit_and @1 { build_int_cst (ty, hi_bits); }) { build_zero_cst (ty); }))
+       (if (code == GT_EXPR && rhs_all_ones_p && mask >= rhs)
+        (ne:type (bit_and @1 { build_int_cst (ty, hi_bits); }) { build_zero_cst (ty); })))))))))
+
 /* -A CMP -B -> B CMP A.  */
 (for cmp (tcc_comparison)
      scmp (swapped_tcc_comparison)
diff -rupN gcc-master/gcc/testsuite/c-c++-common/fold-masked-cmp-1.c gcc-master-67328/gcc/testsuite/c-c++-common/fold-masked-cmp-1.c
--- a/gcc/testsuite/c-c++-common/fold-masked-cmp-1.c	1970-01-01 01:00:00.000000000 +0100
+++ b/gcc/testsuite/c-c++-common/fold-masked-cmp-1.c	2017-01-21 07:11:57.765019999 +0000
@@ -0,0 +1,41 @@
+/* Based on PR 67328 */
+
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O2" } */
+
+enum output_type
+{
+  type_pde,
+  type_pie,
+  type_relocatable,
+  type_dll,
+};
+
+struct bfd_link_info
+{
+  enum output_type type : 2;
+  unsigned int pad : 30;
+};
+
+#define bfd_link_pde(info)	   ((info)->type == type_pde)
+#define bfd_link_dll(info)	   ((info)->type == type_dll)
+#define bfd_link_relocatable(info) ((info)->type == type_relocatable)
+#define bfd_link_pie(info)	   ((info)->type == type_pie)
+#define bfd_link_executable(info)  (bfd_link_pde (info) || bfd_link_pie (info))
+#define bfd_link_pic(info)	   (bfd_link_dll (info) || bfd_link_pie (info))
+
+int result;
+
+int test_exe (struct bfd_link_info *info)
+{
+  if (bfd_link_executable (info))
+    result++;
+}
+
+/* { dg-final { scan-assembler "testn?b" } } */
diff -rupN gcc-master/gcc/testsuite/c-c++-common/fold-masked-cmp-2.c gcc-master-67328/gcc/testsuite/c-c++-common/fold-masked-cmp-2.c
--- a/gcc/testsuite/c-c++-common/fold-masked-cmp-2.c	1970-01-01 01:00:00.000000000 +0100
+++ b/gcc/testsuite/c-c++-common/fold-masked-cmp-2.c	2017-01-21 07:11:58.965019999 +0000
@@ -0,0 +1,42 @@
+/* Based on PR 67328 */
+
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O2" } */
+
+enum output_type
+{
+  type_pde,
+  type_relocatable,
+  type_pie,
+  type_dll,
+};
+
+struct bfd_link_info
+{
+  enum output_type type : 2;
+  unsigned int pad : 30;
+};
+
+#define bfd_link_pde(info)	   ((info)->type == type_pde)
+#define bfd_link_dll(info)	   ((info)->type == type_dll)
+#define bfd_link_relocatable(info) ((info)->type == type_relocatable)
+#define bfd_link_pie(info)	   ((info)->type == type_pie)
+#define bfd_link_executable(info)  (bfd_link_pde (info) || bfd_link_pie (info))
+#define bfd_link_pic(info)	   (bfd_link_dll (info) || bfd_link_pie (info))
+
+int result;
+
+int test_exe (struct bfd_link_info *info)
+{
+  if (bfd_link_executable (info))
+    result++;
+}
+
+/* { dg-final { scan-assembler "testn?b" } } */
+

Re: [PATCH][PR 67328] Improve bitfield testing

[Richard Biener]

On Wed, 25 Jan 2017, Yuri Gribov wrote:

> Hi all,
> 
> This fixes inefficient bitfield code reported in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67328
> 
> Bootstrapped and regtested on x86_64.
> 
> Ok for trunk?

This isn't a regression fix and thus not appropriate at this stage.

Some comments on the patch:

+/* A & (2**N - 1) <= 2**K - 1 -> ~(A & (2**N - 2**K)
+   A & (2**N - 1) <  2**K     -> ~(A & (2**N - 2**K)
+   A & (2**N - 1) >= 2**K     -> A & (2**N - 2**K)
+   A & (2**N - 1) >  2**K - 1 -> A & (2**N - 2**K)
+ */

you miss the != 0/== 0 in the result (and the ~ is redundant then).

Note that A & (2**N - 1) >= 2**K should already have been simplified
to A & (2**N - 1) >  2**K - 1 (we canonicalize to smaller constants).

+  (if (TYPE_UNSIGNED (TREE_TYPE (@0)) && tree_fits_uhwi_p (@2) && 
tree_fits_uhwi_p (@3))
+   (with
+    {

I think you should restrict this to INTEGRAL_TYPE_P types.

Please use wide-ints so you do not restrict yourself to fits_uhwi_p
values.

Thanks,
Richard.

Re: [PATCH][PR 67328] Improve bitfield testing

[Yuri Gribov]

On Wed, Jan 25, 2017 at 10:49 AM, Richard Biener <rguenther@suse.de> wrote:
> On Wed, 25 Jan 2017, Yuri Gribov wrote:
>
>> Hi all,
>>
>> This fixes inefficient bitfield code reported in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67328
>>
>> Bootstrapped and regtested on x86_64.
>>
>> Ok for trunk?
>
> This isn't a regression fix and thus not appropriate at this stage.

Definitely, just wanted to get initial round of comments. Thanks for
review, will get back with updated patch at stage 1.

> Some comments on the patch:
>
> +/* A & (2**N - 1) <= 2**K - 1 -> ~(A & (2**N - 2**K)
> +   A & (2**N - 1) <  2**K     -> ~(A & (2**N - 2**K)
> +   A & (2**N - 1) >= 2**K     -> A & (2**N - 2**K)
> +   A & (2**N - 1) >  2**K - 1 -> A & (2**N - 2**K)
> + */
>
> you miss the != 0/== 0 in the result (and the ~ is redundant then).
>
> Note that A & (2**N - 1) >= 2**K should already have been simplified
> to A & (2**N - 1) >  2**K - 1 (we canonicalize to smaller constants).
>
> +  (if (TYPE_UNSIGNED (TREE_TYPE (@0)) && tree_fits_uhwi_p (@2) &&
> tree_fits_uhwi_p (@3))
> +   (with
> +    {
>
> I think you should restrict this to INTEGRAL_TYPE_P types.
>
> Please use wide-ints so you do not restrict yourself to fits_uhwi_p
> values.
>
> Thanks,
> Richard.