[PATCH v2 4/16]middle-end: Add dissolve code for when SLP fails and non-SLP loop vectorization is to be tried.

[PATCH v2 4/16]middle-end: Add dissolve code for when SLP fails and non-SLP loop vectorization is to be tried.

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi All,

This adds the dissolve code to undo the patterns created by the pattern matcher
in case SLP is to be aborted.

As mentioned in the cover letter this has one issue in that the number of copies
can needed can change depending on whether TWO_OPERATORS is needed or not.

Because of this I don't analyze the original statement when it's replaced by a
pattern and attempt to correct it here by analyzing it after dissolve.

This however seems too late and I would need to change the unroll factor, which
seems a bit odd.  Any advice would be appreciated.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Thanks,
Tamar

gcc/ChangeLog:

	* tree-vect-loop.c (vect_dissolve_slp_only_patterns): New
	(vect_dissolve_slp_only_groups): Call vect_dissolve_slp_only_patterns.

-- 

[-- Attachment #2: rb13508.patch --]
[-- Type: text/x-diff, Size: 3685 bytes --]

diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index b1a6e1508c7f00f5f369ec873f927f30d673059e..8231ad6452af6ff111911a7bfb6aab2257df9fc0 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -1956,6 +1956,92 @@ vect_get_datarefs_in_loop (loop_p loop, basic_block *bbs,
   return opt_result::success ();
 }
 
+/* For every SLP only pattern created by the pattern matched rooted in ROOT
+   restore the relevancy of the original statements over those of the pattern
+   and destroy the pattern relationship.  This restores the SLP tree to a state
+   where it can be used when SLP build is cancelled or re-tried.  */
+
+static opt_result
+vect_dissolve_slp_only_patterns (loop_vec_info loop_vinfo,
+				 hash_set<slp_tree> *visited, slp_tree root)
+{
+  if (!root || visited->contains (root))
+    return opt_result::success ();
+
+  unsigned int i;
+  slp_tree node;
+  opt_result res = opt_result::success ();
+  stmt_vec_info stmt_info;
+  stmt_vec_info related_stmt_info;
+  bool need_to_vectorize = false;
+  auto_vec<stmt_info_for_cost> cost_vec;
+
+  visited->add (root);
+
+  FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (root), i, stmt_info)
+    if (STMT_VINFO_SLP_VECT_ONLY (stmt_info)
+        && (related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info)) != NULL)
+      {
+	if (dump_enabled_p ())
+	  dump_printf_loc (MSG_NOTE, vect_location,
+			   "dissolving relevancy of %G over %G",
+			   STMT_VINFO_STMT (stmt_info),
+			   STMT_VINFO_STMT (related_stmt_info));
+	STMT_VINFO_RELEVANT (stmt_info) = vect_unused_in_scope;
+	STMT_VINFO_RELEVANT (related_stmt_info) = vect_used_in_scope;
+	STMT_VINFO_IN_PATTERN_P (related_stmt_info) = false;
+	STMT_SLP_TYPE (related_stmt_info) = hybrid;
+	/* Now we have to re-analyze the statement since we skipped it in the
+	   the initial analysis due to the differences in copies.  */
+	res = vect_analyze_stmt (loop_vinfo, related_stmt_info,
+				 &need_to_vectorize, NULL, NULL, &cost_vec);
+
+	if (!res)
+	  return res;
+      }
+
+  FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (root), i, node)
+    {
+      res = vect_dissolve_slp_only_patterns (loop_vinfo, visited, node);
+      if (!res)
+	return res;
+    }
+
+  return res;
+}
+
+/* Lookup any SLP Only Pattern statements created by the SLP pattern matcher in
+   all slp_instances in LOOP_VINFO and undo the relevancy of statements such
+   that the original SLP tree before the pattern matching is used.  */
+
+static opt_result
+vect_dissolve_slp_only_patterns (loop_vec_info loop_vinfo)
+{
+
+  unsigned int i;
+  opt_result res = opt_result::success ();
+  hash_set<slp_tree> *visited = new hash_set<slp_tree> ();
+
+  DUMP_VECT_SCOPE ("vect_dissolve_slp_only_patterns");
+
+  /* Unmark any SLP only patterns as relevant and restore the STMT_INFO of the
+     related instruction.  */
+  slp_instance instance;
+  FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), i, instance)
+    {
+      res = vect_dissolve_slp_only_patterns (loop_vinfo, visited,
+					     SLP_INSTANCE_TREE (instance));
+      if (!res)
+	{
+	  delete visited;
+	  return res;
+	}
+    }
+
+  delete visited;
+  return res;
+}
+
 /* Look for SLP-only access groups and turn each individual access into its own
    group.  */
 static void
@@ -2427,6 +2513,11 @@ again:
   /* Ensure that "ok" is false (with an opt_problem if dumping is enabled).  */
   gcc_assert (!ok);
 
+  /* Dissolve any SLP patterns created by the SLP pattern matcher.  */
+  opt_result dissolved = vect_dissolve_slp_only_patterns (loop_vinfo);
+  if (!dissolved)
+    return dissolved;
+
   /* Try again with SLP forced off but if we didn't do any SLP there is
      no point in re-trying.  */
   if (!slp)

Re: [PATCH v2 4/16]middle-end: Add dissolve code for when SLP fails and non-SLP loop vectorization is to be tried.

[Richard Biener]

On Fri, 25 Sep 2020, Tamar Christina wrote:

> Hi All,
> 
> This adds the dissolve code to undo the patterns created by the pattern matcher
> in case SLP is to be aborted.
> 
> As mentioned in the cover letter this has one issue in that the number of copies
> can needed can change depending on whether TWO_OPERATORS is needed or not.
> 
> Because of this I don't analyze the original statement when it's replaced by a
> pattern and attempt to correct it here by analyzing it after dissolve.
> 
> This however seems too late and I would need to change the unroll factor, which
> seems a bit odd.  Any advice would be appreciated.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ah, this is what you mean with the need to dissolve.  Yeah ...

@@ -2427,6 +2513,11 @@ again:
   /* Ensure that "ok" is false (with an opt_problem if dumping is 
enabled).  */
   gcc_assert (!ok);

+  /* Dissolve any SLP patterns created by the SLP pattern matcher.  */
+  opt_result dissolved = vect_dissolve_slp_only_patterns (loop_vinfo);
+  if (!dissolved)
+    return dissolved;
+
   /* Try again with SLP forced off but if we didn't do any SLP there is
      no point in re-trying.  */
   if (!slp)

I think this only belongs after

  if (dump_enabled_p ())
    dump_printf_loc (MSG_NOTE, vect_location,
                     "re-trying with SLP disabled\n");

  /* Roll back state appropriately.  No SLP this time.  */
  slp = false;

thus where everything else is "restored".  In fact I wonder if
it cannot be done as part of

  /* Reset SLP type to loop_vect on all stmts.  */
  for (i = 0; i < LOOP_VINFO_LOOP (loop_vinfo)->num_nodes; ++i)
    {
      basic_block bb = LOOP_VINFO_BBS (loop_vinfo)[i];
...

?  In particular

+       /* Now we have to re-analyze the statement since we skipped it in 
the
+          the initial analysis due to the differences in copies.  */
+       res = vect_analyze_stmt (loop_vinfo, related_stmt_info,
+                                &need_to_vectorize, NULL, NULL, 
&cost_vec);

looks unneeded since we're going to re-analyze all stmts anyway?

The thing is, there's no way to recover the original pattern stmt
in case your SLP pattern stmt was composed of "regular" pattern
stmts (the STMT_VINFO_RELATED_STMT simply gets replaced).  I
wonder if it would be easier to record the SLP pattern stmt
only in SLP_TREE_REPRESENTATIVE but _not_ in SLP_TREE_SCALAR_STMTS
(just leave those alone)?

Richard.


> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 	* tree-vect-loop.c (vect_dissolve_slp_only_patterns): New
> 	(vect_dissolve_slp_only_groups): Call vect_dissolve_slp_only_patterns.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

RE: [PATCH v2 4/16]middle-end: Add dissolve code for when SLP fails and non-SLP loop vectorization is to be tried.

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

Hi All,

Here's a respin of this patch with the requested changes.

Thanks,
Tamar

gcc/ChangeLog:

	* tree-vect-loop.c (vect_dissolve_slp_only_patterns): New
	(vect_dissolve_slp_only_groups): Call vect_dissolve_slp_only_patterns.

> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of Tamar
> Christina
> Sent: Friday, September 25, 2020 3:28 PM
> To: gcc-patches@gcc.gnu.org
> Cc: nd <nd@arm.com>; rguenther@suse.de; ook@ucw.cz
> Subject: [PATCH v2 4/16]middle-end: Add dissolve code for when SLP fails
> and non-SLP loop vectorization is to be tried.
> 
> Hi All,
> 
> This adds the dissolve code to undo the patterns created by the pattern
> matcher in case SLP is to be aborted.
> 
> As mentioned in the cover letter this has one issue in that the number of
> copies can needed can change depending on whether TWO_OPERATORS is
> needed or not.
> 
> Because of this I don't analyze the original statement when it's replaced by a
> pattern and attempt to correct it here by analyzing it after dissolve.
> 
> This however seems too late and I would need to change the unroll factor,
> which seems a bit odd.  Any advice would be appreciated.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 	* tree-vect-loop.c (vect_dissolve_slp_only_patterns): New
> 	(vect_dissolve_slp_only_groups): Call
> vect_dissolve_slp_only_patterns.
> 
> --

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr13508.patch --]
[-- Type: text/x-diff; name="pr13508.patch", Size: 2932 bytes --]

diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 6fa185daa2836062814f9c9a6659011a3153c6a2..9601a83edcb05e994e27d4bb16a537190ad8471d 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -1979,6 +1979,63 @@ vect_get_datarefs_in_loop (loop_p loop, basic_block *bbs,
   return opt_result::success ();
 }
 
+/* For every SLP only pattern created by the pattern matched rooted in ROOT
+   restore the relevancy of the original statements over those of the pattern
+   and destroy the pattern relationship.  This restores the SLP tree to a state
+   where it can be used when SLP build is cancelled or re-tried.  */
+
+static void
+vect_dissolve_slp_only_patterns (loop_vec_info loop_vinfo,
+				 hash_set<slp_tree> *visited, slp_tree root)
+{
+  if (!root || visited->contains (root))
+    return;
+
+  unsigned int i;
+  slp_tree node;
+  stmt_vec_info related_stmt_info;
+  stmt_vec_info stmt_info = SLP_TREE_REPRESENTATIVE (root);
+
+  visited->add (root);
+
+    if (stmt_info && STMT_VINFO_SLP_VECT_ONLY (stmt_info)
+	 && (related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info)) != NULL)
+      {
+	if (dump_enabled_p ())
+	  dump_printf_loc (MSG_NOTE, vect_location,
+			   "dissolving relevancy of %G",
+			   STMT_VINFO_STMT (stmt_info));
+	STMT_VINFO_RELEVANT (stmt_info) = vect_unused_in_scope;
+	STMT_VINFO_RELEVANT (related_stmt_info) = vect_used_in_scope;
+	STMT_VINFO_IN_PATTERN_P (related_stmt_info) = false;
+	STMT_SLP_TYPE (related_stmt_info) = loop_vect;
+      }
+
+  FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (root), i, node)
+    vect_dissolve_slp_only_patterns (loop_vinfo, visited, node);
+}
+
+/* Lookup any SLP Only Pattern statements created by the SLP pattern matcher in
+   all slp_instances in LOOP_VINFO and undo the relevancy of statements such
+   that the original SLP tree before the pattern matching is used.  */
+
+static void
+vect_dissolve_slp_only_patterns (loop_vec_info loop_vinfo)
+{
+
+  unsigned int i;
+  hash_set<slp_tree> visited;
+
+  DUMP_VECT_SCOPE ("vect_dissolve_slp_only_patterns");
+
+  /* Unmark any SLP only patterns as relevant and restore the STMT_INFO of the
+     related instruction.  */
+  slp_instance instance;
+  FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), i, instance)
+    vect_dissolve_slp_only_patterns (loop_vinfo, &visited,
+				     SLP_INSTANCE_TREE (instance));
+}
+
 /* Look for SLP-only access groups and turn each individual access into its own
    group.  */
 static void
@@ -2510,6 +2567,9 @@ again:
   /* Ensure that "ok" is false (with an opt_problem if dumping is enabled).  */
   gcc_assert (!ok);
 
+  /* Dissolve any SLP patterns created by the SLP pattern matcher.  */
+  vect_dissolve_slp_only_patterns (loop_vinfo);
+
   /* Try again with SLP forced off but if we didn't do any SLP there is
      no point in re-trying.  */
   if (!slp)