[PATCH] tree-optimization/111137 - dependence checking for SLP

[PATCH] tree-optimization/111137 - dependence checking for SLP

[Richard Biener]

The following fixes a mistake with SLP dependence checking.  When
checking whether we can hoist loads to the first load place we
special-case stores of the same instance considering them sunk
to the last store place.  But we fail to consider that stores from
other SLP instances are sunk in a similar way.  This leads us to
miss the dependence between (A) and (B) in

  b[0][1] = 0;             (A)
...
  _6 = b[_5 /* 0 */][0];   (B')
  _7 = _6 ^ 1;
  b[_5 /* 0 */][0] = _7;
  b[0][2] = 0;             (A')
  _10 = b[_5 /* 0 */][1];  (B)
  _11 = _10 ^ 1;
  b[_5 /* 0 */][1] = _11;

where the zeroing stores are sunk to (A') and the loads hoisted
to (B').  The following fixes this, treating grouped stores from
other instances similar to stores from our own instance.  The
difference is - and this is more conservative than necessary - that
we don't know which stores of a group are in which SLP instance
(though I believe either all of the grouped stores will be in
a single SLP instance or in none at the moment), so we don't
know which stores are sunk where.  We simply assume they are
all sunk to the last store we run into.  Likewise we do not take
into account that an SLP instance might be cancelled (or a grouped
store not actually belong to any instance).

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

	PR tree-optimization/111137
	* tree-vect-data-refs.cc (vect_slp_analyze_load_dependences):
	Properly handle grouped stores from other SLP instances.

	* gcc.dg/torture/pr111137.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr111137.c | 30 ++++++++++++
 gcc/tree-vect-data-refs.cc              | 64 ++++++++++++++++++-------
 2 files changed, 77 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr111137.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr111137.c b/gcc/testsuite/gcc.dg/torture/pr111137.c
new file mode 100644
index 00000000000..77560487926
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr111137.c
@@ -0,0 +1,30 @@
+/* { dg-do run } */
+
+int b[3][8];
+short d;
+volatile int t = 1;
+
+void __attribute__((noipa))
+foo()
+{
+  int  g = t;
+  for (int e = 1; e >= 0; e--)
+    {
+      d = 1;
+      for (; d >= 0; d--)
+	{
+	  b[0][d * 2 + 1] = 0;
+	  b[g - 1 + d][0] ^= 1;
+	  b[0][d * 2 + 2] = 0;
+	  b[g - 1 + d][1] ^= 1;
+	}
+    }
+}
+
+int
+main()
+{
+  foo ();
+  if (b[0][1] != 1)
+    __builtin_abort();
+}
diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index 0295e256a44..40ab568fe35 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -750,6 +750,7 @@ vect_slp_analyze_load_dependences (vec_info *vinfo, slp_tree node,
       data_reference *dr_a = STMT_VINFO_DATA_REF (access_info);
       ao_ref ref;
       bool ref_initialized_p = false;
+      hash_set<stmt_vec_info> grp_visited;
       for (gimple_stmt_iterator gsi = gsi_for_stmt (access_info->stmt);
 	   gsi_stmt (gsi) != first_access_info->stmt; gsi_prev (&gsi))
 	{
@@ -782,26 +783,55 @@ vect_slp_analyze_load_dependences (vec_info *vinfo, slp_tree node,
 	      continue;
 	    }
 
-	  /* We are hoisting a load - this means we can use TBAA for
-	     disambiguation.  */
-	  if (!ref_initialized_p)
-	    ao_ref_init (&ref, DR_REF (dr_a));
-	  if (stmt_may_clobber_ref_p_1 (stmt, &ref, true))
+	  auto check_hoist = [&] (stmt_vec_info stmt_info) -> bool
 	    {
-	      /* If we couldn't record a (single) data reference for this
-		 stmt we have to give up now.  */
-	      data_reference *dr_b = STMT_VINFO_DATA_REF (stmt_info);
-	      if (!dr_b)
-		return false;
-	      ddr_p ddr = initialize_data_dependence_relation (dr_a,
-							       dr_b, vNULL);
-	      bool dependent
-		= vect_slp_analyze_data_ref_dependence (vinfo, ddr);
-	      free_dependence_relation (ddr);
-	      if (dependent)
+	      /* We are hoisting a load - this means we can use TBAA for
+		 disambiguation.  */
+	      if (!ref_initialized_p)
+		ao_ref_init (&ref, DR_REF (dr_a));
+	      if (stmt_may_clobber_ref_p_1 (stmt_info->stmt, &ref, true))
+		{
+		  /* If we couldn't record a (single) data reference for this
+		     stmt we have to give up now.  */
+		  data_reference *dr_b = STMT_VINFO_DATA_REF (stmt_info);
+		  if (!dr_b)
+		    return false;
+		  ddr_p ddr = initialize_data_dependence_relation (dr_a,
+								   dr_b, vNULL);
+		  bool dependent
+		    = vect_slp_analyze_data_ref_dependence (vinfo, ddr);
+		  free_dependence_relation (ddr);
+		  if (dependent)
+		    return false;
+		}
+	      /* No dependence.  */
+	      return true;
+	    };
+	  if (STMT_VINFO_GROUPED_ACCESS (stmt_info))
+	    {
+	      /* When we run into a store group we have to honor
+		 that earlier stores might be moved here.  We don't
+		 know exactly which and where to since we lack a
+		 back-mapping from DR to SLP node, so assume all
+		 earlier stores are sunk here.  It's enough to
+		 consider the last stmt of a group for this.
+		 ???  Both this and the fact that we disregard that
+		 the conflicting instance might be removed later
+		 is overly conservative.  */
+	      if (!grp_visited.add (DR_GROUP_FIRST_ELEMENT (stmt_info)))
+		for (auto store_info = DR_GROUP_FIRST_ELEMENT (stmt_info);
+		     store_info != NULL;
+		     store_info = DR_GROUP_NEXT_ELEMENT (store_info))
+		  if ((store_info == stmt_info
+		       || get_later_stmt (store_info, stmt_info) == stmt_info)
+		      && !check_hoist (store_info))
+		    return false;
+	    }
+	  else
+	    {
+	      if (!check_hoist (stmt_info))
 		return false;
 	    }
-	  /* No dependence.  */
 	}
     }
   return true;
-- 
2.35.3