[1/3] Refactor to allow internal_fn's

Hi,

I'm posting the patches separately now with ChangeLogs.

I made the suggested changes and tried to simplify the code a bit 
further. Where internal to tree-vect-stmts I changed most functions to 
use code_helper to avoid having to check at places we didn't need to. I 
was trying to simplify things further by also modifying 
supportable_half_widening_operation and supportable_convert_operation 
but the result of that was that I ended up moving the code to cast to 
tree code inside them rather than at the call site and it didn't look 
simpler, so I left those. Though if we did make those changes we'd no 
longer need to keep around the tc1 variable in 
vectorizable_conversion... Let me know what you think.

gcc/ChangeLog:

2023-04-28  Andre Vieira  <andre.simoesdiasvieira@arm.com>
             Joel Hutton  <joel.hutton@arm.com>

         * tree-vect-patterns.cc (vect_gimple_build): New Function.
         (vect_recog_widen_op_pattern): Refactor to use code_helper.
         * tree-vect-stmts.cc (vect_gen_widened_results_half): Likewise.
         (vect_create_vectorized_demotion_stmts): Likewise.
         (vect_create_vectorized_promotion_stmts): Likewise.
         (vect_create_half_widening_stmts): Likewise.
         (vectorizable_conversion): Likewise.
         (vectorizable_call): Likewise.
         (supportable_widening_operation): Likewise.
         (supportable_narrowing_operation): Likewise.
         (simple_integer_narrowing): Likewise.
         * tree-vectorizer.h (supportable_widening_operation): Likewise.
         (supportable_narrowing_operation): Likewise.
         (vect_gimple_build): New function prototype.
         * tree.h (code_helper::safe_as_tree_code): New function.
         (code_helper::safe_as_fn_code): New function.

On Fri, 28 Apr 2023, Andre Vieira (lists) wrote:

> Hi,
> 
> I'm posting the patches separately now with ChangeLogs.
> 
> I made the suggested changes and tried to simplify the code a bit further.
> Where internal to tree-vect-stmts I changed most functions to use code_helper
> to avoid having to check at places we didn't need to. I was trying to simplify
> things further by also modifying supportable_half_widening_operation and
> supportable_convert_operation but the result of that was that I ended up
> moving the code to cast to tree code inside them rather than at the call site
> and it didn't look simpler, so I left those. Though if we did make those
> changes we'd no longer need to keep around the tc1 variable in
> vectorizable_conversion... Let me know what you think.

I see that

-  else if (CONVERT_EXPR_CODE_P (code)
+  else if (CONVERT_EXPR_CODE_P (code.safe_as_tree_code ())

is convenient (as much as I dislike safe_as_tree_code).  Isn't
the following

-  if (!CONVERT_EXPR_CODE_P (code))
+  if (!CONVERT_EXPR_CODE_P ((tree_code) code))
     return false;

then wrong?  In other places you added an assert - I assume
that we might want to checking assert in the cast operators?
(those were added mainly for convenience, maybe we want
as_a <>, etc. here - not sure if those will play well with
enums though).  Just suggestions for eventual followups in
this area.

+inline enum tree_code
+code_helper::safe_as_tree_code () const
+{
+  return is_tree_code () ? (tree_code) *this : MAX_TREE_CODES;
+}
+
+inline combined_fn
+code_helper::safe_as_fn_code () const {
+  return is_fn_code () ? (combined_fn) *this : CFN_LAST;
+}
+

newline after the last 'const'.  Can you place a comment before
these to explain their intended use?  Aka give the case the
code isn't the desired kind a safe value?

The patch is OK with just the last bit fixed.

Thanks,
Richard.

> gcc/ChangeLog:
> 
> 2023-04-28  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>             Joel Hutton  <joel.hutton@arm.com>
> 
>         * tree-vect-patterns.cc (vect_gimple_build): New Function.
>         (vect_recog_widen_op_pattern): Refactor to use code_helper.
>         * tree-vect-stmts.cc (vect_gen_widened_results_half): Likewise.
>         (vect_create_vectorized_demotion_stmts): Likewise.
>         (vect_create_vectorized_promotion_stmts): Likewise.
>         (vect_create_half_widening_stmts): Likewise.
>         (vectorizable_conversion): Likewise.
>         (vectorizable_call): Likewise.
>         (supportable_widening_operation): Likewise.
>         (supportable_narrowing_operation): Likewise.
>         (simple_integer_narrowing): Likewise.
>         * tree-vectorizer.h (supportable_widening_operation): Likewise.
>         (supportable_narrowing_operation): Likewise.
>         (vect_gimple_build): New function prototype.
>         * tree.h (code_helper::safe_as_tree_code): New function.
>         (code_helper::safe_as_fn_code): New function.
>

On 03/05/2023 12:55, Richard Biener wrote:
> On Fri, 28 Apr 2023, Andre Vieira (lists) wrote:
> 
>> Hi,
>>
>> I'm posting the patches separately now with ChangeLogs.
>>
>> I made the suggested changes and tried to simplify the code a bit further.
>> Where internal to tree-vect-stmts I changed most functions to use code_helper
>> to avoid having to check at places we didn't need to. I was trying to simplify
>> things further by also modifying supportable_half_widening_operation and
>> supportable_convert_operation but the result of that was that I ended up
>> moving the code to cast to tree code inside them rather than at the call site
>> and it didn't look simpler, so I left those. Though if we did make those
>> changes we'd no longer need to keep around the tc1 variable in
>> vectorizable_conversion... Let me know what you think.
> 
> I see that
> 
> -  else if (CONVERT_EXPR_CODE_P (code)
> +  else if (CONVERT_EXPR_CODE_P (code.safe_as_tree_code ())
> 
> is convenient (as much as I dislike safe_as_tree_code).  Isn't
> the following
> 
> -  if (!CONVERT_EXPR_CODE_P (code))
> +  if (!CONVERT_EXPR_CODE_P ((tree_code) code))
>       return false;
For some reason I thought the code could only reach here if code was a 
tree code, but I guess if we have an ifn and the modes aren't the same 
as the wide_vectype it would fall to this, which for an ifn this would 
fail. I am wondering whether it needs to though, the multi-step widening 
should also work for ifn's no? We'd need to adapt it, to not use c1, c2 
but hi, lo in case of ifn I guess.. and then use a different optab look 
up too?

Though I'm thinking, maybe this should be a follow-up and just not have 
that 'feature' for now. The feature being, supporting multi-step 
conversion for new widening IFN's.

On Thu, 4 May 2023, Andre Vieira (lists) wrote:

> 
> 
> On 03/05/2023 12:55, Richard Biener wrote:
> > On Fri, 28 Apr 2023, Andre Vieira (lists) wrote:
> > 
> >> Hi,
> >>
> >> I'm posting the patches separately now with ChangeLogs.
> >>
> >> I made the suggested changes and tried to simplify the code a bit further.
> >> Where internal to tree-vect-stmts I changed most functions to use
> >> code_helper
> >> to avoid having to check at places we didn't need to. I was trying to
> >> simplify
> >> things further by also modifying supportable_half_widening_operation and
> >> supportable_convert_operation but the result of that was that I ended up
> >> moving the code to cast to tree code inside them rather than at the call
> >> site
> >> and it didn't look simpler, so I left those. Though if we did make those
> >> changes we'd no longer need to keep around the tc1 variable in
> >> vectorizable_conversion... Let me know what you think.
> > 
> > I see that
> > 
> > -  else if (CONVERT_EXPR_CODE_P (code)
> > +  else if (CONVERT_EXPR_CODE_P (code.safe_as_tree_code ())
> > 
> > is convenient (as much as I dislike safe_as_tree_code).  Isn't
> > the following
> > 
> > -  if (!CONVERT_EXPR_CODE_P (code))
> > +  if (!CONVERT_EXPR_CODE_P ((tree_code) code))
> >       return false;
> For some reason I thought the code could only reach here if code was a tree
> code, but I guess if we have an ifn and the modes aren't the same as the
> wide_vectype it would fall to this, which for an ifn this would fail. I am
> wondering whether it needs to though, the multi-step widening should also work
> for ifn's no? We'd need to adapt it, to not use c1, c2 but hi, lo in case of
> ifn I guess.. and then use a different optab look up too?
> 
> Though I'm thinking, maybe this should be a follow-up and just not have that
> 'feature' for now. The feature being, supporting multi-step conversion for new
> widening IFN's.

Yes, I think we should address this in a followup if needed.

Richard.

Hi,

I think I tackled all of your comments, let me know if I missed something.


gcc/ChangeLog:

2023-05-12  Andre Vieira  <andre.simoesdiasvieira@arm.com>
             Joel Hutton  <joel.hutton@arm.com>

         * tree-vect-patterns.cc (vect_gimple_build): New Function.
         (vect_recog_widen_op_pattern): Refactor to use code_helper.
         * tree-vect-stmts.cc (vect_gen_widened_results_half): Likewise.
         (vect_create_vectorized_demotion_stmts): Likewise.
         (vect_create_vectorized_promotion_stmts): Likewise.
         (vect_create_half_widening_stmts): Likewise.
         (vectorizable_conversion): Likewise.
         (vectorizable_call): Likewise.
         (supportable_widening_operation): Likewise.
         (supportable_narrowing_operation): Likewise.
         (simple_integer_narrowing): Likewise.
         * tree-vectorizer.h (supportable_widening_operation): Likewise.
         (supportable_narrowing_operation): Likewise.
         (vect_gimple_build): New function prototype.
         * tree.h (code_helper::safe_as_tree_code): New function.
         (code_helper::safe_as_fn_code): New function.
diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
index 33a8b2bb60601dc1a67de62a56bbf3c355e12dbd..1778af0242898e3dc73d94d22a5b8505628a53b5 100644
--- a/gcc/tree-vect-patterns.cc
+++ b/gcc/tree-vect-patterns.cc
@@ -25,6 +25,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl.h"
 #include "tree.h"
 #include "gimple.h"
+#include "gimple-iterator.h"
+#include "gimple-fold.h"
 #include "ssa.h"
 #include "expmed.h"
 #include "optabs-tree.h"
@@ -1392,7 +1394,7 @@ vect_recog_sad_pattern (vec_info *vinfo,
 static gimple *
 vect_recog_widen_op_pattern (vec_info *vinfo,
 			     stmt_vec_info last_stmt_info, tree *type_out,
-			     tree_code orig_code, tree_code wide_code,
+			     tree_code orig_code, code_helper wide_code,
 			     bool shift_p, const char *name)
 {
   gimple *last_stmt = last_stmt_info->stmt;
@@ -1435,7 +1437,7 @@ vect_recog_widen_op_pattern (vec_info *vinfo,
       vecctype = get_vectype_for_scalar_type (vinfo, ctype);
     }
 
-  enum tree_code dummy_code;
+  code_helper dummy_code;
   int dummy_int;
   auto_vec<tree> dummy_vec;
   if (!vectype
@@ -1456,8 +1458,7 @@ vect_recog_widen_op_pattern (vec_info *vinfo,
 		       2, oprnd, half_type, unprom, vectype);
 
   tree var = vect_recog_temp_ssa_var (itype, NULL);
-  gimple *pattern_stmt = gimple_build_assign (var, wide_code,
-					      oprnd[0], oprnd[1]);
+  gimple *pattern_stmt = vect_gimple_build (var, wide_code, oprnd[0], oprnd[1]);
 
   if (vecctype != vecitype)
     pattern_stmt = vect_convert_output (vinfo, last_stmt_info, ctype,
@@ -6808,3 +6809,20 @@ vect_pattern_recog (vec_info *vinfo)
   /* After this no more add_stmt calls are allowed.  */
   vinfo->stmt_vec_info_ro = true;
 }
+
+/* Build a GIMPLE_ASSIGN or GIMPLE_CALL with the tree_code,
+   or internal_fn contained in ch, respectively.  */
+gimple *
+vect_gimple_build (tree lhs, code_helper ch, tree op0, tree op1)
+{
+  gcc_assert (op0 != NULL_TREE);
+  if (ch.is_tree_code ())
+    return gimple_build_assign (lhs, (tree_code) ch, op0, op1);
+
+  gcc_assert (ch.is_internal_fn ());
+  gimple* stmt = gimple_build_call_internal (as_internal_fn ((combined_fn) ch),
+					     op1 == NULL_TREE ? 1 : 2,
+					     op0, op1);
+  gimple_call_set_lhs (stmt, lhs);
+  return stmt;
+}
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 61a2da4ecee9c449c1469cab3c4cfa1a782471d5..d152ae9ab10b361b88c0f839d6951c43b954750a 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -3261,13 +3261,13 @@ vectorizable_bswap (vec_info *vinfo,
 
 static bool
 simple_integer_narrowing (tree vectype_out, tree vectype_in,
-			  tree_code *convert_code)
+			  code_helper *convert_code)
 {
   if (!INTEGRAL_TYPE_P (TREE_TYPE (vectype_out))
       || !INTEGRAL_TYPE_P (TREE_TYPE (vectype_in)))
     return false;
 
-  tree_code code;
+  code_helper code;
   int multi_step_cvt = 0;
   auto_vec <tree, 8> interm_types;
   if (!supportable_narrowing_operation (NOP_EXPR, vectype_out, vectype_in,
@@ -3481,7 +3481,7 @@ vectorizable_call (vec_info *vinfo,
   tree callee = gimple_call_fndecl (stmt);
 
   /* First try using an internal function.  */
-  tree_code convert_code = ERROR_MARK;
+  code_helper convert_code = MAX_TREE_CODES;
   if (cfn != CFN_LAST
       && (modifier == NONE
 	  || (modifier == NARROW
@@ -3667,8 +3667,8 @@ vectorizable_call (vec_info *vinfo,
 			  continue;
 			}
 		      new_temp = make_ssa_name (vec_dest);
-		      new_stmt = gimple_build_assign (new_temp, convert_code,
-						      prev_res, half_res);
+		      new_stmt = vect_gimple_build (new_temp, convert_code,
+						    prev_res, half_res);
 		      vect_finish_stmt_generation (vinfo, stmt_info,
 						   new_stmt, gsi);
 		    }
@@ -3758,8 +3758,8 @@ vectorizable_call (vec_info *vinfo,
 		  continue;
 		}
 	      new_temp = make_ssa_name (vec_dest);
-	      new_stmt = gimple_build_assign (new_temp, convert_code,
-					      prev_res, half_res);
+	      new_stmt = vect_gimple_build (new_temp, convert_code, prev_res,
+					    half_res);
 	      vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
 	    }
 	  else
@@ -4771,7 +4771,7 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info,
    STMT_INFO is the original scalar stmt that we are vectorizing.  */
 
 static gimple *
-vect_gen_widened_results_half (vec_info *vinfo, enum tree_code code,
+vect_gen_widened_results_half (vec_info *vinfo, code_helper ch,
                                tree vec_oprnd0, tree vec_oprnd1, int op_type,
 			       tree vec_dest, gimple_stmt_iterator *gsi,
 			       stmt_vec_info stmt_info)
@@ -4780,12 +4780,11 @@ vect_gen_widened_results_half (vec_info *vinfo, enum tree_code code,
   tree new_temp;
 
   /* Generate half of the widened result:  */
-  gcc_assert (op_type == TREE_CODE_LENGTH (code));
   if (op_type != binary_op)
     vec_oprnd1 = NULL;
-  new_stmt = gimple_build_assign (vec_dest, code, vec_oprnd0, vec_oprnd1);
+  new_stmt = vect_gimple_build (vec_dest, ch, vec_oprnd0, vec_oprnd1);
   new_temp = make_ssa_name (vec_dest, new_stmt);
-  gimple_assign_set_lhs (new_stmt, new_temp);
+  gimple_set_lhs (new_stmt, new_temp);
   vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
 
   return new_stmt;
@@ -4802,7 +4801,7 @@ vect_create_vectorized_demotion_stmts (vec_info *vinfo, vec<tree> *vec_oprnds,
 				       stmt_vec_info stmt_info,
 				       vec<tree> &vec_dsts,
 				       gimple_stmt_iterator *gsi,
-				       slp_tree slp_node, enum tree_code code)
+				       slp_tree slp_node, code_helper code)
 {
   unsigned int i;
   tree vop0, vop1, new_tmp, vec_dest;
@@ -4814,9 +4813,9 @@ vect_create_vectorized_demotion_stmts (vec_info *vinfo, vec<tree> *vec_oprnds,
       /* Create demotion operation.  */
       vop0 = (*vec_oprnds)[i];
       vop1 = (*vec_oprnds)[i + 1];
-      gassign *new_stmt = gimple_build_assign (vec_dest, code, vop0, vop1);
+      gimple *new_stmt = vect_gimple_build (vec_dest, code, vop0, vop1);
       new_tmp = make_ssa_name (vec_dest, new_stmt);
-      gimple_assign_set_lhs (new_stmt, new_tmp);
+      gimple_set_lhs (new_stmt, new_tmp);
       vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
 
       if (multi_step_cvt)
@@ -4864,8 +4863,8 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo,
 					vec<tree> *vec_oprnds1,
 					stmt_vec_info stmt_info, tree vec_dest,
 					gimple_stmt_iterator *gsi,
-					enum tree_code code1,
-					enum tree_code code2, int op_type)
+					code_helper ch1,
+					code_helper ch2, int op_type)
 {
   int i;
   tree vop0, vop1, new_tmp1, new_tmp2;
@@ -4881,10 +4880,10 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo,
 	vop1 = NULL_TREE;
 
       /* Generate the two halves of promotion operation.  */
-      new_stmt1 = vect_gen_widened_results_half (vinfo, code1, vop0, vop1,
+      new_stmt1 = vect_gen_widened_results_half (vinfo, ch1, vop0, vop1,
 						 op_type, vec_dest, gsi,
 						 stmt_info);
-      new_stmt2 = vect_gen_widened_results_half (vinfo, code2, vop0, vop1,
+      new_stmt2 = vect_gen_widened_results_half (vinfo, ch2, vop0, vop1,
 						 op_type, vec_dest, gsi,
 						 stmt_info);
       if (is_gimple_call (new_stmt1))
@@ -4915,7 +4914,7 @@ vect_create_half_widening_stmts (vec_info *vinfo,
 					vec<tree> *vec_oprnds1,
 					stmt_vec_info stmt_info, tree vec_dest,
 					gimple_stmt_iterator *gsi,
-					enum tree_code code1,
+					code_helper code1,
 					int op_type)
 {
   int i;
@@ -4945,13 +4944,13 @@ vect_create_half_widening_stmts (vec_info *vinfo,
 	  new_stmt2 = gimple_build_assign (new_tmp2, NOP_EXPR, vop1);
 	  vect_finish_stmt_generation (vinfo, stmt_info, new_stmt2, gsi);
 	  /* Perform the operation.  With both vector inputs widened.  */
-	  new_stmt3 = gimple_build_assign (vec_dest, code1, new_tmp1, new_tmp2);
+	  new_stmt3 = vect_gimple_build (vec_dest, code1, new_tmp1, new_tmp2);
 	}
       else
 	{
 	  /* Perform the operation.  With the single vector input widened.  */
-	  new_stmt3 = gimple_build_assign (vec_dest, code1, new_tmp1, vop1);
-      }
+	  new_stmt3 = vect_gimple_build (vec_dest, code1, new_tmp1, vop1);
+	}
 
       new_tmp3 = make_ssa_name (vec_dest, new_stmt3);
       gimple_assign_set_lhs (new_stmt3, new_tmp3);
@@ -4981,8 +4980,9 @@ vectorizable_conversion (vec_info *vinfo,
   tree scalar_dest;
   tree op0, op1 = NULL_TREE;
   loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo);
-  enum tree_code code, code1 = ERROR_MARK, code2 = ERROR_MARK;
-  enum tree_code codecvt1 = ERROR_MARK, codecvt2 = ERROR_MARK;
+  tree_code tc1;
+  code_helper code, code1, code2;
+  code_helper codecvt1 = ERROR_MARK, codecvt2 = ERROR_MARK;
   tree new_temp;
   enum vect_def_type dt[2] = {vect_unknown_def_type, vect_unknown_def_type};
   int ndts = 2;
@@ -5011,31 +5011,43 @@ vectorizable_conversion (vec_info *vinfo,
       && ! vec_stmt)
     return false;
 
-  gassign *stmt = dyn_cast <gassign *> (stmt_info->stmt);
-  if (!stmt)
+  gimple* stmt = stmt_info->stmt;
+  if (!(is_gimple_assign (stmt) || is_gimple_call (stmt)))
     return false;
 
-  if (TREE_CODE (gimple_assign_lhs (stmt)) != SSA_NAME)
+  if (gimple_get_lhs (stmt) == NULL_TREE
+      || TREE_CODE (gimple_get_lhs (stmt)) != SSA_NAME)
     return false;
 
-  code = gimple_assign_rhs_code (stmt);
-  if (!CONVERT_EXPR_CODE_P (code)
-      && code != FIX_TRUNC_EXPR
-      && code != FLOAT_EXPR
-      && code != WIDEN_PLUS_EXPR
-      && code != WIDEN_MINUS_EXPR
-      && code != WIDEN_MULT_EXPR
-      && code != WIDEN_LSHIFT_EXPR)
+  if (TREE_CODE (gimple_get_lhs (stmt)) != SSA_NAME)
+    return false;
+
+  if (is_gimple_assign (stmt))
+    {
+      code = gimple_assign_rhs_code (stmt);
+      op_type = TREE_CODE_LENGTH ((tree_code) code);
+    }
+  else if (gimple_call_internal_p (stmt))
+    {
+      code = gimple_call_internal_fn (stmt);
+      op_type = gimple_call_num_args (stmt);
+    }
+  else
     return false;
 
   bool widen_arith = (code == WIDEN_PLUS_EXPR
-		      || code == WIDEN_MINUS_EXPR
-		      || code == WIDEN_MULT_EXPR
-		      || code == WIDEN_LSHIFT_EXPR);
-  op_type = TREE_CODE_LENGTH (code);
+		 || code == WIDEN_MINUS_EXPR
+		 || code == WIDEN_MULT_EXPR
+		 || code == WIDEN_LSHIFT_EXPR);
+
+  if (!widen_arith
+      && !CONVERT_EXPR_CODE_P (code)
+      && code != FIX_TRUNC_EXPR
+      && code != FLOAT_EXPR)
+    return false;
 
   /* Check types of lhs and rhs.  */
-  scalar_dest = gimple_assign_lhs (stmt);
+  scalar_dest = gimple_get_lhs (stmt);
   lhs_type = TREE_TYPE (scalar_dest);
   vectype_out = STMT_VINFO_VECTYPE (stmt_info);
 
@@ -5073,10 +5085,14 @@ vectorizable_conversion (vec_info *vinfo,
 
   if (op_type == binary_op)
     {
-      gcc_assert (code == WIDEN_MULT_EXPR || code == WIDEN_LSHIFT_EXPR
-		  || code == WIDEN_PLUS_EXPR || code == WIDEN_MINUS_EXPR);
+      gcc_assert (code == WIDEN_MULT_EXPR
+		  || code == WIDEN_LSHIFT_EXPR
+		  || code == WIDEN_PLUS_EXPR
+		  || code == WIDEN_MINUS_EXPR);
+
 
-      op1 = gimple_assign_rhs2 (stmt);
+      op1 = is_gimple_assign (stmt) ? gimple_assign_rhs2 (stmt) :
+				     gimple_call_arg (stmt, 0);
       tree vectype1_in;
       if (!vect_is_simple_use (vinfo, stmt_info, slp_node, 1,
 			       &op1, &slp_op1, &dt[1], &vectype1_in))
@@ -5160,8 +5176,13 @@ vectorizable_conversion (vec_info *vinfo,
 	  && code != FLOAT_EXPR
 	  && !CONVERT_EXPR_CODE_P (code))
 	return false;
-      if (supportable_convert_operation (code, vectype_out, vectype_in, &code1))
+      gcc_assert (code.is_tree_code ());
+      if (supportable_convert_operation ((tree_code) code, vectype_out,
+					 vectype_in, &tc1))
+      {
+	code1 = tc1;
 	break;
+      }
       /* FALLTHRU */
     unsupported:
       if (dump_enabled_p ())
@@ -5172,9 +5193,12 @@ vectorizable_conversion (vec_info *vinfo,
     case WIDEN:
       if (known_eq (nunits_in, nunits_out))
 	{
-	  if (!supportable_half_widening_operation (code, vectype_out,
-						   vectype_in, &code1))
+	  if (!(code.is_tree_code ()
+		&& supportable_half_widening_operation ((tree_code) code,
+							vectype_out, vectype_in,
+							&tc1)))
 	    goto unsupported;
+	  code1 = tc1;
 	  gcc_assert (!(multi_step_cvt && op_type == binary_op));
 	  break;
 	}
@@ -5208,14 +5232,17 @@ vectorizable_conversion (vec_info *vinfo,
 
 	  if (GET_MODE_SIZE (rhs_mode) == fltsz)
 	    {
-	      if (!supportable_convert_operation (code, vectype_out,
-						  cvt_type, &codecvt1))
+	      tc1 = ERROR_MARK;
+	      gcc_assert (code.is_tree_code ());
+	      if (!supportable_convert_operation ((tree_code) code, vectype_out,
+						  cvt_type, &tc1))
 		goto unsupported;
+	      codecvt1 = tc1;
 	    }
-	  else if (!supportable_widening_operation (vinfo, code, stmt_info,
-						    vectype_out, cvt_type,
-						    &codecvt1, &codecvt2,
-						    &multi_step_cvt,
+	  else if (!supportable_widening_operation (vinfo, code,
+						    stmt_info, vectype_out,
+						    cvt_type, &codecvt1,
+						    &codecvt2, &multi_step_cvt,
 						    &interm_types))
 	    continue;
 	  else
@@ -5223,8 +5250,9 @@ vectorizable_conversion (vec_info *vinfo,
 
 	  if (supportable_widening_operation (vinfo, NOP_EXPR, stmt_info,
 					      cvt_type,
-					      vectype_in, &code1, &code2,
-					      &multi_step_cvt, &interm_types))
+					      vectype_in, &code1,
+					      &code2, &multi_step_cvt,
+					      &interm_types))
 	    {
 	      found_mode = true;
 	      break;
@@ -5260,9 +5288,11 @@ vectorizable_conversion (vec_info *vinfo,
       cvt_type = get_same_sized_vectype (cvt_type, vectype_in);
       if (cvt_type == NULL_TREE)
 	goto unsupported;
-      if (!supportable_convert_operation (code, cvt_type, vectype_in,
-					  &codecvt1))
+      if (!code.is_tree_code ()
+	  || !supportable_convert_operation ((tree_code) code, cvt_type,
+					     vectype_in, &tc1))
 	goto unsupported;
+      codecvt1 = tc1;
       if (supportable_narrowing_operation (NOP_EXPR, vectype_out, cvt_type,
 					   &code1, &multi_step_cvt,
 					   &interm_types))
@@ -5380,10 +5410,9 @@ vectorizable_conversion (vec_info *vinfo,
       FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
 	{
 	  /* Arguments are ready, create the new vector stmt.  */
-	  gcc_assert (TREE_CODE_LENGTH (code1) == unary_op);
-	  gassign *new_stmt = gimple_build_assign (vec_dest, code1, vop0);
+	  gimple *new_stmt = vect_gimple_build (vec_dest, code1, vop0);
 	  new_temp = make_ssa_name (vec_dest, new_stmt);
-	  gimple_assign_set_lhs (new_stmt, new_temp);
+	  gimple_set_lhs (new_stmt, new_temp);
 	  vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
 
 	  if (slp_node)
@@ -5413,17 +5442,16 @@ vectorizable_conversion (vec_info *vinfo,
       for (i = multi_step_cvt; i >= 0; i--)
 	{
 	  tree this_dest = vec_dsts[i];
-	  enum tree_code c1 = code1, c2 = code2;
+	  code_helper c1 = code1, c2 = code2;
 	  if (i == 0 && codecvt2 != ERROR_MARK)
 	    {
 	      c1 = codecvt1;
 	      c2 = codecvt2;
 	    }
 	  if (known_eq (nunits_out, nunits_in))
-	    vect_create_half_widening_stmts (vinfo, &vec_oprnds0,
-						    &vec_oprnds1, stmt_info,
-						    this_dest, gsi,
-						    c1, op_type);
+	    vect_create_half_widening_stmts (vinfo, &vec_oprnds0, &vec_oprnds1,
+					     stmt_info, this_dest, gsi, c1,
+					     op_type);
 	  else
 	    vect_create_vectorized_promotion_stmts (vinfo, &vec_oprnds0,
 						    &vec_oprnds1, stmt_info,
@@ -5436,9 +5464,8 @@ vectorizable_conversion (vec_info *vinfo,
 	  gimple *new_stmt;
 	  if (cvt_type)
 	    {
-	      gcc_assert (TREE_CODE_LENGTH (codecvt1) == unary_op);
 	      new_temp = make_ssa_name (vec_dest);
-	      new_stmt = gimple_build_assign (new_temp, codecvt1, vop0);
+	      new_stmt = vect_gimple_build (new_temp, codecvt1, vop0);
 	      vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
 	    }
 	  else
@@ -5462,10 +5489,8 @@ vectorizable_conversion (vec_info *vinfo,
       if (cvt_type)
 	FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
 	  {
-	    gcc_assert (TREE_CODE_LENGTH (codecvt1) == unary_op);
 	    new_temp = make_ssa_name (vec_dest);
-	    gassign *new_stmt
-	      = gimple_build_assign (new_temp, codecvt1, vop0);
+	    gimple *new_stmt = vect_gimple_build (new_temp, codecvt1, vop0);
 	    vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
 	    vec_oprnds0[i] = new_temp;
 	  }
@@ -12294,9 +12319,11 @@ vect_maybe_update_slp_op_vectype (slp_tree op, tree vectype)
 
 bool
 supportable_widening_operation (vec_info *vinfo,
-				enum tree_code code, stmt_vec_info stmt_info,
+				code_helper code,
+				stmt_vec_info stmt_info,
 				tree vectype_out, tree vectype_in,
-                                enum tree_code *code1, enum tree_code *code2,
+				code_helper *code1,
+				code_helper *code2,
                                 int *multi_step_cvt,
                                 vec<tree> *interm_types)
 {
@@ -12307,7 +12334,7 @@ supportable_widening_operation (vec_info *vinfo,
   optab optab1, optab2;
   tree vectype = vectype_in;
   tree wide_vectype = vectype_out;
-  enum tree_code c1, c2;
+  tree_code c1 = MAX_TREE_CODES, c2 = MAX_TREE_CODES;
   int i;
   tree prev_type, intermediate_type;
   machine_mode intermediate_mode, prev_mode;
@@ -12317,8 +12344,12 @@ supportable_widening_operation (vec_info *vinfo,
   if (loop_info)
     vect_loop = LOOP_VINFO_LOOP (loop_info);
 
-  switch (code)
+  switch (code.safe_as_tree_code ())
     {
+    case MAX_TREE_CODES:
+      /* Don't set c1 and c2 if code is not a tree_code.  */
+      break;
+
     case WIDEN_MULT_EXPR:
       /* The result of a vectorized widening operation usually requires
 	 two vectors (because the widened results do not fit into one vector).
@@ -12358,8 +12389,9 @@ supportable_widening_operation (vec_info *vinfo,
 	  && !nested_in_vect_loop_p (vect_loop, stmt_info)
 	  && supportable_widening_operation (vinfo, VEC_WIDEN_MULT_EVEN_EXPR,
 					     stmt_info, vectype_out,
-					     vectype_in, code1, code2,
-					     multi_step_cvt, interm_types))
+					     vectype_in, code1,
+					     code2, multi_step_cvt,
+					     interm_types))
         {
           /* Elements in a vector with vect_used_by_reduction property cannot
              be reordered if the use chain with this property does not have the
@@ -12435,7 +12467,7 @@ supportable_widening_operation (vec_info *vinfo,
       optab1 = optab_for_tree_code (c1, vectype_out, optab_default);
       optab2 = optab_for_tree_code (c2, vectype_out, optab_default);
     }
-  else if (CONVERT_EXPR_CODE_P (code)
+  else if (CONVERT_EXPR_CODE_P (code.safe_as_tree_code ())
 	   && VECTOR_BOOLEAN_TYPE_P (wide_vectype)
 	   && VECTOR_BOOLEAN_TYPE_P (vectype)
 	   && TYPE_MODE (wide_vectype) == TYPE_MODE (vectype)
@@ -12460,8 +12492,12 @@ supportable_widening_operation (vec_info *vinfo,
        || (icode2 = optab_handler (optab2, vec_mode)) == CODE_FOR_nothing)
     return false;
 
-  *code1 = c1;
-  *code2 = c2;
+  if (code.is_tree_code ())
+  {
+    *code1 = c1;
+    *code2 = c2;
+  }
+
 
   if (insn_data[icode1].operand[0].mode == TYPE_MODE (wide_vectype)
       && insn_data[icode2].operand[0].mode == TYPE_MODE (wide_vectype))
@@ -12482,7 +12518,7 @@ supportable_widening_operation (vec_info *vinfo,
   prev_type = vectype;
   prev_mode = vec_mode;
 
-  if (!CONVERT_EXPR_CODE_P (code))
+  if (!CONVERT_EXPR_CODE_P (code.safe_as_tree_code ()))
     return false;
 
   /* We assume here that there will not be more than MAX_INTERM_CVT_STEPS
@@ -12580,9 +12616,9 @@ supportable_widening_operation (vec_info *vinfo,
    narrowing operation (short in the above example).   */
 
 bool
-supportable_narrowing_operation (enum tree_code code,
+supportable_narrowing_operation (code_helper code,
 				 tree vectype_out, tree vectype_in,
-				 enum tree_code *code1, int *multi_step_cvt,
+				 code_helper *code1, int *multi_step_cvt,
                                  vec<tree> *interm_types)
 {
   machine_mode vec_mode;
@@ -12597,8 +12633,11 @@ supportable_narrowing_operation (enum tree_code code,
   unsigned HOST_WIDE_INT n_elts;
   bool uns;
 
+  if (!code.is_tree_code ())
+    return false;
+
   *multi_step_cvt = 0;
-  switch (code)
+  switch ((tree_code) code)
     {
     CASE_CONVERT:
       c1 = VEC_PACK_TRUNC_EXPR;
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 9cf2fb23fe397b467d89aa7cc5ebeaa293ed4cce..f215cd0639bcf803c9d0554cfdc57823431991d5 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -2139,13 +2139,12 @@ extern bool vect_is_simple_use (vec_info *, stmt_vec_info, slp_tree,
 				enum vect_def_type *,
 				tree *, stmt_vec_info * = NULL);
 extern bool vect_maybe_update_slp_op_vectype (slp_tree, tree);
-extern bool supportable_widening_operation (vec_info *,
-					    enum tree_code, stmt_vec_info,
-					    tree, tree, enum tree_code *,
-					    enum tree_code *, int *,
-					    vec<tree> *);
-extern bool supportable_narrowing_operation (enum tree_code, tree, tree,
-					     enum tree_code *, int *,
+extern bool supportable_widening_operation (vec_info*, code_helper,
+					    stmt_vec_info, tree, tree,
+					    code_helper*, code_helper*,
+					    int*, vec<tree> *);
+extern bool supportable_narrowing_operation (code_helper, tree, tree,
+					     code_helper *, int *,
 					     vec<tree> *);
 
 extern unsigned record_stmt_cost (stmt_vector_for_cost *, int,
@@ -2583,4 +2582,7 @@ vect_is_integer_truncation (stmt_vec_info stmt_info)
 	  && TYPE_PRECISION (lhs_type) < TYPE_PRECISION (rhs_type));
 }
 
+/* Build a GIMPLE_ASSIGN or GIMPLE_CALL with the tree_code,
+   or internal_fn contained in ch, respectively.  */
+gimple * vect_gimple_build (tree, code_helper, tree, tree = NULL_TREE);
 #endif  /* GCC_TREE_VECTORIZER_H  */
diff --git a/gcc/tree.h b/gcc/tree.h
index 0b72663e6a1a94406127f6253460f498b7a3ea9c..6dcb28ebc1df456e3798b8f0b43bae42c145d43d 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -93,6 +93,8 @@ public:
   bool is_internal_fn () const;
   bool is_builtin_fn () const;
   int get_rep () const { return rep; }
+  tree_code safe_as_tree_code () const;
+  combined_fn safe_as_fn_code () const;
   bool operator== (const code_helper &other) { return rep == other.rep; }
   bool operator!= (const code_helper &other) { return rep != other.rep; }
   bool operator== (tree_code c) { return rep == code_helper (c).rep; }
@@ -102,6 +104,25 @@ private:
   int rep;
 };
 
+/* Helper function that returns the tree_code representation of THIS
+   code_helper if it is a tree_code and MAX_TREE_CODES otherwise.  This is
+   useful when passing a code_helper to a tree_code only check.  */
+
+inline tree_code
+code_helper::safe_as_tree_code () const
+{
+  return is_tree_code () ? (tree_code) *this : MAX_TREE_CODES;
+}
+
+/* Helper function that returns the combined_fn representation of THIS
+   code_helper if it is a fn_code and CFN_LAST otherwise.  This is useful when
+   passing a code_helper to a combined_fn only check.  */
+
+inline combined_fn
+code_helper::safe_as_fn_code () const {
+  return is_fn_code () ? (combined_fn) *this : CFN_LAST;
+}
+
 inline code_helper::operator internal_fn () const
 {
   return as_internal_fn (combined_fn (*this));

On Fri, 12 May 2023, Andre Vieira (lists) wrote:

> Hi,
> 
> I think I tackled all of your comments, let me know if I missed something.

This first and the last patch look good to me now.  Let me comment on the
second.

Thanks,
Richard.

> 
> gcc/ChangeLog:
> 
> 2023-05-12  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>             Joel Hutton  <joel.hutton@arm.com>
> 
>         * tree-vect-patterns.cc (vect_gimple_build): New Function.
>         (vect_recog_widen_op_pattern): Refactor to use code_helper.
>         * tree-vect-stmts.cc (vect_gen_widened_results_half): Likewise.
>         (vect_create_vectorized_demotion_stmts): Likewise.
>         (vect_create_vectorized_promotion_stmts): Likewise.
>         (vect_create_half_widening_stmts): Likewise.
>         (vectorizable_conversion): Likewise.
>         (vectorizable_call): Likewise.
>         (supportable_widening_operation): Likewise.
>         (supportable_narrowing_operation): Likewise.
>         (simple_integer_narrowing): Likewise.
>         * tree-vectorizer.h (supportable_widening_operation): Likewise.
>         (supportable_narrowing_operation): Likewise.
>         (vect_gimple_build): New function prototype.
>         * tree.h (code_helper::safe_as_tree_code): New function.
>         (code_helper::safe_as_fn_code): New function.
>