Re: [PATCH 05/18] rs6000: Support for vectorizing built-in functions

[Segher Boessenkool <segher@kernel.crashing.org>]

On Wed, Sep 01, 2021 at 11:13:41AM -0500, Bill Schmidt wrote:
> This patch just duplicates a couple of functions and adjusts them to use the
> new builtin names.  There's no logical change otherwise.

> +/* Returns a function decl for a vectorized version of the builtin function
> +   with builtin function code FN and the result vector type TYPE, or NULL_TREE
> +   if it is not available.  */
> +
> +static tree
> +rs6000_new_builtin_vectorized_function (unsigned int fn, tree type_out,
> +					tree type_in)
> +{
> +  machine_mode in_mode, out_mode;
> +  int in_n, out_n;
> +
> +  if (TARGET_DEBUG_BUILTIN)
> +    fprintf (stderr, "rs6000_new_builtin_vectorized_function (%s, %s, %s)\n",
> +	     combined_fn_name (combined_fn (fn)),
> +	     GET_MODE_NAME (TYPE_MODE (type_out)),
> +	     GET_MODE_NAME (TYPE_MODE (type_in)));
> +
> +  if (TREE_CODE (type_out) != VECTOR_TYPE
> +      || TREE_CODE (type_in) != VECTOR_TYPE)
> +    return NULL_TREE;

This is not described in the function comment.  Should it?  Should this
be here at all, should it be an assert instead?

It also should say it implements the
TARGET_VECTORIZE_BUILTIN_VECTORIZED_FUNCTION macro?

> +static tree
> +rs6000_new_builtin_md_vectorized_function (tree fndecl, tree type_out,
> +					   tree type_in)
> +{
> +  machine_mode in_mode, out_mode;
> +  int in_n, out_n;
> +
> +  if (TARGET_DEBUG_BUILTIN)
> +    fprintf (stderr,
> +	     "rs6000_new_builtin_md_vectorized_function (%s, %s, %s)\n",
> +	     IDENTIFIER_POINTER (DECL_NAME (fndecl)),
> +	     GET_MODE_NAME (TYPE_MODE (type_out)),
> +	     GET_MODE_NAME (TYPE_MODE (type_in)));
> +
> +  if (TREE_CODE (type_out) != VECTOR_TYPE
> +      || TREE_CODE (type_in) != VECTOR_TYPE)
> +    return NULL_TREE;

Here it definitely should be an assert, the documentation of this hook
says so.

Other than that this is fine of course (or not worse than what there
already was, anyway ;-) ).  So put this on the big "one day we will
clean this up" list?

Okay for trunk.  Thanks!


Segher