Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Richard Sandiford <richard.sandiford@arm.com>]

Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote:
>> On Wed, 6 Apr 2022, Jakub Jelinek wrote:
>> 
>> > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote:
>> > > 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?
>> > 
>> > I've already committed it, so it can be only dealt with an incremental
>> > patch.
>> > One possibility is to do it inside of
>> > gimple_builtin_call_types_compatible_p, after the assert do that:
>> >   if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>> >     if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> >       fndecl = decl;
>> > but we then lose the theoretical possibility of comparing against the
>> > actual user declaration.  Though I guess in the
>> > gimple-fold.cc
>> > gimple-low.cc
>> > gimple-match-head.cc
>> > calls to that function we also want this rather than what they do currently.
>> 
>> Yes, I think it would be clearer to pass a BUILT_IN_* code to
>> gimple_builtin_call_types_compatible_p and no decl and simply return
>> false if we cannot get out hands at the "proper" decl from
>> builtin_decl_explicit ...
>
> That would mean we wouldn't verify the md or FE builtins anymore
> and we would need to check for BUILT_IN_NORMAL in every caller (right now
> we do that only in some of them).
>
> Here is what I had in mind (untested so far):
>
> 2022-04-06  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/105150
> 	* gimple.cc (gimple_builtin_call_types_compatible_p): Use
> 	builtin_decl_explicit here...
> 	(gimple_call_builtin_p, gimple_call_combined_fn): ... rather than
> 	here.

Nice!  Thanks for doing this.

Richard

> --- gcc/gimple.cc.jj	2022-04-06 10:07:23.043064595 +0200
> +++ gcc/gimple.cc	2022-04-06 11:31:31.704255242 +0200
> @@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p (
>  {
>    gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
>  
> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> +    if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> +      fndecl = decl;
> +
>    tree ret = gimple_call_lhs (stmt);
>    if (ret
>        && !useless_type_conversion_p (TREE_TYPE (ret),
> @@ -2841,12 +2845,7 @@ 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)
> -    {
> -      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 gimple_builtin_call_types_compatible_p (stmt, fndecl);
>    return false;
>  }
>  
> @@ -2859,12 +2858,7 @@ 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)
> -    {
> -      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 gimple_builtin_call_types_compatible_p (stmt, fndecl);
>    return false;
>  }
>  
> @@ -2877,11 +2871,7 @@ 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))
> -    {
> -      if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> -	fndecl = decl;
> -      return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -    }
> +    return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>    return false;
>  }
>  
> @@ -2898,14 +2888,10 @@ 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))
> -	{
> -	  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));
> -	}
> +      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));
>      }
>    return CFN_LAST;
>  }
>
>
> 	Jakub