When building operand zero of a commutative BB SLP node, a failed child
discovery may build operands from scalars right away. That hides the failure
from the existing retry path, even when retrying with swapped operands could
still fix the current node.
Use a boolean to let operand-zero subtree discovery know that the current node
may still be fixed by the swap retry. In that case, skip building operands
from scalars on failure and reach the retry path first.
My local tests show no regression for vect.exp, only a few for rvv.exp,
but those are reasonable, just need update test expectations.
PR tree-optimization/125567
gcc/
* tree-vect-slp.cc (vect_slp_commutative_retry_depth): New.
(vect_build_slp_tree_2): Compute swap checks before building operand
zero. Skip external fallback while swap retry is possible. Reuse the
swap checks in the retry path.
gcc/testsuite/
* gcc.dg/vect/pr125567.c: New test.
Signed-off-by: Zhongyao Chen <[email protected]>
---
gcc/testsuite/gcc.dg/vect/pr125567.c | 40 ++++++++++
gcc/tree-vect-slp.cc | 113 +++++++++++++++++----------
2 files changed, 112 insertions(+), 41 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vect/pr125567.c
diff --git a/gcc/testsuite/gcc.dg/vect/pr125567.c
b/gcc/testsuite/gcc.dg/vect/pr125567.c
new file mode 100644
index 00000000000..6bc2268a1a9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr125567.c
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_int } */
+/* { dg-additional-options "-O3 -ftree-vectorize -fdump-tree-slp1-details" } */
+
+typedef short i16;
+
+void
+foo (i16 *__restrict out, i16 *__restrict in, i16 x, unsigned n)
+{
+ unsigned k = 0;
+ i16 tmp = x + 3;
+
+ while (k < n)
+ {
+ tmp = tmp ^ 0x3f;
+
+ out[k + 0] = in[k + 0] + tmp;
+ out[k + 1] = in[k + 1] + tmp;
+ out[k + 2] = in[k + 2] + tmp;
+ out[k + 3] = in[k + 3] + tmp;
+ out[k + 4] = in[k + 4] + tmp;
+ out[k + 5] = in[k + 5] + tmp;
+ out[k + 6] = in[k + 6] + tmp;
+ out[k + 7] = in[k + 7] + tmp;
+ out[k + 8] = in[k + 8] + tmp;
+ out[k + 9] = in[k + 9] + tmp;
+ out[k + 10] = in[k + 10] + tmp;
+ out[k + 11] = in[k + 11] + tmp;
+ out[k + 12] = in[k + 12] + tmp;
+ out[k + 13] = in[k + 13] + tmp;
+ out[k + 14] = in[k + 14] + tmp;
+ out[k + 15] = in[k + 15] + tmp;
+
+ k += 16;
+ }
+}
+
+/* { dg-final { scan-tree-dump "Re-trying with swapped operands of stmts"
"slp1" } } */
+/* { dg-final { scan-tree-dump "Using a splat of the uniform operand" "slp1" }
} */
+/* { dg-final { scan-tree-dump "optimized: basic block part vectorized" "slp1"
} } */
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 075e93f04a9..9e1ffaba796 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -1873,6 +1873,12 @@ vect_slp_linearize_chain (vec_info *vinfo,
}
}
+/* Use this flag to let operand-zero subtree discovery know that the
+ current node may still be fixed by retrying with swapped operands of the
+ immediate parent, so it can avoid building operands from scalars on
failure. */
+
+static bool vect_slp_swappable_op_upthread;
+
static slp_tree
vect_build_slp_tree_2 (vec_info *vinfo, slp_tree node,
vec<stmt_vec_info> stmts, unsigned int group_size,
@@ -2815,6 +2821,10 @@ out:
{
slp_tree child = nullptr;
unsigned int j;
+ bool old_swappable;
+ bool can_swap;
+ bool can_swap_nonmatching;
+ bool *stmt_can_swap;
/* We're skipping certain operands from processing, for example
outer loop reduction initial defs. */
@@ -2965,10 +2975,54 @@ out:
def_stmts2.release ();
}
- if ((child = vect_build_slp_tree (vinfo, oprnd_info->def_stmts,
- group_size, &this_max_nunits,
- matches, limit,
- &this_tree_size, bst_map)) != NULL)
+ can_swap = (i == 0
+ && (nops == 2 || nops == 3)
+ && oprnds_info.length () > 1
+ && oprnds_info[1]->first_dt == vect_internal_def
+ && (is_gimple_assign (stmt_info->stmt)
+ || is_gimple_call (stmt_info->stmt))
+ /* Swapping operands for reductions breaks assumptions
+ later on. */
+ && STMT_VINFO_REDUC_IDX (stmt_info) == -1);
+ can_swap_nonmatching = can_swap;
+ stmt_can_swap = NULL;
+ if (can_swap)
+ {
+ stmt_can_swap = XALLOCAVEC (bool, group_size);
+ for (j = 0; j < group_size; ++j)
+ {
+ stmt_can_swap[j] = false;
+ if (!stmts[j])
+ /* NULL lanes are gaps and have no stmt to swap. */
+ stmt_can_swap[j] = true;
+ else if (gassign *stmt = dyn_cast <gassign *> (stmts[j]->stmt))
+ {
+ tree_code code = gimple_assign_rhs_code (stmt);
+ stmt_can_swap[j] = (commutative_tree_code (code)
+ || commutative_ternary_tree_code (code));
+ }
+ else if (gcall *call = dyn_cast <gcall *> (stmts[j]->stmt))
+ {
+ internal_fn fn = (gimple_call_internal_p (call)
+ ? gimple_call_internal_fn (call) :
IFN_LAST);
+ stmt_can_swap[j] = ((commutative_binary_fn_p (fn)
+ || commutative_ternary_fn_p (fn))
+ && first_commutative_argument (fn) == 0);
+ }
+
+ if (j != 0 && !stmt_can_swap[j])
+ can_swap_nonmatching = false;
+ }
+ }
+
+ old_swappable = vect_slp_swappable_op_upthread;
+ vect_slp_swappable_op_upthread = can_swap_nonmatching;
+ child = vect_build_slp_tree (vinfo, oprnd_info->def_stmts,
+ group_size, &this_max_nunits,
+ matches, limit,
+ &this_tree_size, bst_map);
+ vect_slp_swappable_op_upthread = old_swappable;
+ if (child != NULL)
{
oprnd_info->def_stmts = vNULL;
children.safe_push (child);
@@ -2978,17 +3032,9 @@ out:
/* If the SLP build for operand zero failed and operand zero
and one can be commuted try that for the scalar stmts
that failed the match. */
- if (i == 0
- /* A first scalar stmt mismatch signals a fatal mismatch. */
- && matches[0]
- /* ??? For COND_EXPRs we can swap the comparison operands
- as well as the arms under some constraints. */
- && (nops == 2 || nops == 3)
- && oprnds_info[1]->first_dt == vect_internal_def
- && (is_gimple_assign (stmt_info->stmt)
- || is_gimple_call (stmt_info->stmt))
- /* Swapping operands for reductions breaks assumptions later on. */
- && STMT_VINFO_REDUC_IDX (stmt_info) == -1)
+ if (/* A first scalar stmt mismatch signals a fatal mismatch. */
+ matches[0]
+ && can_swap)
{
/* See whether we can swap the matching or the non-matching
stmt operands. */
@@ -2999,34 +3045,13 @@ out:
{
if (matches[j] != !swap_not_matching)
continue;
- stmt_vec_info stmt_info = stmts[j];
/* Verify if we can swap operands of this stmt. */
- if (gassign *stmt = dyn_cast <gassign *> (stmt_info->stmt))
- {
- tree_code code = gimple_assign_rhs_code (stmt);
- if (! commutative_tree_code (code)
- && ! commutative_ternary_tree_code (code))
- {
- if (!swap_not_matching)
- goto fail;
- swap_not_matching = false;
- break;
- }
- }
- else if (gcall *call = dyn_cast <gcall *> (stmt_info->stmt))
+ if (!stmt_can_swap[j])
{
- internal_fn fn = (gimple_call_internal_p (call)
- ? gimple_call_internal_fn (call)
- : IFN_LAST);
- if ((! commutative_binary_fn_p (fn)
- && ! commutative_ternary_fn_p (fn))
- || first_commutative_argument (fn) != 0)
- {
- if (!swap_not_matching)
- goto fail;
- swap_not_matching = false;
- break;
- }
+ if (!swap_not_matching)
+ goto fail;
+ swap_not_matching = false;
+ break;
}
}
}
@@ -3078,6 +3103,12 @@ fail:
/* ??? Rejecting patterns this way doesn't work. We'd have to
do extra work to cancel the pattern so the uses see the
scalar version. */
+ /* Skip building vector operands from scalars while operand
+ discovery may still be fixed by retrying with swapped operands. */
+ && (!vect_slp_swappable_op_upthread
+ /* A first scalar stmt mismatch signals a fatal mismatch
+ that the parent commutative retry cannot recover. */
+ || !matches[0])
&& !is_pattern_stmt_p (stmt_info)
&& !oprnd_info->any_pattern)
{
--
2.43.0
