On Tue, 23 Jun 2026, Zhongyao Chen wrote: > For commutative BB SLP nodes, don't retry a swap on a lane if doing so > would turn the first-lane operand columns into a mixed child shape. > > PR125800 is exactly this case, the first lane already has ADD and > BIT_FIELD_REF in separate columns, and swapping a parent there breaks > that and externalizes more nodes. > > The new testcase restricts exact dump checks to non-partial-vector targets, > since those targets may lower the reduction via fold-left and produce > different dump output.
I'm trying to understand from the above and the patch, but fail. What's the set of stmts we avoid to externalize because we can swap the (immediate) parent? As said in the PR for the initial patch for the greedy search we perform any heuristic will have downsides and for BB vectorization swapping is as good as not swapping if operand zero discovery ends up with an externalized node. Thanks, Richard. > PR tree-optimization/125800 > > gcc/ > * tree-vect-slp.cc (vect_build_slp_tree_2): Skip stmt_can_swap > for lanes that would create a mixed child shape on swap retry. > > gcc/testsuite/ > * gcc.dg/vect/pr125800.c: New test. > > Signed-off-by: Zhongyao Chen <[email protected]> > --- > gcc/testsuite/gcc.dg/vect/pr125800.c | 51 ++++++++++++++++++++++++++++ > gcc/tree-vect-slp.cc | 33 ++++++++++++++++++ > 2 files changed, 84 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/vect/pr125800.c > > diff --git a/gcc/testsuite/gcc.dg/vect/pr125800.c > b/gcc/testsuite/gcc.dg/vect/pr125800.c > new file mode 100644 > index 00000000000..39cffc73c3f > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/pr125800.c > @@ -0,0 +1,51 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target vect_double } */ > +/* { dg-additional-options "-O2 -ftree-vectorize -fno-vect-cost-model > -fdump-tree-slp1-details" } */ > + > +void > +pr125800 (double *__restrict colY0, double *__restrict colY1, > + const double *__restrict colA0, const double *__restrict colA1, > + const double *__restrict colX0, const double *__restrict colX1, > + int ncolA) > +{ > + double rsum00 = 0, isum00 = 0, rsum01 = 0, isum01 = 0; > + double rsum10 = 0, isum10 = 0, rsum11 = 0, isum11 = 0; > + > + for (int kcolA = 0; kcolA < ncolA; kcolA++) > + { > + int raloc = 2 * kcolA; > + int ialoc = raloc + 1; > + double ar0 = colA0[raloc]; > + double ai0 = colA0[ialoc]; > + double ar1 = colA1[raloc]; > + double ai1 = colA1[ialoc]; > + > + int rxloc = 2 * kcolA; > + int ixloc = rxloc + 1; > + double xr0 = colX0[rxloc]; > + double xi0 = colX0[ixloc]; > + double xr1 = colX1[rxloc]; > + double xi1 = colX1[ixloc]; > + > + rsum00 += ar0 * xr0 - ai0 * xi0; > + isum00 += ar0 * xi0 + ai0 * xr0; > + rsum01 += ar0 * xr1 - ai0 * xi1; > + isum01 += ar0 * xi1 + ai0 * xr1; > + rsum10 += ar1 * xr0 - ai1 * xi0; > + isum10 += ar1 * xi0 + ai1 * xr0; > + rsum11 += ar1 * xr1 - ai1 * xi1; > + isum11 += ar1 * xi1 + ai1 * xr1; > + } > + > + colY0[0] -= rsum00; > + colY0[1] -= isum00; > + colY1[0] -= rsum01; > + colY1[1] -= isum01; > + colY0[2] -= rsum10; > + colY0[3] -= isum10; > + colY1[2] -= rsum11; > + colY1[3] -= isum11; > +} > + > +/* { dg-final { scan-tree-dump "vectorizing stmts using SLP" "slp1" } } */ > +/* { dg-final { scan-tree-dump "SLP size 19" "slp1" { target { ! > vect_partial_vectors } } } } */ > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc > index 2250f6f74a1..8a776ee01f0 100644 > --- a/gcc/tree-vect-slp.cc > +++ b/gcc/tree-vect-slp.cc > @@ -2988,6 +2988,14 @@ out: > stmt_can_swap = NULL; > if (can_swap) > { > + gassign *first_op0 > + = dyn_cast <gassign *> (oprnds_info[0]->def_stmts[0]->stmt); > + gassign *first_op1 > + = dyn_cast <gassign *> (oprnds_info[1]->def_stmts[0]->stmt); > + tree_code first_op0_code > + = first_op0 ? gimple_assign_rhs_code (first_op0) : ERROR_MARK; > + tree_code first_op1_code > + = first_op1 ? gimple_assign_rhs_code (first_op1) : ERROR_MARK; > stmt_can_swap = XALLOCAVEC (bool, group_size); > for (j = 0; j < group_size; ++j) > { > @@ -3010,6 +3018,31 @@ out: > && first_commutative_argument (fn) == 0); > } > > + if (j != 0 > + && stmt_can_swap[j] > + && first_op0 > + && first_op1 > + && first_op0_code != first_op1_code) > + { > + gassign *lane_op0 > + = oprnds_info[0]->def_stmts[j] > + ? dyn_cast <gassign *> > + (oprnds_info[0]->def_stmts[j]->stmt) > + : nullptr; > + gassign *lane_op1 > + = oprnds_info[1]->def_stmts[j] > + ? dyn_cast <gassign *> > + (oprnds_info[1]->def_stmts[j]->stmt) > + : nullptr; > + /* Check column alignment by tree_code. */ > + if (lane_op0 > + && lane_op1 > + && gimple_assign_rhs_code (lane_op0) == first_op0_code > + && gimple_assign_rhs_code (lane_op1) == first_op1_code) > + /* Swap would create a mixed child shape. */ > + stmt_can_swap[j] = false; > + } > + > if (j != 0 && !stmt_can_swap[j]) > can_swap_nonmatching = false; > } > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
