The following tries to address us BB vectorizing a loop body that
swaps consecutive elements of an array like for bubble-sort.  This
causes the vector store in the previous iteration to fail to forward
to the vector load in the current iteration since there's a partial
overlap.

We try to detect this situation by looking for a load to store
data dependence and analyze this with respect to the containing loop
for a proven problematic access.  Currently the search for a
problematic pair is limited to loads and stores in the same SLP
instance which means the problematic load happens in the next
loop iteration and larger dependence distances are not considered.

On x86 with generic costing this avoids vectorizing the loop body,
but once you do core-specific tuning the saved cost for the vector
store vs. the scalar stores makes vectorization still profitable,
but at least the STLF issue is avoided.

For example on my Zen4 machine with -O2 -march=znver4 the testcase in
the PR is improving from
  insertion_sort  =>     2327
to
  insertion_sort  =>      997
but plain -O2 (or -fno-tree-slp-vectorize) gives
  insertion_sort  =>      183
In the end a better target-side cost model for small vector
vectorization is needed to reject this vectorization from this side.

I'll note this is a machine independent heuristic (similar to the
avoid-store-forwarding RTL optimization pass), I expect that uarchs
implementing vectors will suffer from this kind of issue.  I know
some aarch64 uarchs can forward from upper/lower part stores, this
isn't considered at the moment.  The actual vector size/overlap
distance check could be moved to a target hook if it turns out
necessary.

There might be the chance to use a smaller vector size for the loads
avoiding the penalty rather than falling back to elementwise accesses,
that's not implemented either.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  At this point
queued for stage1, possibly for backport for 15.2.

Richard.

        PR tree-optimization/1157777
        * tree-vectorizer.h (_slp_tree::avoid_stlf_fail): New member.
        * tree-vet-slp.cc (_slp_tree::_slp_tree): Initialize it.
        (vect_print_slp_tree): Dump it.
        * tree-vect-data.refs.cc (vect_slp_analyze_instance_dependence):
        For dataflow dependent loads of a store check whether there's
        a cross-iteration data dependence that for sure prohibits
        store-to-load forwarding and mark involved loads.
        * tree-vect-stmts.cc (get_group_load_store_type): For avoid_stlf_fail
        marked loads use VMAT_ELEMENTWISE.

        * gcc.dg/vect/bb-slp-pr115777.c: New testcase.
---
 gcc/testsuite/gcc.dg/vect/bb-slp-pr115777.c | 15 ++++
 gcc/tree-vect-data-refs.cc                  | 91 +++++++++++++++++++++
 gcc/tree-vect-slp.cc                        |  4 +-
 gcc/tree-vect-stmts.cc                      |  8 ++
 gcc/tree-vectorizer.h                       |  3 +
 5 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/bb-slp-pr115777.c

diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pr115777.c 
b/gcc/testsuite/gcc.dg/vect/bb-slp-pr115777.c
new file mode 100644
index 00000000000..bba0dc75f6f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pr115777.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+
+typedef unsigned int T;
+
+#define SWAP(A, B) do { T tmp = A; A = B; B = tmp; } while (0)
+
+void
+insertion_sort(T *v, int n)
+{
+  for (int i = 1; i < n; ++i)
+    for (int k = i; k > 0 && v[k-1] > v[k]; --k)
+      SWAP(v[k-1], v[k]);
+}
+
+/* { dg-final { scan-tree-dump "using element-wise load" "slp1" { target { { 
x86_64-*-* i?86-*-* } && { ! ia32 } } } } } */
diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index c9395e33fcd..231a3cab4f8 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -1203,6 +1203,97 @@ vect_slp_analyze_instance_dependence (vec_info *vinfo, 
slp_instance instance)
     for (unsigned k = 0; k < SLP_TREE_SCALAR_STMTS (store).length (); ++k)
       gimple_set_visited (SLP_TREE_SCALAR_STMTS (store)[k]->stmt, false);
 
+  /* If this is a SLP instance with a store check if there's a dependent
+     load that cannot be forwarded from a previous iteration of a loop
+     both are in.  This is to avoid situations like that in PR115777.  */
+  if (res && store)
+    {
+      stmt_vec_info store_info
+       = DR_GROUP_FIRST_ELEMENT (SLP_TREE_SCALAR_STMTS (store)[0]);
+      class loop *store_loop = gimple_bb (store_info->stmt)->loop_father;
+      if (! loop_outer (store_loop))
+       return res;
+      vec<loop_p> loop_nest;
+      loop_nest.create (1);
+      loop_nest.quick_push (store_loop);
+      data_reference *drs = nullptr;
+      for (slp_tree &load : SLP_INSTANCE_LOADS (instance))
+       {
+         if (! STMT_VINFO_GROUPED_ACCESS (SLP_TREE_SCALAR_STMTS (load)[0]))
+           continue;
+         stmt_vec_info load_info
+           = DR_GROUP_FIRST_ELEMENT (SLP_TREE_SCALAR_STMTS (load)[0]);
+         if (gimple_bb (load_info->stmt)->loop_father != store_loop)
+           continue;
+
+         /* For now concern ourselves with write-after-read as we also
+            only look for re-use of the store within the same SLP instance.
+            We can still get a RAW here when the instance contais a PHI
+            with a backedge though, thus this test.  */
+         if (! vect_stmt_dominates_stmt_p (STMT_VINFO_STMT (load_info),
+                                           STMT_VINFO_STMT (store_info)))
+           continue;
+
+         if (! drs)
+           {
+             drs = create_data_ref (loop_preheader_edge (store_loop),
+                                    store_loop,
+                                    DR_REF (STMT_VINFO_DATA_REF (store_info)),
+                                    store_info->stmt, false, false);
+             if (! DR_BASE_ADDRESS (drs)
+                 || TREE_CODE (DR_STEP (drs)) != INTEGER_CST)
+               break;
+           }
+         data_reference *drl
+           = create_data_ref (loop_preheader_edge (store_loop),
+                              store_loop,
+                              DR_REF (STMT_VINFO_DATA_REF (load_info)),
+                              load_info->stmt, true, false);
+
+         /* See whether the DRs have a known constant distance throughout
+            the containing loop iteration.  */
+         if (! DR_BASE_ADDRESS (drl)
+             || ! operand_equal_p (DR_STEP (drs), DR_STEP (drl))
+             || ! operand_equal_p (DR_BASE_ADDRESS (drs),
+                                   DR_BASE_ADDRESS (drl))
+             || ! operand_equal_p (DR_OFFSET (drs), DR_OFFSET (drl)))
+           {
+             free_data_ref (drl);
+             continue;
+           }
+
+         /* If the next iteration load overlaps with a non-power-of-two offset
+            we are surely failing any STLF attempt.  */
+         HOST_WIDE_INT step = TREE_INT_CST_LOW (DR_STEP (drl));
+         unsigned HOST_WIDE_INT sizes
+           = (TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (drs))))
+              * DR_GROUP_SIZE (store_info));
+         unsigned HOST_WIDE_INT sizel
+           = (TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (drl))))
+              * DR_GROUP_SIZE (load_info));
+         if (ranges_overlap_p (TREE_INT_CST_LOW (DR_INIT (drl)) + step, sizel,
+                               TREE_INT_CST_LOW (DR_INIT (drs)), sizes))
+           {
+             unsigned HOST_WIDE_INT dist
+               = absu_hwi (TREE_INT_CST_LOW (DR_INIT (drl)) + step
+                           - TREE_INT_CST_LOW (DR_INIT (drs)));
+             poly_uint64 loadsz = tree_to_poly_uint64
+                                    (TYPE_SIZE_UNIT (SLP_TREE_VECTYPE (load)));
+             poly_uint64 storesz = tree_to_poly_uint64
+                                   (TYPE_SIZE_UNIT (SLP_TREE_VECTYPE (store)));
+             /* When the overlap aligns with vector sizes used for the loads
+                and the vector stores are larger or equal to the loads
+                forwarding should work.  */
+             if (maybe_gt (loadsz, storesz) || ! multiple_p (dist, loadsz))
+               load->avoid_stlf_fail = true;
+           }
+         free_data_ref (drl);
+       }
+      if (drs)
+       free_data_ref (drs);
+      loop_nest.release ();
+    }
+
   return res;
 }
 
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 19beeed8a3a..23a14ae6071 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -122,6 +122,7 @@ _slp_tree::_slp_tree ()
   SLP_TREE_DEF_TYPE (this) = vect_uninitialized_def;
   SLP_TREE_CODE (this) = ERROR_MARK;
   this->ldst_lanes = false;
+  this->avoid_stlf_fail = false;
   SLP_TREE_VECTYPE (this) = NULL_TREE;
   SLP_TREE_REPRESENTATIVE (this) = NULL;
   SLP_TREE_MEMORY_ACCESS_TYPE (this) = VMAT_INVARIANT;
@@ -3104,7 +3105,8 @@ vect_print_slp_tree (dump_flags_t dump_kind, 
dump_location_t loc,
                                         SLP_TREE_REF_COUNT (node));
   if (SLP_TREE_VECTYPE (node))
     dump_printf (metadata, " %T", SLP_TREE_VECTYPE (node));
-  dump_printf (metadata, "\n");
+  dump_printf (metadata, "%s\n",
+              node->avoid_stlf_fail ? " (avoid-stlf-fail)" : "");
   if (SLP_TREE_DEF_TYPE (node) == vect_internal_def)
     {
       if (SLP_TREE_CODE (node) == VEC_PERM_EXPR)
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 51b66a1883e..ffca2ab50d5 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -2134,6 +2134,14 @@ get_group_load_store_type (vec_info *vinfo, 
stmt_vec_info stmt_info,
                            : vect_store_lanes_supported (vectype, group_size,
                                                          masked_p))) != 
IFN_LAST)
            *memory_access_type = VMAT_LOAD_STORE_LANES;
+         else if (!loop_vinfo && slp_node->avoid_stlf_fail)
+           {
+             *memory_access_type = VMAT_ELEMENTWISE;
+             if (dump_enabled_p ())
+               dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+                                "using element-wise load to avoid disrupting "
+                                "cross iteration store-to-load forwarding\n");
+           }
          else
            *memory_access_type = VMAT_CONTIGUOUS;
 
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 97caf61b345..933d1a4a1ac 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -265,6 +265,9 @@ struct _slp_tree {
   /* Whether uses of this load or feeders of this store are suitable
      for load/store-lanes.  */
   bool ldst_lanes;
+  /* For BB vect, flag to indicate this load node should be vectorized
+     as to avoid STLF fails because of related stores.  */
+  bool avoid_stlf_fail;
 
   int vertex;
 
-- 
2.43.0

Reply via email to