Re: [PATCH 3/3] vect: inbranch SIMD clones

[Jakub Jelinek <jakub@redhat.com>]

On Wed, Nov 30, 2022 at 03:17:30PM +0000, Andrew Stubbs wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-16.c
> @@ -0,0 +1,89 @@
> +/* { dg-require-effective-target vect_simd_clones } */
> +/* { dg-additional-options "-fopenmp-simd -fdump-tree-optimized" } */
> +/* { dg-additional-options "-mavx" { target avx_runtime } } */
...
> +/* Ensure the the in-branch simd clones are used on targets that support
> +   them.  These counts include all call and definitions.  */
> +
> +/* { dg-skip-if "" { x86_64-*-* } { "-flto" } { "" } } */

Maybe better add -ffat-lto-objects to dg-additional-options and drop
the dg-skip-if (if it works with that, for all similar tests)?

> @@ -1063,7 +1064,8 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
>     A statement is if-convertible if:
>     - it is an if-convertible GIMPLE_ASSIGN,
>     - it is a GIMPLE_LABEL or a GIMPLE_COND,
> -   - it is builtins call.  */
> +   - it is builtins call.

s/call\./call,/ above

> +   - it is a call to a function with a SIMD clone.  */
>  
>  static bool
>  if_convertible_stmt_p (gimple *stmt, vec<data_reference_p> refs)
> @@ -1083,13 +1085,23 @@ if_convertible_stmt_p (gimple *stmt, vec<data_reference_p> refs)
>  	tree fndecl = gimple_call_fndecl (stmt);
>  	if (fndecl)
>  	  {
> +	    /* We can vectorize some builtins and functions with SIMD
> +	       "inbranch" clones.  */
>  	    int flags = gimple_call_flags (stmt);
> +	    struct cgraph_node *node = cgraph_node::get (fndecl);
>  	    if ((flags & ECF_CONST)
>  		&& !(flags & ECF_LOOPING_CONST_OR_PURE)
> -		/* We can only vectorize some builtins at the moment,
> -		   so restrict if-conversion to those.  */
>  		&& fndecl_built_in_p (fndecl))
>  	      return true;
> +	    else if (node && node->simd_clones != NULL)

I don't see much value in the "else " above, the if branch returns
if condition is true, so just
	    if (node && node->simd_clones != NULL)
would do it.

> +	      /* Ensure that at least one clone can be "inbranch".  */
> +	      for (struct cgraph_node *n = node->simd_clones; n != NULL;
> +		   n = n->simdclone->next_clone)
> +		if (n->simdclone->inbranch)
> +		  {
> +		    need_to_predicate = true;
> +		    return true;
> +		  }
>  	  }
>  	return false;
>        }
> @@ -2603,6 +2615,29 @@ predicate_statements (loop_p loop)
>  	      gimple_assign_set_rhs1 (stmt, ifc_temp_var (type, rhs, &gsi));
>  	      update_stmt (stmt);
>  	    }
> +
> +	  /* Convert functions that have a SIMD clone to IFN_MASK_CALL.  This
> +	     will cause the vectorizer to match the "in branch" clone variants,
> +	     and serves to build the mask vector in a natural way.  */
> +	  gcall *call = dyn_cast <gcall *> (gsi_stmt (gsi));
> +	  if (call && !gimple_call_internal_p (call))
> +	    {
> +	      tree orig_fn = gimple_call_fn (call);
> +	      int orig_nargs = gimple_call_num_args (call);
> +	      auto_vec<tree> args;
> +	      args.safe_push (orig_fn);
> +	      for (int i=0; i < orig_nargs; i++)

Formatting - int i = 0;

> +		args.safe_push (gimple_call_arg (call, i));
> +	      args.safe_push (cond);
> +
> +	      /* Replace the call with a IFN_MASK_CALL that has the extra
> +		 condition parameter. */
> +	      gcall *new_call = gimple_build_call_internal_vec (IFN_MASK_CALL,
> +								args);
> +	      gimple_call_set_lhs (new_call, gimple_call_lhs (call));
> +	      gsi_replace (&gsi, new_call, true);
> +	    }
> +
>  	  lhs = gimple_get_lhs (gsi_stmt (gsi));
>  	  if (lhs && TREE_CODE (lhs) == SSA_NAME)
>  	    ssa_names.add (lhs);
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -3987,6 +3987,7 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info,
>    size_t i, nargs;
>    tree lhs, rtype, ratype;
>    vec<constructor_elt, va_gc> *ret_ctor_elts = NULL;
> +  int arg_offset = 0;
>  
>    /* Is STMT a vectorizable call?   */
>    gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt);
> @@ -3994,6 +3995,16 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info,
>      return false;
>  
>    fndecl = gimple_call_fndecl (stmt);
> +  if (fndecl == NULL_TREE
> +      && gimple_call_internal_p (stmt)
> +      && gimple_call_internal_fn (stmt) == IFN_MASK_CALL)

Replace the above 2 lines with
      && gimple_call_internal_p (stmt, IFN_MASK_CALL))
?

> --- a/gcc/tree-vect-loop.cc                                                                                                                                                           
> +++ b/gcc/tree-vect-loop.cc                                                                                                                                                           
> @@ -2121,6 +2121,15 @@ vect_get_datarefs_in_loop (loop_p loop, basic_block *bbs,                                                                                                      
>             if (is_gimple_call (stmt) && loop->safelen)                                                                                                                               
>               {                                                                                                                                                                       
>                 tree fndecl = gimple_call_fndecl (stmt), op;                                                                                                                          
> +               if (fndecl == NULL_TREE                                                                                                                                               
> +                   && gimple_call_internal_p (stmt)                                                                                                                                  
> +                   && gimple_call_internal_fn (stmt) == IFN_MASK_CALL)                                                                                                               

Similarly.

Otherwise LGTM.

	Jakub