[PATCH 1/2] choose VF independent of externs/constants

Richard Biener <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <[email protected]>
The following decides on the VF based on assigned SLP_TREE_VECTYPE
rather than on the tracked max_nunits in the SLP graph which also
factors in external and constant nodes.  Those get their vector
type assigned in vectorizable_* via vect_maybe_update_slp_op_vectype
and there's freedom to vectorizable_* to pick a suitable one.

Specifically conversions of externs/constants can cause a higher
than necessary VF which would be a missed optimization.

Changing the VF as outlined will run into both vect_prologue_cost_for_slp
and vect_create_constant_vectors ICEing when computing
vect_get_num_copies since that asserts it can exact_div the
number of lanes as in the unrolled loop by the number of lanes in the
(out of loop) vector type.  But vectorizable_* can opt to just
use the lowpart of such vectors but require it in full due to target
constraints.  So this RFC patch introduces vect_get_num_copies_for_invariant
which performs a ceil_div instead (I think we can maybe change the global
copy to do that).  I added verification to vect_slp_analyze_node_operations
which should make the vect_maybe_update_slp_op_vectype change in
the predicated tails series unneeded.

This shifts the ICE to vectorizable_conversion which is mightly confused
by invariant vectors with too many lanes (in O3-vect-pr32243.c it is
unswitching that exposes a loop invariant conversion in the loop).
I have installed a narrow fix that is likely incomplete and I expect
similar issues to appear in multi-operand widening/narrowing operations.

The fundamental issue is that we expose constant/externals to
vectorizable_* that might have an excess number of lanes, and some
vectorizable_* might not be prepared (and most should be unaffected).
As vectorizable_* control the vector types themselves the fix lies
within them.  Once fuzzers increase coverage.

Bootstrapped and tested on x86_64-unknown-linux-gnu and
on aarch64-linux-gnu.  I've also built SPEC CPU 2017 on x86_64
without hitting additional issues.

OK?  As said, this should elide two patches from the predicated
tails series which is the motivation to come back to this now.

Thanks,
Richard.

	* tree-vect-slp.cc (vect_update_slp_vf_for_node): Compute
	VF based on SLP_TREE_VECTYPE only.
	(vect_get_num_copies_for_invariant): New.
	(vect_prologue_cost_for_slp): Take nvectors as argument.
	(vect_slp_analyze_node_operations): Verify that we can
	div_away_from_zero for vect_prologue_cost_for_slp.
	(vect_create_constant_vectors): Use vect_get_num_copies_for_invariant.
	* tree-vect-stmts.cc (vect_create_vectorized_promotion_stmts):
	Support creating half of the promoted results.
	(vectorizable_conversion): When we have excess input elements
	request half of the promoted results.
---
 gcc/tree-vect-slp.cc   | 52 +++++++++++++++++++++++++++++++++---------
 gcc/tree-vect-stmts.cc | 43 +++++++++++++++++++---------------
 2 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index ed0ebad744e..505fe1dbed4 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -8678,16 +8678,19 @@ vect_update_slp_vf_for_node (slp_tree node, poly_uint64 &vf,
     vect_update_slp_vf_for_node (child, vf, visited);
 
   /* We do not visit SLP nodes for constants or externals - those neither
-     have a vector type set yet (vectorizable_* does this) nor do they
-     have max_nunits set.  Instead we rely on internal nodes max_nunit
-     to cover constant/external operands.
+     have a vector type set yet (vectorizable_* does this).
      Note that when we stop using fixed size vectors externs and constants
      shouldn't influence the (minimum) vectorization factor, instead
      vectorizable_* should honor the vectorization factor when trying to
      assign vector types to constants and externals and cause iteration
      to a higher vectorization factor when required.  */
+  tree vectype = SLP_TREE_VECTYPE (node);
+  if (!vectype)
+    /* OMP SIMD calls w/o LHS have no SLP_TREE_VECTYPE set.  */
+    return;
   poly_uint64 node_vf
-    = calculate_unrolling_factor (node->max_nunits, SLP_TREE_LANES (node));
+    = calculate_unrolling_factor (TYPE_VECTOR_SUBPARTS (vectype),
+				  SLP_TREE_LANES (node));
   vf = force_common_multiple (vf, node_vf);
 
   /* For permute nodes that are fed from externs or constants we have to
@@ -8697,7 +8700,7 @@ vect_update_slp_vf_for_node (slp_tree node, poly_uint64 &vf,
       if (SLP_TREE_DEF_TYPE (child) != vect_internal_def)
 	{
 	  poly_uint64 child_vf
-	    = calculate_unrolling_factor (node->max_nunits,
+	    = calculate_unrolling_factor (TYPE_VECTOR_SUBPARTS (vectype),
 					  SLP_TREE_LANES (child));
 	  vf = force_common_multiple (vf, child_vf);
 	}
@@ -8994,11 +8997,33 @@ vect_scalar_ops_slice_hash::equal (const value_type &s1,
   return true;
 }
 
+/* Like vect_get_num_copies but N copies of the vector might have
+   excess elements in the last vector.  Returns false if *NVECTORS
+   cannot be computed.  */
+
+static bool
+vect_get_num_copies_for_invariant (vec_info *vinfo, slp_tree node,
+				   unsigned *nvectors)
+{
+  poly_uint64 vf;
+
+  if (loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo))
+    vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
+  else
+    vf = 1;
+  vf *= SLP_TREE_LANES (node);
+
+  tree vectype = SLP_TREE_VECTYPE (node);
+  bool res = can_div_away_from_zero_p (vf, TYPE_VECTOR_SUBPARTS (vectype),
+				       nvectors);
+  return res;
+}
+
 /* Compute the prologue cost for invariant or constant operands represented
    by NODE.  */
 
 static void
-vect_prologue_cost_for_slp (vec_info *vinfo, slp_tree node,
+vect_prologue_cost_for_slp (slp_tree node, unsigned nvectors,
 			    stmt_vector_for_cost *cost_vec)
 {
   /* There's a special case of an existing vector, that costs nothing.  */
@@ -9012,7 +9037,6 @@ vect_prologue_cost_for_slp (vec_info *vinfo, slp_tree node,
   unsigned group_size = SLP_TREE_LANES (node);
   unsigned HOST_WIDE_INT const_nunits;
   unsigned nelt_limit;
-  unsigned nvectors = vect_get_num_copies (vinfo, node);
   auto ops = &SLP_TREE_SCALAR_OPS (node);
   auto_vec<unsigned int> starts (nvectors);
   if (TYPE_VECTOR_SUBPARTS (vectype).is_constant (&const_nunits)
@@ -9071,7 +9095,7 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node,
   int i, j;
   slp_tree child;
 
-  /* Assume we can code-generate all invariants.  */
+  /* Costing and analysis of invariants is delayed.  */
   if (!node
       || SLP_TREE_DEF_TYPE (node) == vect_constant_def
       || SLP_TREE_DEF_TYPE (node) == vect_external_def)
@@ -9174,8 +9198,11 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node,
 	      continue;
 	    }
 
-	  /* And cost them.  */
-	  vect_prologue_cost_for_slp (vinfo, child, cost_vec);
+	  /* Make sure we can generate them and then cost them.  */
+	  unsigned nvectors;
+	  if (!vect_get_num_copies_for_invariant (vinfo, node, &nvectors))
+	    return false;
+	  vect_prologue_cost_for_slp (child, nvectors, cost_vec);
 	}
 
   /* If this node or any of its children can't be vectorized, try pruning
@@ -10942,7 +10969,10 @@ vect_create_constant_vectors (vec_info *vinfo, slp_tree op_node)
   /* We always want SLP_TREE_VECTYPE (op_node) here correctly set.  */
   vector_type = SLP_TREE_VECTYPE (op_node);
 
-  unsigned int number_of_vectors = vect_get_num_copies (vinfo, op_node);
+  unsigned int number_of_vectors;
+  bool res = vect_get_num_copies_for_invariant (vinfo, op_node,
+						&number_of_vectors);
+  gcc_assert (res);
   SLP_TREE_VEC_DEFS (op_node).create (number_of_vectors);
   auto_vec<tree> voprnds (number_of_vectors);
 
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 38a17e29a84..a8a2ec62920 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -5160,7 +5160,8 @@ vect_create_vectorized_demotion_stmts (vec_info *vinfo, vec<tree> *vec_oprnds,
 /* Create vectorized promotion statements for vector operands from VEC_OPRNDS0
    and VEC_OPRNDS1, for a binary operation associated with scalar statement
    STMT_INFO.  For multi-step conversions store the resulting vectors and
-   call the function recursively.  */
+   call the function recursively.  When HALF is true only generate half
+   of the result.  */
 
 static void
 vect_create_vectorized_promotion_stmts (vec_info *vinfo,
@@ -5169,14 +5170,15 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo,
 					stmt_vec_info stmt_info, tree vec_dest,
 					gimple_stmt_iterator *gsi,
 					code_helper ch1,
-					code_helper ch2, int op_type)
+					code_helper ch2, int op_type,
+					bool half)
 {
   int i;
   tree vop0, vop1, new_tmp1, new_tmp2;
   gimple *new_stmt1, *new_stmt2;
   vec<tree> vec_tmp = vNULL;
 
-  vec_tmp.create (vec_oprnds0->length () * 2);
+  vec_tmp.create ((half ? 1 : 2) * vec_oprnds0->length ());
   FOR_EACH_VEC_ELT (*vec_oprnds0, i, vop0)
     {
       if (op_type == binary_op)
@@ -5188,23 +5190,20 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo,
       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, ch2, vop0, vop1,
-						 op_type, vec_dest, gsi,
-						 stmt_info);
-      if (is_gimple_call (new_stmt1))
-	{
-	  new_tmp1 = gimple_call_lhs (new_stmt1);
-	  new_tmp2 = gimple_call_lhs (new_stmt2);
-	}
-      else
+      new_tmp1 = gimple_get_lhs (new_stmt1);
+      vec_tmp.quick_push (new_tmp1);
+
+      if (vec_tmp.space (1))
 	{
-	  new_tmp1 = gimple_assign_lhs (new_stmt1);
-	  new_tmp2 = gimple_assign_lhs (new_stmt2);
+	  new_stmt2 = vect_gen_widened_results_half (vinfo, ch2, vop0, vop1,
+						     op_type, vec_dest, gsi,
+						     stmt_info);
+	  new_tmp2 = gimple_get_lhs (new_stmt2);
+	  vec_tmp.quick_push (new_tmp2);
 	}
 
-      /* Store the results for the next step.  */
-      vec_tmp.quick_push (new_tmp1);
-      vec_tmp.quick_push (new_tmp2);
+      if (!vec_tmp.space (1))
+	break;
     }
 
   vec_oprnds0->release ();
@@ -5760,6 +5759,7 @@ vectorizable_conversion (vec_info *vinfo,
 					    widen_or_narrow_float_p
 					    ? vectype_out : cvt_type);
 
+  unsigned num_vectors = vect_get_num_copies (vinfo, slp_node);
   switch (modifier)
     {
     case NONE:
@@ -5843,10 +5843,17 @@ vectorizable_conversion (vec_info *vinfo,
 					     stmt_info, this_dest, gsi, c1,
 					     op_type);
 	  else
+	    /* ???  For constant/external inputs we can end up with
+	       excess lanes.  When the number of inputs already match
+	       the number of required outputs request half of the
+	       lanes (gcc.dg/vect/O3-vect-pr32243.c).  Low coverage
+	       makes this likely incomplete.  */
 	    vect_create_vectorized_promotion_stmts (vinfo, &vec_oprnds0,
 						    &vec_oprnds1, stmt_info,
 						    this_dest, gsi,
-						    c1, c2, op_type);
+						    c1, c2, op_type,
+						    vec_oprnds0.length ()
+						    == num_vectors);
 	}
 
       FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
-- 
2.51.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.