[PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Qing Zhao]

Hi,

this is the first patch for PR78809 (totally 3 patches)

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
inline strcmp with small constant strings

The design doc is at:
https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html

this patch is for the first part of change:

A. for strncmp (s1, s2, n)
     if one of "s1" or "s2" is a constant string, "n" is a constant, and
larger than the length of the constant string:
     change strncmp (s1, s2, n) to strcmp (s1, s2);

adding test case strcmpopt_1.c into gcc.dg

bootstraped and tested on both X86 and aarch64. no regression.

Okay for commit?

thanks.

Qing

======================

gcc/ChangeLog

2017-11-15  Qing Zhao  <qing.zhao@oracle.com>

       * gimple-fold.c (gimple_fold_builtin_string_compare): Add handling
       of replacing call to strncmp with corresponding call to strcmp when
       meeting conditions.

gcc/testsuite/ChangeLog

2017-11-15  Qing Zhao  <qing.zhao@oracle.com>

       PR middle-end/78809
       * gcc.dg/strcmpopt_1.c: New test.

---
 gcc/gimple-fold.c                  | 15 +++++++++++++++
 gcc/testsuite/gcc.dg/strcmpopt_1.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/strcmpopt_1.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index adb6f3b..1ed6383 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2258,6 +2258,21 @@ gimple_fold_builtin_string_compare (gimple_stmt_iterator *gsi)
       return true;
     }
 
+  /* If length is larger than the length of one constant string, 
+     replace strncmp with corresponding strcmp */ 
+  if (fcode == BUILT_IN_STRNCMP 
+      && length > 0
+      && ((p2 && (size_t) length > strlen (p2)) 
+          || (p1 && (size_t) length > strlen (p1))))
+    {
+      tree fn = builtin_decl_implicit (BUILT_IN_STRCMP);
+      if (!fn)
+        return false;
+      gimple *repl = gimple_build_call (fn, 2, str1, str2);
+      replace_call_with_call_and_fold (gsi, repl);
+      return true;
+    }
+
   return false;
 }
 
diff --git a/gcc/testsuite/gcc.dg/strcmpopt_1.c b/gcc/testsuite/gcc.dg/strcmpopt_1.c
new file mode 100644
index 0000000..40596a2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strcmpopt_1.c
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-options "-fdump-tree-gimple" } */
+
+#include <string.h>
+#include <stdlib.h>
+
+int cmp1 (char *p)
+{
+  return strncmp (p, "fis", 4);
+}
+int cmp2 (char *q)
+{
+  return strncmp ("fis", q, 4);
+}
+
+int main ()
+{
+
+  char *p = "fish";
+  char *q = "fis\0";
+
+  if (cmp1 (p) == 0 || cmp2 (q) != 0)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "strcmp \\(" 2 "gimple" } } */
-- 
1.9.1

Re: [PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Jeff Law]

On 11/15/2017 08:00 AM, Qing Zhao wrote:
> Hi,
> 
> this is the first patch for PR78809 (totally 3 patches)
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
> inline strcmp with small constant strings
> 
> The design doc is at:
> https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html
> 
> this patch is for the first part of change:
> 
> A. for strncmp (s1, s2, n)
>      if one of "s1" or "s2" is a constant string, "n" is a constant, and
> larger than the length of the constant string:
>      change strncmp (s1, s2, n) to strcmp (s1, s2);
> 
> adding test case strcmpopt_1.c into gcc.dg
> 
> bootstraped and tested on both X86 and aarch64. no regression.
> 
> Okay for commit?
> 
> thanks.
> 
> Qing
> 
> ======================
> 
> gcc/ChangeLog
> 
> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com>
> 
>        * gimple-fold.c (gimple_fold_builtin_string_compare): Add handling
>        of replacing call to strncmp with corresponding call to strcmp when
>        meeting conditions.
> 
> gcc/testsuite/ChangeLog
> 
> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com>
> 
>        PR middle-end/78809
>        * gcc.dg/strcmpopt_1.c: New test.
This is OK for the trunk.

Please put the PR middle-end/78809 marker in the main ChangeLog entry as
well.

Do you have write access to the repository?

jeff

Re: [PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Qing Zhao]

> On Nov 16, 2017, at 6:57 PM, Jeff Law <law@redhat.com> wrote:
> 
> On 11/15/2017 08:00 AM, Qing Zhao wrote:
>> Hi,
>> 
>> this is the first patch for PR78809 (totally 3 patches)
>> 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
>> inline strcmp with small constant strings
>> 
>> The design doc is at:
>> https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html
>> 
>> this patch is for the first part of change:
>> 
>> A. for strncmp (s1, s2, n)
>>     if one of "s1" or "s2" is a constant string, "n" is a constant, and
>> larger than the length of the constant string:
>>     change strncmp (s1, s2, n) to strcmp (s1, s2);
>> 
>> adding test case strcmpopt_1.c into gcc.dg
>> 
>> bootstraped and tested on both X86 and aarch64. no regression.
>> 
>> Okay for commit?
>> 
>> thanks.
>> 
>> Qing
>> 
>> ======================
>> 
>> gcc/ChangeLog
>> 
>> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com>
>> 
>>       * gimple-fold.c (gimple_fold_builtin_string_compare): Add handling
>>       of replacing call to strncmp with corresponding call to strcmp when
>>       meeting conditions.
>> 
>> gcc/testsuite/ChangeLog
>> 
>> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com>
>> 
>>       PR middle-end/78809
>>       * gcc.dg/strcmpopt_1.c: New test.
> This is OK for the trunk.
> 
> Please put the PR middle-end/78809 marker in the main ChangeLog entry as
> well.

Okay.  (as following?)

2017-11-15  Qing Zhao <qing.zhao@oracle.com>

        PR middle-end/78809
        * gimple-fold.c (gimple_fold_builtin_string_compare): Add handling
        of replacing call to strncmp with corresponding call to strcmp when
        meeting conditions.


gcc/testsuite/ChangeLog

2017-11-15  Qing Zhao  <qing.zhao@oracle.com <mailto:qing.zhao@oracle.com>>

      PR middle-end/78809
      * gcc.dg/strcmpopt_1.c: New test.

> 
> Do you have write access to the repository?

No, I don’t have. 
Qing


> 
> jeff

Re: [PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Jeff Law]

On 11/16/2017 06:53 PM, Qing Zhao wrote:
> 
>> On Nov 16, 2017, at 6:57 PM, Jeff Law <law@redhat.com
>> <mailto:law@redhat.com>> wrote:
>>
>> On 11/15/2017 08:00 AM, Qing Zhao wrote:
>>> Hi,
>>>
>>> this is the first patch for PR78809 (totally 3 patches)
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
>>> inline strcmp with small constant strings
>>>
>>> The design doc is at:
>>> https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html
>>>
>>> this patch is for the first part of change:
>>>
>>> A. for strncmp (s1, s2, n)
>>>     if one of "s1" or "s2" is a constant string, "n" is a constant, and
>>> larger than the length of the constant string:
>>>     change strncmp (s1, s2, n) to strcmp (s1, s2);
>>>
>>> adding test case strcmpopt_1.c into gcc.dg
>>>
>>> bootstraped and tested on both X86 and aarch64. no regression.
>>>
>>> Okay for commit?
>>>
>>> thanks.
>>>
>>> Qing
>>>
>>> ======================
>>>
>>> gcc/ChangeLog
>>>
>>> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com>
>>>
>>>       * gimple-fold.c (gimple_fold_builtin_string_compare): Add handling
>>>       of replacing call to strncmp with corresponding call to strcmp when
>>>       meeting conditions.
>>>
>>> gcc/testsuite/ChangeLog
>>>
>>> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com>
>>>
>>>       PR middle-end/78809
>>>       * gcc.dg/strcmpopt_1.c: New test.
>> This is OK for the trunk.
>>
>> Please put the PR middle-end/78809 marker in the main ChangeLog entry as
>> well.
> 
> Okay.  (as following?)
> 
> 2017-11-15  Qing Zhao <qing.zhao@oracle.com <mailto:qing.zhao@oracle.com>>
> 
>         PR middle-end/78809
>         * gimple-fold.c (gimple_fold_builtin_string_compare): Add handling
>         of replacing call to strncmp with corresponding call to strcmp when
>         meeting conditions.
> 
> 
> gcc/testsuite/ChangeLog
> 
> 2017-11-15  Qing Zhao  <qing.zhao@oracle.com <mailto:qing.zhao@oracle.com>>
> 
>       PR middle-end/78809
>       * gcc.dg/strcmpopt_1.c: New test.
> 
>>
>> Do you have write access to the repository?
> 
> No, I don’t have. 
OK.  I'll go ahead and commit for you.

I think this patch is small enough to not require a copyright
assignment.  However further work likely will.  I don't offhand know if
Oracle has a blanket assignment in place.  Can you work with Paolo to
ensure that the proper paperwork is in place so that future
contributions don't get held up on legal stuff.



Thanks,
jeff

Re: [PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Paolo Carlini]

Hi,

On 17/11/2017 06:29, Jeff Law wrote:
> OK. I'll go ahead and commit for you.
Beautiful. Thanks Jeff.
> I think this patch is small enough to not require a copyright
> assignment.  However further work likely will.  I don't offhand know if
> Oracle has a blanket assignment in place.  Can you work with Paolo to
> ensure that the proper paperwork is in place so that future
> contributions don't get held up on legal stuff.
Oracle is fine, has a blanket assignment, thus Qing is already covered 
and I think this one is her third small (but useful ;) contribution so far.

While we are at it, I'm thinking that probably Qing could be added to 
sourceware.org, if you don't disagree. Qing, if nobody objects over the 
next day or so, please go ahead, submit this form (include my name as 
sponsor):

     https://sourceware.org/cgi-bin/pdw/ps_form.cgi

Thanks again!
Paolo.

Re: [PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Qing Zhao]

thanks Jeff and Paolo. 

really appreciate  for all the help so far.

Qing
> On Nov 17, 2017, at 3:17 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> 
> Hi,
> 
> On 17/11/2017 06:29, Jeff Law wrote:
>> OK. I'll go ahead and commit for you.
> Beautiful. Thanks Jeff.
>> I think this patch is small enough to not require a copyright
>> assignment.  However further work likely will.  I don't offhand know if
>> Oracle has a blanket assignment in place.  Can you work with Paolo to
>> ensure that the proper paperwork is in place so that future
>> contributions don't get held up on legal stuff.
> Oracle is fine, has a blanket assignment, thus Qing is already covered and I think this one is her third small (but useful ;) contribution so far.
> 
> While we are at it, I'm thinking that probably Qing could be added to sourceware.org, if you don't disagree. Qing, if nobody objects over the next day or so, please go ahead, submit this form (include my name as sponsor):
> 
>     https://sourceware.org/cgi-bin/pdw/ps_form.cgi
> 
> Thanks again!
> Paolo.

Re: [PATCH 1/3][middle-end]PR78809 (Inline strcmp with small constant strings)

[Jeff Law]

On 11/17/2017 02:17 AM, Paolo Carlini wrote:
> Hi,
> 
> On 17/11/2017 06:29, Jeff Law wrote:
>> OK. I'll go ahead and commit for you.
> Beautiful. Thanks Jeff.
>> I think this patch is small enough to not require a copyright
>> assignment.  However further work likely will.  I don't offhand know if
>> Oracle has a blanket assignment in place.  Can you work with Paolo to
>> ensure that the proper paperwork is in place so that future
>> contributions don't get held up on legal stuff.
> Oracle is fine, has a blanket assignment, thus Qing is already covered
> and I think this one is her third small (but useful ;) contribution so far.
> 
> While we are at it, I'm thinking that probably Qing could be added to
> sourceware.org, if you don't disagree. Qing, if nobody objects over the
> next day or so, please go ahead, submit this form (include my name as
> sponsor):
> 
>     https://sourceware.org/cgi-bin/pdw/ps_form.cgi
Seems quite reasonable to me.

jeff