Re: [PATCH] Improve VPR for some builtins and non pointer checks

[Andrew Pinski <andrew dot pinski at caviumnetworks dot com>]

On Mon, Sep 3, 2012 at 12:03 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Sep 02, 2012 at 10:18:15PM -0700, Andrew Pinski wrote:
>>   While fixing some code not to have aliasing violations in it, I can
>> across that some builtins were not causing their arguments or their
>> return values being marked as non-null.  This patch implements just
>> that in VPR while allowing to remove some null pointer checks later
>> on.
>>
>> OK?  Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>> @@ -1057,6 +1057,20 @@ vrp_stmt_computes_nonzero (gimple stmt,
>>       }
>>      }
>>
>> +  /* With some builtins, we can infer if the pointer return value
>> +     will be non null.  */
>> +  if (flag_delete_null_pointer_checks
>> +      && is_gimple_call (stmt) && gimple_call_fndecl (stmt)
>> +      && DECL_BUILT_IN_CLASS (gimple_call_fndecl (stmt)) == BUILT_IN_NORMAL)
>> +    {
>> +      switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt)))
>> +     {
>> +       case BUILT_IN_MEMCPY:
>> +       case BUILT_IN_MEMMOVE:
>> +         return true;
>> +     }
>> +    }
>> +
>>    return false;
>>  }
>>
>
> That is too hackish and lists way too few builtins.
> If you rely on nonnull attribute marked builtins, I'd say you want
> flags = gimple_call_return_flags (stmt);
> if ((flags & ERF_RETURNS_ARG)
>     && (flags & ERF_RETURN_ARG_MASK) < gimple_call_num_args (stmt))
>   {
>     /* Test nonnull attribute on the decl, either argument-less or
>        on the (flags & ERF_RETURN_ARG_MASK)th argument.  */
>   }
> Or at least handle builtins e.g. CCP handles as pass-thru arg1:
> BUILT_IN_MEMCPY, BUILT_IN_MEMMOVE, BUILT_IN_MEMSET, BUILT_IN_STRCPY,
> BUILT_IN_STRNCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_MEMMOVE_CHK,
> BUILT_IN_MEMSET_CHK, BUILT_IN_STRCPY_CHK, BUILT_IN_STRNCPY_CHK
> (which reminds me that some of these apparently aren't marked
> with ATTR_RET1_NOTHROW_NONNULL_LEAF, why?).


I did not know of these attributes and flags on the decl.  Yes I know
it only lists only a few of the builtins.
I will look into those flags and see if I can improve it.

>
>> @@ -4231,6 +4245,32 @@ infer_value_range (gimple stmt, tree op,
>>       }
>>      }
>>
>> +  /* With some builtins, we can infer if the pointer argument
>> +     will be non null.  */
>> +  if (flag_delete_null_pointer_checks
>> +      && is_gimple_call (stmt) && gimple_call_fndecl (stmt))
>> +    {
>> +      tree callee = gimple_call_fndecl (stmt);
>> +      if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL)
>> +     {
>> +       switch (DECL_FUNCTION_CODE (callee))
>> +         {
>> +           case BUILT_IN_MEMCPY:
>> +           case BUILT_IN_MEMMOVE:
>> +           case BUILT_IN_STRCMP:
>> +           case BUILT_IN_MEMCMP:
>> +             /* The first and second arguments of memcpy and memmove will be non null after the call. */
>> +             if (gimple_call_arg (stmt, 0) == op
>> +                 || gimple_call_arg (stmt, 1) == op)
>> +               {
>> +                 *val_p = build_int_cst (TREE_TYPE (op), 0);
>> +                 *comp_code_p = NE_EXPR;
>> +                 return true;
>> +               }
>> +         }
>> +     }
>> +    }
>
> Again, what you are looking for here?  Passing pointers to nonnull
> attributes, or something more specific?  What exactly?  There are tons of
> builtins that behave similarly to memcpy/memmove/strcmp/memcmp.

Passing pointers arguments to nonnull attributed functions.  I will
see if I can use the nonnull attribute on those functions so that this
can be done without checking the builtin functions.

Thanks,
Andrew Pinski

>
>         Jakub