| Message ID | 20200925142816.GA14929@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | middle-end Add support for SLP vectorization of complex number instructions. | expand |
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. > >
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. > > --
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)