Re: fold strlen (s) eq/ne 0 to *s eq/ne 0 on GIMPLE

[Richard Biener <rguenther at suse dot de>]

On Mon, 1 Aug 2016, Andrew Pinski wrote:

> On Mon, Aug 1, 2016 at 12:22 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> > On Mon, Aug 1, 2016 at 12:15 AM, Prathamesh Kulkarni
> > <prathamesh.kulkarni@linaro.org> wrote:
> >> Hi Richard,
> >> The attached patch tries to fold strlen (s) eq/ne 0 to *s eq/ne 0 on GIMPLE.
> >> I am not sure where was the ideal place to put this transform in and ended up
> >> adding it to strlen_optimize_stmt().
> >> Does that look OK ?
> >
> > I suspect it might be better in match.pd.
> 
> The main reason is it is already in fold-const.c:
>       /* Optimize comparisons of strlen vs zero to a compare of the
>          first character of the string vs zero.  To wit,
>                 strlen(ptr) == 0   =>  *ptr == 0
>                 strlen(ptr) != 0   =>  *ptr != 0
>          Other cases should reduce to one of these two (or a constant)
>          due to the return value of strlen being unsigned.  */
>       if (TREE_CODE (arg0) == CALL_EXPR
>           && integer_zerop (arg1))
>         {
>           tree fndecl = get_callee_fndecl (arg0);
> 
>           if (fndecl
>               && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
>               && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRLEN
>               && call_expr_nargs (arg0) == 1
>               && TREE_CODE (TREE_TYPE (CALL_EXPR_ARG (arg0, 0))) ==
> POINTER_TYPE)
>             {
>               tree iref = build_fold_indirect_ref_loc (loc,
>                                                    CALL_EXPR_ARG (arg0, 0));
>               return fold_build2_loc (loc, code, type, iref,
>                                   build_int_cst (TREE_TYPE (iref), 0));
>             }
>         }
> 
> So you are basically moving that to match.pd instead of adding extra code.

The issue is that we currently cannot update virtual operands in most
cases so

(simplify
 (ne (BUILT_IN_STRLEN @0) integer_zerop)
 (ne (MEM_REF:char_type_node @0 { build_int_cst (ptr_type_node, 0); } ) { 
build_zero_cst (char_type_node); } ))

will work but ICE on

int foo (const char *s)
{
  int a = 0;
  return __builtin_strlen (s) != a;
}

when optimizing.  I didn't yet arrive at a good way to solve this issue.
The most straight-forward way would maybe be sth explicit, like

(simplify
 (ne (BUILT_IN_STRLEN:vop@2 @0) integer_zerop)
 (ne (MEM_REF:char_type_node:vop@2 @0 { ...

but inventing a good syntax here is difficult.  Basically you have to
connect memory references in the matched pattern with those in the
replacement.

There is also the possibility to at least capture an existing VUSE during 
matching
and make sure to set that on the replacement.  That would work
automagically but doing this more generically to also handle
stores is difficult.

Note that it all works if the replacement has the only memory
reference in the last stmt.

Richard.