Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, Nov 9, 2015 at 5:25 PM, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>> This patch replaces the fndecl argument to builtin_vectorized_function
>> with a combined_fn and gets the vectoriser to call it for internal
>> functions too. The patch also moves vectorisation of machine-specific
>> built-ins to a new hook, builtin_md_vectorized_function.
>>
>> I've attached a -b version too since that's easier to read.
>
> @@ -42095,8 +42018,7 @@ ix86_builtin_vectorized_function (tree fndecl,
> tree type_out,
>
> /* Dispatch to a handler for a vectorization library. */
> if (ix86_veclib_handler)
> - return ix86_veclib_handler ((enum built_in_function) fn, type_out,
> - type_in);
> + return ix86_veclib_handler (combined_fn (fn), type_out, type_in);
>
> return NULL_TREE;
> }
>
> fn is already a combined_fn? Why does the builtin_vectorized_function
> not take one but an unsigned int?
Not everything that includes the target headers includes tree.h.
This is like builtin_conversion taking a tree_code as an unsigned int.
> @@ -42176,11 +42077,12 @@ ix86_veclibabi_svml (enum built_in_function
> fn, tree type_out, tree type_in)
> return NULL_TREE;
> }
>
> - bname = IDENTIFIER_POINTER (DECL_NAME (builtin_decl_implicit (fn)));
> + tree fndecl = mathfn_built_in (TREE_TYPE (type_in), fn);
> + bname = IDENTIFIER_POINTER (DECL_NAME (fndecl));
>
> - if (fn == BUILT_IN_LOGF)
> + if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_LOGF)
>
> with 'fn' now a combined_fn how is this going to work with IFNs?
By this point we already know that the function has one of the
supported modes. A previous patch extended matchfn_built_in
to handle combined_fns. E.g.
mathfn_built_in (float_type_node, IFN_SQRT)
returns BUILT_IN_SQRTF.
> +/* Implement TARGET_VECTORIZE_BUILTIN_MD_VECTORIZED_FUNCTION. */
> +
> +static tree
> +rs6000_builtin_md_vectorized_function (tree fndecl, tree type_out,
> + tree type_in)
> +{
>
> any reason you are using a fndecl for this hook instead of the function code?
It just seems more helpful to pass the fndecl when we have it.
It's cheap to go from the decl to the code but it's less cheap
to go the other way.
> @@ -1639,20 +1639,20 @@ vect_finish_stmt_generation (gimple *stmt,
> gimple *vec_stmt,
> tree
> vectorizable_function (gcall *call, tree vectype_out, tree vectype_in)
> {
> - tree fndecl = gimple_call_fndecl (call);
> -
> - /* We only handle functions that do not read or clobber memory -- i.e.
> - const or novops ones. */
> - if (!(gimple_call_flags (call) & (ECF_CONST | ECF_NOVOPS)))
> + /* We only handle functions that do not read or clobber memory. */
> + if (gimple_vuse (call))
> return NULL_TREE;
>
> - if (!fndecl
> - || TREE_CODE (fndecl) != FUNCTION_DECL
> - || !DECL_BUILT_IN (fndecl))
> - return NULL_TREE;
> + combined_fn fn = gimple_call_combined_fn (call);
> + if (fn != CFN_LAST)
> + return targetm.vectorize.builtin_vectorized_function
> + (fn, vectype_out, vectype_in);
>
> - return targetm.vectorize.builtin_vectorized_function (fndecl, vectype_out,
> - vectype_in);
> + if (gimple_call_builtin_p (call, BUILT_IN_MD))
> + return targetm.vectorize.builtin_md_vectorized_function
> + (gimple_call_fndecl (call), vectype_out, vectype_in);
> +
> + return NULL_TREE;
>
> Looking at this and the issues above wouldn't it be easier to simply
> pass the call stmt to the hook (which then can again handle
> both normal and target builtins)? And it has context available
> (actual arguments and number of arguments for IFN calls).
I'd rather not do that, since it means we have to construct a gcall *
in cases where we're not asking about a straight-forward vectorisation
of a preexisting scalar statement.
The number of arguments is an inherent property of the function,
it doesn't require access to a particular call. The hook tells
us what vector types we're using (and by extension what types
the scalar op would have).
Thanks,
Richard
