Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Wed, 6 Apr 2022, Jakub Jelinek wrote:
>
>> On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote:
>> > On Tue, 5 Apr 2022, Jakub Jelinek wrote:
>> > 
>> > > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote:
>> > > > > In GIMPLE, we call:
>> > > > >   && gimple_builtin_call_types_compatible_p (stmt, fndecl)
>> > > > > but that is insufficient, that verifies whether the arguments passed 
>> > > > > to
>> > > > > GIMPLE_CALL match the fndecl argument types.  But that fndecl may 
>> > > > > very well
>> > > > > be the user declaration, so doesn't have to match exactly the builtin
>> > > > > as predeclared by builtins.def etc. (though, there is the cotcha 
>> > > > > that say
>> > > > > for FILE * we just use void * etc.).  So e.g. in tree-ssa-strlen.cc
>> > > > > we use additional:
>> > > > >   tree callee = gimple_call_fndecl (stmt);
>> > > > >   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));
>> > > > >   if (decl
>> > > > >       && decl != callee
>> > > > >       && !gimple_builtin_call_types_compatible_p (stmt, decl))
>> > > > >     return false;
>> > > > 
>> > > > Yeah, I think we should use that (and only that) function decl
>> > > > in get_call_combined_fn and gimple_call_combined_fn until the
>> > > > frontend stops slapping wrong BUILT_IN_* on random decls.
>> > > 
>> > > So, as a preparation step, this patch adjusts gimple_call_builtin_p
>> > > and gimple_call_combined_fn so that they check argument types against
>> > > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the
>> > > actual used fndecl.
>> > > 
>> > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
>> > 
>> > You forgot to attach the patch ...
>> 
>> Oops, here it is.  Also bootstrapped/regtested on {x86_64,i686}-linux.
>
> OK.

But it seems like the magic incantation to detect “real” built-in
function calls is getting longer and longer.  Can we not abstract this
in a single place rather than have to repeat the same long sequence in
multiple places?

Thanks,
Richard

>
> Thanks,
> Richard.
>
>> 2022-04-06  Jakub Jelinek  <ja...@redhat.com>
>> 
>>      PR tree-optimization/105150
>>      * gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn):
>>      For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p
>>      preferrably on builtin_decl_explicit decl rather than fndecl.
>>      * tree-ssa-strlen.cc (valid_builtin_call): Don't call
>>      gimple_builtin_call_types_compatible_p here.
>> 
>> --- gcc/gimple.cc.jj 2022-02-09 15:15:59.436837973 +0100
>> +++ gcc/gimple.cc    2022-04-05 13:04:58.405586995 +0200
>> @@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm
>>    if (is_gimple_call (stmt)
>>        && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>        && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
>> -    return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +    {
>> +      if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>> +    if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> +      fndecl = decl;
>> +      return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +    }
>>    return false;
>>  }
>>  
>> @@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm
>>    if (is_gimple_call (stmt)
>>        && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>        && DECL_BUILT_IN_CLASS (fndecl) == klass)
>> -    return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +    {
>> +      if (klass == BUILT_IN_NORMAL)
>> +    if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> +      fndecl = decl;
>> +      return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +    }
>>    return false;
>>  }
>>  
>> @@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm
>>    if (is_gimple_call (stmt)
>>        && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>        && fndecl_built_in_p (fndecl, code))
>> -    return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +    {
>> +      if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> +    fndecl = decl;
>> +      return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +    }
>>    return false;
>>  }
>>  
>> @@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s
>>      return as_combined_fn (gimple_call_internal_fn (call));
>>  
>>        tree fndecl = gimple_call_fndecl (stmt);
>> -      if (fndecl
>> -      && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
>> -      && gimple_builtin_call_types_compatible_p (stmt, fndecl))
>> -    return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>> +      if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
>> +    {
>> +      tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
>> +      if (!decl)
>> +        decl = fndecl;
>> +      if (gimple_builtin_call_types_compatible_p (stmt, decl))
>> +        return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>> +    }
>>      }
>>    return CFN_LAST;
>>  }
>> --- gcc/tree-ssa-strlen.cc.jj        2022-03-30 09:11:53.310103911 +0200
>> +++ gcc/tree-ssa-strlen.cc   2022-04-05 12:22:56.023057416 +0200
>> @@ -1736,12 +1736,6 @@ valid_builtin_call (gimple *stmt)
>>      return false;
>>  
>>    tree callee = gimple_call_fndecl (stmt);
>> -  tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));
>> -  if (decl
>> -      && decl != callee
>> -      && !gimple_builtin_call_types_compatible_p (stmt, decl))
>> -    return false;
>> -
>>    switch (DECL_FUNCTION_CODE (callee))
>>      {
>>      case BUILT_IN_MEMCMP:
>> 
>> 
>>      Jakub
>> 
>> 

Reply via email to