Hi, Wilco,

thanks a lot for your review and comments.

> On Dec 15, 2017, at 6:41 AM, Wilco Dijkstra <wilco.dijks...@arm.com> wrote:
> 
> Hi Qing,
> 
> Just looking at a very high level, I have a few comments:
> 
> 1. Constant folding str(n)cmp - folding is done separately in 
> fold-const-call.c
>   and gimple-fold.c.  There is already code for folding strcmp and strncmp,
>   so we shouldn't need to add new foldings.  Or do you have an example that
>   isn't folded as expected? If so, a fix should be added to the existing code.

are you referring the following:

 B.1. (PR83026) When both arguments are constant strings and it's a strcmp:
    * if the length of the strings are NOT equal, we can safely fold the call
      to a non-zero value.
    * otherwise, do nothing now.


the following testing case cannot be folded by the current gimple-fold or 
fold-const-call:

int f1 (void)
{
  char *s0= "abcd";
  char s[8];
  strcpy (s, s0);
  return __builtin_strcmp(s, "abc") != 0;
}

in order to fold the above call, we need the strlen information that provided 
by the tree-ssa-strlen.c.
and I don’t think that improving the gimple-fold or fold-const-call can fold 
the above call to 1. 

with the strlen info in tree-ssa-strlen.c,  the strlen of s could be determined 
as 4, which is not equal
to the strlen of the constant string “abc”, then we can safely fold 

__builtin_strcmp(s, "abc") != 0

to 1. 

(actually, your above  comments remind me that one of my testcases added has 
some issue, I will fix the testcase,
strcmpopt_3.c). 

> 
> 2. Why check for str(n)cmp == 0 / != 0? There is no need to explicitly check
>   for equality comparisons since folding into memcmp is always good.

If the result is Not used to compare with 0, then we need the exact value of 
strcmp of two strings.  negative value, zero, or positive value.

I am not sure changing str(n)cmp to memcmp under such situation will still 
return the correct negative or positive value?

> 
> 3. Why handle strncmp? There is already code to convert strncmp into strcmp,
>   so why repeat that again in a different way? It just seems to make the
>   code significantly more complex for no benefit.

if strncmp cannot be convert to strcmp, there is still call to strncmp that we 
should handle, right? 

Qing


> 
> You can achieve the same effect by just optimizing strcmp into memcmp when
> legal without checking for equality comparison.  As a result you can 
> considerably
> reduce the size of this patch while handling more useful cases.
> 
> Wilco
> 

Reply via email to