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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
