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

Reply via email to