On 02/07/2018 08:36 AM, Qing Zhao wrote: > Hi, this is the 3rd version for this patch. > > the main change compared with 2nd version are: > 1. do not use “compute_objsize” to get the minimum object size per Jeff > and Richard’s > comment. Instead, add a new function “determine_min_objsize” for this > purpose. This new > function calls “compute_builtin_object_size” to get the minimum objsize, > however, due to > the fact that “compute_builtin_object_size” does nothing for SSA_NAME when > optimize > 0 (don’t > know the exactly reason for this), inside “determine_min_objsize”, I have to > add more code > to catch up some simple SSA_NAME cases. > > 2. in gimple-fold.c and tree-ssa-structalias.c, add the handling of the > new > BUILT_IN_STRCMP_EQ and BUILT_IN_STRNCMP_EQ in the same places where > BUILT_IN_STRCMP and BUILT_IN_STRNCMP is checked. > > 3. some format change and comments change per Jeff’s comment. > > let me know if you have any comments. > > thanks a lot. > > Qing > > ********************************* > > 2nd Patch for PR78009 > Patch for PR83026 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809 > Inline strcmp with small constant strings > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83026 > missing strlen optimization for strcmp of unequal strings > > The design doc for PR78809 is at: > https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html > > this patch is for the second part of change of PR78809 and PR83026: > > B. for strncmp (s1, s2, n) (!)= 0 or strcmp (s1, s2) (!)= 0 > > B.1. (PR83026) When the lengths of both arguments are constant and > it's a strcmp: > * if the lengths are NOT equal, we can safely fold the call > to a non-zero value. > * otherwise, do nothing now. > > B.2. (PR78809) When the length of one argument is constant, try to replace > the call with a __builtin_str(n)cmp_eq call where possible, i.e: > > strncmp (s, STR, C) (!)= 0 in which, s is a pointer to a string, STR is a > string with constant length, C is a constant. > if (C <= strlen(STR) && sizeof_array(s) > C) > { > replace this call with > __builtin_strncmp_eq (s, STR, C) (!)= 0 > } > if (C > strlen(STR) > { > it can be safely treated as a call to strcmp (s, STR) (!)= 0 > can handled by the following strcmp. > } > > strcmp (s, STR) (!)= 0 in which, s is a pointer to a string, STR is a > string with constant length. > if (sizeof_array(s) > strlen(STR)) > { > replace this call with > __builtin_strcmp_eq (s, STR, strlen(STR)+1) (!)= 0 > } > > later when expanding the new __builtin_str(n)cmp_eq calls, first expand them > as __builtin_memcmp_eq, if the expansion does not succeed, change them back > to call to __builtin_str(n)cmp. > > adding test case strcmpopt_2.c and strcmpopt_4.c into gcc.dg for part B of > PR78809 adding test case strcmpopt_3.c into gcc.dg for PR83026 > > bootstraped and tested on both X86 and Aarch64. no regression. > > ************************************************ > gcc/ChangeLog > > +2018-02-02 <qing.z...@oracle.com> > + > + PR middle-end/78809 > + PR middle-end/83026 > + * builtins.c (expand_builtin): Add the handling of BUILT_IN_STRCMP_EQ > + and BUILT_IN_STRNCMP_EQ. > + * builtins.def: Add new builtins BUILT_IN_STRCMP_EQ and > + BUILT_IN_STRNCMP_EQ. > + * gimple-fold.c (gimple_fold_builtin_string_compare): Add the > + handling of BUILTIN_IN_STRCMP_EQ and BUILT_IN_STRNCMP_EQ. > + (gimple_fold_builtin): Likewise. > + * tree-ssa-strlen.c (compute_string_length): New function. > + (determine_min_obsize): New function. > + (handle_builtin_string_cmp): New function to handle calls to > + string compare functions. > + (strlen_optimize_stmt): Add handling to builtin string compare > + calls. > + * tree-ssa-structalias.c (find_func_aliases_for_builtin_call): > + Add the handling of BUILT_IN_STRCMP_EQ and BUILT_IN_STRNCMP_EQ. > + * tree.c (build_common_builtin_nodes): Add new defines of > + BUILT_IN_STRNCMP_EQ and BUILT_IN_STRCMP_EQ. > + > > gcc/testsuite/ChangeLog > > +2018-02-02 <qing.z...@oracle.com> > + > + PR middle-end/78809 > + * gcc.dg/strcmpopt_2.c: New testcase. > + * gcc.dg/strcmpopt_3.c: New testcase. > + > + PR middle-end/83026 > + * gcc.dg/strcmpopt_3.c: New testcase. > > > > + > +/* Handle a call to strcmp or strncmp. When the result is ONLY used to do > + equality test against zero: > + > + A. When the lengths of both arguments are constant and it's a strcmp: > + * if the lengths are NOT equal, we can safely fold the call > + to a non-zero value. > + * otherwise, do nothing now. > + > + B. When the length of one argument is constant, try to replace the call > with > + a __builtin_str(n)cmp_eq call where possible, i.e: > + > + strncmp (s, STR, C) (!)= 0 in which, s is a pointer to a string, STR is a > + string with constant length , C is a constant. > + if (C <= strlen(STR) && sizeof_array(s) > C) > + { > + replace this call with > + strncmp_eq (s, STR, C) (!)= 0 > + } > + if (C > strlen(STR) > + { > + it can be safely treated as a call to strcmp (s, STR) (!)= 0 > + can handled by the following strcmp. > + } > + > + strcmp (s, STR) (!)= 0 in which, s is a pointer to a string, STR is a > + string with constant length. > + if (sizeof_array(s) > strlen(STR)) > + { > + replace this call with > + strcmp_eq (s, STR, strlen(STR)+1) (!)= 0 > + } > + > + Return false when the call is transformed, return true otherwise. > + */ > + > +static bool > +handle_builtin_string_cmp (gimple_stmt_iterator *gsi) So I originally thought you had the core logic wrong in the immediate uses loop. But it's actually the case that the return value is the exact opposite of what I expected.
ie, I expected "TRUE" to mean the call was transformed, "FALSE" if it was not transformed. Can you fix that so it's not so confusing? I think with that change we'll be good to go, but please repost for a final looksie. THanks, Jeff
