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

[Jakub Jelinek]

On Fri, Dec 15, 2017 at 11:17:37AM -0600, Qing Zhao wrote:
>   HOST_WIDE_INT const_string_leni = -1;
> 
>   if (idx1)
>     {
>       const_string_leni = compute_string_length (idx1);
>       var_string = arg2;
>     }
>   else if (idx2)
>     {
>       const_string_leni = compute_string_length (idx2);
>       var_string = arg1;
>     }
> 
> so, the -Wmaybe-uninitialized should NOT issue warning, right?

Well, you had the var_string var uninitialized, so that is what I was
talking about.

> but anyway, I can change the above as following:
> 
>  HOST_WIDE_INT const_string_leni = -1;

And here you don't need to initialize it.

>   if (idx1)
>     {
>       const_string_leni = compute_string_length (idx1);
>       var_string = arg2;
>     }
>   else
>     {
>       gcc_assert (idx2);
>       const_string_leni = compute_string_length (idx2);
>       var_string = arg1;
>     }
> 
> is this better?

Yes, though the gcc_assert could be just gcc_checking_assert (idx2);

> > so that would be mode 2, though that
> > mode isn't actually used in real-world code and thus might be not fully
> > tested.
> 
> so, using this routine with mode 2 should be the right approach to go? 
> and we need fully testing on this too?

It has been a while since I wrote it, so it would need careful analysis.

> >> 	B. use “get_range_strlen” in gimple-fold.h to decide the size of the object.  however, 
> >> it cannot provide valid info for simple array, either. 
> > 
> > get_range_strlen returns you a range, the minval is not what you're looking
> > for, that is the minimum string length, so might be too short for your
> > purposes.  And maxval is an upper bound, but you are looking for lower
> > bound, you need guarantees this amount of memory can be accessed, even if
> > there is 0 in the first byte.
> 
> my understanding is that: get_range_strlen returns the minimum and maximum length of the string pointed by the 
> pointer, and the maximum length of the string is determined by the size of the allocated memory pointed by the
> pointer, so, it should serve my purpose,   did I misunderstand it?

What I'm worried about is:
struct S { int a; char b[64]; };
struct T { struct S c; char d; };
int
foo (struct T *x)
{
  return strcmp (x->c.b, "01234567890123456789012345678901234567890123456789") == 0;
}
int
bar (void)
{
  struct S *p = malloc (offsetof (struct S, b) + 8);
  p->a = 123;
  strcpy (p->b, "0123456");
  return foo ((struct T *) p);
}
etc.  where if you transform that into memcmp (x->c.b, "01234567890123456789012345678901234567890123456789", 51) == 0
it will segfault, whereas strcmp would not.

> >  But if we find out during
> > expansion we don't want to expand it inline, we should fall back to calling
> > strcmp or strncmp.
> 
> under what situation we will NOT expand the memcpy_eq call inline?

      target = expand_builtin_memcmp (exp, target, fcode == BUILT_IN_MEMCMP_EQ);
      if (target)
        return target;
      if (fcode == BUILT_IN_MEMCMP_EQ)
        {
          tree newdecl = builtin_decl_explicit (BUILT_IN_MEMCMP);
          TREE_OPERAND (exp, 1) = build_fold_addr_expr (newdecl);
        }
is what builtins.c has, so it certainly counts with the possibility.
Now, both expand_builtin_memcmp, and emit_block_cmp_hints has several cases
when it fails.  E.g. can_do_by_pieces decides it is too expensive to do it
inline, and emit_block_cmp_via_cmpmem fails because the target doesn't have
cmpmemsi expander.  Various other cases.

Also, note that some target might have cmpstr*si expanders implemented, but
not cmpmemsi, in which case trying to optimize strcmp as memcmp_eq might be a
severe pessimization.

	Jakub