[PATCH] tree-optimization/96028 - fix bogus externalizing of SLP node

[PATCH] tree-optimization/96028 - fix bogus externalizing of SLP node

[Richard Biener]

This guards externalizing a SLP node when it fails to code generate
to actually have scalar defs we can use.  It also makes failure
to do so not fell the whole SLP instance but instead try this again
on the parent.

This is the variant I installed.

2020-07-02  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/96028
	* tree-vect-slp.c (vect_slp_convert_to_external): Make sure
	we have scalar stmts to use.
	(vect_slp_analyze_node_operations): When analyzing a child
	failed try externalizing the parent node.
---
 gcc/tree-vect-slp.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 532809d2667..be6b7a1ab29 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -2788,6 +2788,7 @@ vect_slp_convert_to_external (vec_info *vinfo, slp_tree node,
 
   if (!is_a <bb_vec_info> (vinfo)
       || node == SLP_INSTANCE_TREE (node_instance)
+      || !SLP_TREE_SCALAR_STMTS (node).exists ()
       || vect_contains_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (node)))
     return false;
 
@@ -2892,16 +2893,25 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node,
      doesn't result in any issue since we throw away the lvisited set
      when we fail.  */
   if (visited.contains (node)
-      || lvisited.add (node))
+      || lvisited.contains (node))
     return true;
 
+  bool res = true;
   FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)
-    if (!vect_slp_analyze_node_operations (vinfo, child, node_instance,
-					   visited, lvisited, cost_vec))
-      return false;
+    {
+      res = vect_slp_analyze_node_operations (vinfo, child, node_instance,
+					      visited, lvisited, cost_vec);
+      if (!res)
+	break;
+    }
 
-  bool res = vect_slp_analyze_node_operations_1 (vinfo, node, node_instance,
-						 cost_vec);
+  if (res)
+    {
+      res = vect_slp_analyze_node_operations_1 (vinfo, node, node_instance,
+						cost_vec);
+      if (res)
+	lvisited.add (node);
+    }
 
   /* When the node can be vectorized cost invariant nodes it references.
      This is not done in DFS order to allow the refering node
@@ -2943,13 +2953,12 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node,
 	  vect_prologue_cost_for_slp (child, cost_vec);
 	}
 
-  /* If this node can't be vectorized, try pruning the tree here rather
-     than felling the whole thing.  */
+  /* If this node or any of its children can't be vectorized, try pruning
+     the tree here rather than felling the whole thing.  */
   if (!res && vect_slp_convert_to_external (vinfo, node, node_instance))
     {
       /* We'll need to revisit this for invariant costing and number
 	 of vectorized stmt setting.   */
-      lvisited.remove (node);
       res = true;
     }
 
-- 
2.26.2

[PATCH] tree-optimization/96028 - fix bogus externalizing of SLP node

[Richard Biener]

This guards externalizing a SLP node when it fails to code generate
to actually have scalar defs we can use.  It also makes failure
to do so not fell the whole SLP instance but instead try this again
on the parent.

Bootstrap / regtest running on x86_64-unknown-linux-gnu and should
fix an ICE observed on sparc-solaris.

2020-07-02  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/96028
	* tree-vect-slp.c (vect_slp_convert_to_external): Make sure
	we have scalar stmts to use.
	(vect_slp_analyze_node_operations): When analyzing a child
	failed try externalizing the parent node.
---
 gcc/tree-vect-slp.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 532809d2667..a471f89248c 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -2788,6 +2788,7 @@ vect_slp_convert_to_external (vec_info *vinfo, slp_tree node,
 
   if (!is_a <bb_vec_info> (vinfo)
       || node == SLP_INSTANCE_TREE (node_instance)
+      || !SLP_TREE_SCALAR_STMTS (node).exists ()
       || vect_contains_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (node)))
     return false;
 
@@ -2895,13 +2896,18 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node,
       || lvisited.add (node))
     return true;
 
+  bool res;
   FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)
-    if (!vect_slp_analyze_node_operations (vinfo, child, node_instance,
-					   visited, lvisited, cost_vec))
-      return false;
+    {
+      res = vect_slp_analyze_node_operations (vinfo, child, node_instance,
+					      visited, lvisited, cost_vec);
+      if (!res)
+	break;
+    }
 
-  bool res = vect_slp_analyze_node_operations_1 (vinfo, node, node_instance,
-						 cost_vec);
+  if (res)
+    res = vect_slp_analyze_node_operations_1 (vinfo, node, node_instance,
+					      cost_vec);
 
   /* When the node can be vectorized cost invariant nodes it references.
      This is not done in DFS order to allow the refering node
@@ -2943,8 +2949,8 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node,
 	  vect_prologue_cost_for_slp (child, cost_vec);
 	}
 
-  /* If this node can't be vectorized, try pruning the tree here rather
-     than felling the whole thing.  */
+  /* If this node or any of its children can't be vectorized, try pruning
+     the tree here rather than felling the whole thing.  */
   if (!res && vect_slp_convert_to_external (vinfo, node, node_instance))
     {
       /* We'll need to revisit this for invariant costing and number
-- 
2.26.2