PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Hurugalawadi, Naveen]

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

Hi,

Please find attached the patch that performs optimization on unsigned values.

Original fold-const part implemented in match.pd.

Please review the patch and let us know if it's OK?

Regression Tested on X86_64 with no regressions.

Thanks,
Naveen

ChangeLog:
* match.pd (cmp (convert (bit_not @0)) INTEGER_CST@1): New Simplifier.

Testsuite/ChangeLog:
* gcc.dg/pr31531.c : New testcase.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr31531.patch --]
[-- Type: text/x-diff; name="pr31531.patch", Size: 1303 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 6c8ebd5..42f772b 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1452,6 +1452,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        && (TYPE_SIZE (TREE_TYPE (@0)) == TYPE_SIZE (TREE_TYPE (@1))))
    (view_convert @1)))
 
+/* Fold ((CAST)~X) op C as ((CAST)X) op' ~C, where op' is the
+   swapped comparison.  */
+(for cmp (tcc_comparison)
+     scmp (swapped_tcc_comparison)
+ (simplify
+  (cmp (convert (bit_not @0)) INTEGER_CST@1)
+  (if (TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0))
+       && TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (TREE_TYPE (@1)))
+   (with { tree cst = fold_convert (TREE_TYPE (@0), @1); }
+    (scmp @0 (bit_not { cst; }))))))
+
 /* Re-association barriers around constants and other re-association
    barriers can be removed.  */
 (simplify
diff --git a/gcc/testsuite/gcc.dg/pr31531.c b/gcc/testsuite/gcc.dg/pr31531.c
new file mode 100644
index 0000000..d687c91
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr31531.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-gimple" } */
+int isnegative_optimized_4 (unsigned int X)
+{
+  int result;
+  if ((~X) >> 31)
+    result = 0;
+  else
+    result = 1;
+  return result;
+}
+/* { dg-final { scan-tree-dump-times "0 != 0" 1 "gimple" } } */

Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Senthil Kumar Selvaraj]

Hurugalawadi, Naveen writes:

> Hi,
>
> Please find attached the patch that performs optimization on unsigned values.
>
> Original fold-const part implemented in match.pd.
>
> Please review the patch and let us know if it's OK?
>
> Regression Tested on X86_64 with no regressions.
>
> Thanks,
> Naveen
>
> ChangeLog:
> * match.pd (cmp (convert (bit_not @0)) INTEGER_CST@1): New Simplifier.
>
> Testsuite/ChangeLog:
> * gcc.dg/pr31531.c : New testcase.

Could you please add dg-require-effective-target int32 to the test to
avoid breaking targets with smaller ints (right shift by 31 bits)?

Regards
Senthil

Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Richard Biener]

On Mon, Feb 8, 2016 at 10:16 AM, Senthil Kumar Selvaraj
<senthil_kumar.selvaraj@atmel.com> wrote:
>
> Hurugalawadi, Naveen writes:
>
>> Hi,
>>
>> Please find attached the patch that performs optimization on unsigned values.
>>
>> Original fold-const part implemented in match.pd.
>>
>> Please review the patch and let us know if it's OK?
>>
>> Regression Tested on X86_64 with no regressions.
>>
>> Thanks,
>> Naveen
>>
>> ChangeLog:
>> * match.pd (cmp (convert (bit_not @0)) INTEGER_CST@1): New Simplifier.
>>
>> Testsuite/ChangeLog:
>> * gcc.dg/pr31531.c : New testcase.
>
> Could you please add dg-require-effective-target int32 to the test to
> avoid breaking targets with smaller ints (right shift by 31 bits)?

+/* Fold ((CAST)~X) op C as ((CAST)X) op' ~C, where op' is the
+   swapped comparison.  */
+(for cmp (tcc_comparison)
+     scmp (swapped_tcc_comparison)
+ (simplify
+  (cmp (convert (bit_not @0)) INTEGER_CST@1)
+  (if (TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0))
+       && TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (TREE_TYPE (@1)))
+   (with { tree cst = fold_convert (TREE_TYPE (@0), @1); }
+    (scmp @0 (bit_not { cst; }))))))

I fail to see how this pattern is a good fit for the testcase, can you
try explaining?
I'm wondering that you require 'type's precision (a bool!) to be equal
to that of @0.
I'm also failing to see why you can't enhance the existing

/* Fold ~X op C as X op' ~C, where op' is the swapped comparison.  */
(for cmp (simple_comparison)
     scmp (swapped_simple_comparison)
 (simplify
  (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
  (if (single_use (@2)
       && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
   (scmp @0 (bit_not @1)))))

Thanks,
Richard.

> Regards
> Senthil

Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Hurugalawadi, Naveen]

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

Hi,

>> I'm also failing to see why you can't enhance the existing

Please find attached the patch that enhances the existing pattern.
Please review the patch and let me know if any further modifications
are required.

Thanks,
Naveen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr31531-1.patch --]
[-- Type: text/x-diff; name="pr31531-1.patch", Size: 1749 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 6c8ebd5..bd47a91 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1871,10 +1871,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (for cmp (simple_comparison)
      scmp (swapped_simple_comparison)
  (simplify
-  (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
+  (cmp (convert?@3 (bit_not@2 @0)) CONSTANT_CLASS_P@1)
   (if (single_use (@2)
-       && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
-   (scmp @0 (bit_not @1)))))
+       && ((TREE_CODE (@1) == INTEGER_CST && TREE_TYPE (@3) == TREE_TYPE (@2))
+            || (TREE_CODE (@1) == VECTOR_CST
+		&& (VECTOR_TYPE_P (TREE_TYPE (@3))
+		    == VECTOR_TYPE_P (TREE_TYPE (@2)))
+		&& (TYPE_VECTOR_SUBPARTS (TREE_TYPE (@3))
+		    == TYPE_VECTOR_SUBPARTS (TREE_TYPE (@2)))
+		&& (TYPE_MODE (TREE_TYPE (TREE_TYPE (@3)))
+		    == TYPE_MODE (TREE_TYPE (TREE_TYPE (@2)))))))
+   (scmp @0 (bit_not @1))
+  (if (TYPE_PRECISION (TREE_TYPE (@3)) == TYPE_PRECISION (TREE_TYPE (@2))
+       && (TREE_CODE (@1) == INTEGER_CST))
+   (with { tree newtype = TREE_TYPE (@1); }
+    (scmp (convert:newtype @0) (bit_not @1)))))))
 
 (for cmp (simple_comparison)
  /* Fold (double)float1 CMP (double)float2 into float1 CMP float2.  */
diff --git a/gcc/testsuite/gcc.dg/pr31531.c b/gcc/testsuite/gcc.dg/pr31531.c
new file mode 100644
index 0000000..cf9dd82
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr31531.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-gimple" } */
+/* { dg-require-effective-target int32 } */
+
+int isnegative_optimized_4 (unsigned int X)
+{
+  int result;
+  if ((~X) >> 31)
+    result = 0;
+  else
+    result = 1;
+  return result;
+}
+
+/* { dg-final { scan-tree-dump-times "signed int X.0" 1 "gimple" } } */

[PING] [PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Hurugalawadi, Naveen]

Hi,

Please find attached the patch that enhances the existing pattern.

Please review the patch at the following link and let me know
if there should be any modifications in it:-

https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01035.html

Thanks,
Naveen

[PING2] [PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Hurugalawadi, Naveen]

Hi,

Please find attached the patch that enhances the existing pattern.

Please review the patch at the following link and let me know
if there should be any modifications in it:-

https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01035.html

Thanks,
Naveen

[PING 3] [PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Hurugalawadi, Naveen]

Hi,

Please review the patch at the following link and let me know
if there should be any modifications in it:-

https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01035.html

Thanks,
Naveen

Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Richard Biener]

On Tue, Feb 16, 2016 at 5:50 AM, Hurugalawadi, Naveen
<Naveen.Hurugalawadi@caviumnetworks.com> wrote:
> Hi,
>
>>> I'm also failing to see why you can't enhance the existing
>
> Please find attached the patch that enhances the existing pattern.
> Please review the patch and let me know if any further modifications
> are required.

What's the motivation of splitting this into a equal type (borken for
the vector case)
and a non-equal type case?  Simply only allow nop-conversions here
(tree_nop_conversion_p)
and unconditionally emit

 (scmp (view_convert:newtype @0) (bit_not @1))

?  The conversion will be omitted if it turns out to be not necessary
and a view_convert
will be turned into a regular conversion for non-vector cases.

Richard.

> Thanks,
> Naveen

Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Hurugalawadi, Naveen]

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

Hi Richard,

>> What's the motivation of splitting this into a equal type 
>> and a non-equal type case?  Simply only allow nop-conversions here
>> (tree_nop_conversion_p) and unconditionally emit

Done.

Thanks for the review and Comments. Implemented the modifications
as per you review comments.
Please review the modified patch and let me know if its okay?

Thanks,
Naveen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr31531.patch --]
[-- Type: text/x-diff; name="pr31531.patch", Size: 896 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 75aa601..928a529 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1896,10 +1896,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (for cmp (simple_comparison)
      scmp (swapped_simple_comparison)
  (simplify
-  (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
+  (cmp (convert?@3 (bit_not@2 @0)) CONSTANT_CLASS_P@1)
   (if (single_use (@2)
-       && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
-   (scmp @0 (bit_not @1)))))
+       && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)
+       && (TYPE_PRECISION (TREE_TYPE (@3)) == TYPE_PRECISION (TREE_TYPE (@2)))
+       && tree_nop_conversion_p (type, TREE_TYPE (@0)))
+   (with { tree newtype = TREE_TYPE (@1); }
+    (scmp (view_convert:newtype @0) (bit_not @1))))))
 
 (for cmp (simple_comparison)
  /* Fold (double)float1 CMP (double)float2 into float1 CMP float2.  */

Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

[Marc Glisse]

--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1896,10 +1896,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (for cmp (simple_comparison)
       scmp (swapped_simple_comparison)
   (simplify
-  (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
+  (cmp (convert?@3 (bit_not@2 @0)) CONSTANT_CLASS_P@1)
    (if (single_use (@2)
-       && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
-   (scmp @0 (bit_not @1)))))
+       && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)
+       && (TYPE_PRECISION (TREE_TYPE (@3)) == TYPE_PRECISION (TREE_TYPE (@2)))
+       && tree_nop_conversion_p (type, TREE_TYPE (@0)))
+   (with { tree newtype = TREE_TYPE (@1); }
+    (scmp (view_convert:newtype @0) (bit_not @1))))))

Could you explain in English what you are doing?
'type' is always the outer type, in this case the result of the 
comparison, could be boolean for instance. Asking for 
tree_nop_conversion_p between @0 and that type seems strange. Also, you 
are not allowed to use TYPE_PRECISION on a vector type.

-- 
Marc Glisse