On Wed, 6 Apr 2022, Jakub Jelinek wrote:
> Hi!
>
> And here is the follow-up patch that does the argument checking
> on GENERIC. It ensures TYPE_MAIN_VARIANT == TYPE_MAIN_VARIANT
> compatibility on the arguments, except for pointer arguments like
> FILE *, const struct tm * etc. that would never match that way
> and similarly to the gimple one also char/short promotions.
> Tested that it works correctly with fprintf.
>
> Not sure if we shouldn't allow any useless_type_conversion_p
> differences for all pointers though, say if somebody uses
> a K&R declared builtin and passes in char msg[] = "...";
> then it will be char * instead of const char *.
>
> Bootstrapped/regtested on {powerpc64le,x86_64,i686}-linux, ok for trunk?
>
> 2022-04-06 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/105150
> * tree.cc (tree_builtin_call_types_compatible_p): New function.
> (get_call_combined_fn): Use it.
>
> * gcc.dg/pr105150.c: New test.
>
> --- gcc/tree.cc.jj 2022-03-26 08:11:28.447530077 +0100
> +++ gcc/tree.cc 2022-04-05 17:09:46.892114064 +0200
> @@ -8406,6 +8406,61 @@ get_callee_fndecl (const_tree call)
> return NULL_TREE;
> }
>
> +/* Return true when STMTs arguments and return value match those of FNDECL,
> + a decl of a builtin function. */
> +
> +static bool
> +tree_builtin_call_types_compatible_p (const_tree call, tree fndecl)
> +{
> + gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
> +
> + if (TYPE_MAIN_VARIANT (TREE_TYPE (call))
> + != TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (fndecl))))
> + return false;
> +
> + tree targs = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
> + unsigned nargs = call_expr_nargs (call);
> + for (unsigned i = 0; i < nargs; ++i, targs = TREE_CHAIN (targs))
> + {
> + /* Variadic args follow. */
> + if (!targs)
> + return true;
> + tree arg = CALL_EXPR_ARG (call, i);
> + tree type = TREE_VALUE (targs);
> + if (TYPE_MAIN_VARIANT (type) != TYPE_MAIN_VARIANT (TREE_TYPE (arg)))
> + {
> + /* If argument in the builtin fndecl is the artificial pointer
> + for FILE * etc., allow any pointer type arg for it. */
> + if (POINTER_TYPE_P (type)
> + && POINTER_TYPE_P (TREE_TYPE (arg))
> + && useless_type_conversion_p (type, TREE_TYPE (arg)))
> + {
> + unsigned int j;
> + for (j = 0; j < ARRAY_SIZE (builtin_structptr_types); ++j)
> + if (type == builtin_structptr_types[j].node
> + && (builtin_structptr_types[j].node
> + != builtin_structptr_types[j].base))
> + break;
> + if (j < ARRAY_SIZE (builtin_structptr_types))
> + continue;
> + }
> + /* char/short integral arguments are promoted to int
> + by several frontends if targetm.calls.promote_prototypes
> + is true. Allow such promotion too. */
> + if (INTEGRAL_TYPE_P (type)
> + && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node)
> + && targetm.calls.promote_prototypes (TREE_TYPE (fndecl))
> + && useless_type_conversion_p (integer_type_node,
> + TREE_TYPE (arg)))
On trees we'd use tree_[sign_]nop_conversion () instead of
useless_type_conversion_p, I think it's OK to allow all such
pointer conversions. In the end this probably means being
more forgiving than TYPE_MAIN_VARIANT equivalence throughout, that
would also make the code more similar to
gimple_builtin_call_types_compatible_p besides
s/useless_type_conversion_p/tree_sign_nop_conversion/
What do you think? If we don't go for strict TYPE_MAIN_VARIANT
compatibility we should apply lax rules for all, the return
type and all argument types. It's not that the GENERIC we end
up building from GIMPLE in various places adheres to the strict
GENERIC type compatibility rules ...
Richard.
> + continue;
> + return false;
> + }
> + }
> + if (targs && !VOID_TYPE_P (TREE_VALUE (targs)))
> + return false;
> + return true;
> +}
> +
> /* If CALL_EXPR CALL calls a normal built-in function or an internal
> function,
> return the associated function code, otherwise return CFN_LAST. */
>
> @@ -8420,7 +8475,13 @@ get_call_combined_fn (const_tree call)
>
> tree fndecl = get_callee_fndecl (call);
> if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
> - return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> + {
> + tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
> + if (!decl)
> + decl = fndecl;
> + if (tree_builtin_call_types_compatible_p (call, decl))
> + return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> + }
>
> return CFN_LAST;
> }
> --- gcc/testsuite/gcc.dg/pr105150.c.jj 2022-04-05 16:56:22.894319239
> +0200
> +++ gcc/testsuite/gcc.dg/pr105150.c 2022-04-05 16:56:16.646406314 +0200
> @@ -0,0 +1,8 @@
> +/* PR tree-optimization/105150 */
> +/* { dg-options "-w -Ofast" } */
> +
> +#define A(name) __typeof (__builtin_##name (0)) name (); \
> + float name##1 () { return !name (1); } \
> + double name##2 () { return name (1.0L); }
> +#define B(name) A(name) A(name##l)
> +B (sqrt)
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
