| Message ID | DFBEFF33-8012-4631-9117-52E82C68545E@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3,middle-end] PR78809 (Inline strcmp with small constant strings) | expand |
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
> 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
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
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.
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.
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
======================
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" } } */
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