[PATCH][Middle-end][version 3]2nd patch of PR78809 and PR83026

[Jeff Law]

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.zhao@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.zhao@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