[PATCH][RFC] Remove max_nunits, 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_
which performs a ceil_div instead (I think we can change the global
copy to do that).  This shifts the ICE to vectorizable_conversion
which is mightly confused by "too large" vector types.  IMO rather
than picking a vector type randomly when not already assigned, it
should chose it based on target constraints.  Or, even better,
emit the conversion in scalar.  But I expect similar issues to
appear in multi-operand widening/narrowing operations.

The fundamental issue is that we eventually break the same-size
constraint for loop vectorization here (which we want!), and some
vectorizable_* are not prepared.

On x86_64 I see the following (unsure if related):

FAIL: gcc.target/i386/pr108938-3.c scan-assembler-times bswap[\\t ]+ 3

Otherwise bootstrapped/tested on x86_64-unknown-linux-gnu and
aarch64-linux-gnu.

I suspect coverage for constant/extern operands isn't great, of course
and the vectorizable_conversion hack needs to be properly fixed.
Plus, I wonder when can_div_away_from_zero_p can possibly fail ...

Comments?

Thanks,
Richard.

	* tree-vect-slp.cc (vect_update_slp_vf_for_node): Compute
	VF based on SLP_TREE_VECTYPE only.
	(vect_get_num_copies_): New.
	(vect_prologue_cost_for_slp): Use vect_get_num_copies_.
	(vect_create_constant_vectors): Likewise.
	* tree-vect-stmts.cc (vectorizable_conversion): Chose
	constant/extern vectype based on legacy constraints.
	Add workaround for missed handling.
---
 gcc/tree-vect-slp.cc   | 35 +++++++++++++++++++++++++++++++----
 gcc/tree-vect-stmts.cc | 13 ++++++++++++-
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 30f70453b2b..fcd9329f176 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -8766,8 +8766,13 @@ vect_update_slp_vf_for_node (slp_tree node, poly_uint64 &vf,
      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
@@ -8777,7 +8782,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);
 	}
@@ -9074,6 +9079,28 @@ vect_scalar_ops_slice_hash::equal (const value_type &s1,
   return true;
 }
 
+static unsigned
+vect_get_num_copies_ (vec_info *vinfo, slp_tree node)
+{
+  unsigned nvectors; // = vect_get_num_copies (vinfo, node);
+  /* Inline copy of vect_get_num_copies with ceil_div instead of exact_div.  */
+    {
+      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);
+      gcc_assert (res);
+    }
+  return nvectors;
+}
+
 /* Compute the prologue cost for invariant or constant operands represented
    by NODE.  */
 
@@ -9092,7 +9119,7 @@ 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);
+  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)
@@ -11022,7 +11049,7 @@ 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 = vect_get_num_copies_ (vinfo, op_node);
   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 344aeddfb5a..802b0de40de 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -5442,7 +5442,14 @@ vectorizable_conversion (vec_info *vinfo,
   if (!cost_vec)
     gcc_assert (vectype_in);
   if (!vectype_in)
-    vectype_in = get_vectype_for_scalar_type (vinfo, rhs_type, slp_node);
+    {
+      if (loop_vinfo)
+	vectype_in = get_same_sized_vectype (rhs_type, vectype_out);
+      else
+	vectype_in = get_related_vectype_for_scalar_type
+		       (vinfo->vector_mode, rhs_type,
+			TYPE_VECTOR_SUBPARTS (vectype_out));
+    }
   if (!vectype_in)
     {
       if (dump_enabled_p ())
@@ -5875,6 +5882,10 @@ vectorizable_conversion (vec_info *vinfo,
 
       FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)
 	{
+	  /* ???  FIXME - NARROW/WIDEN assumes all lanes are used
+	     (gcc.dg/vect/O3-vect-pr32243.c).  */
+	  if (!slp_node->vec_defs.space (1))
+	    break;
 	  gimple *new_stmt;
 	  if (cvt_type)
 	    {
-- 
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.