On Tue, 7 Jul 2026, Zhongyao Chen wrote:
> On Fri, Jul 3, 2026 at 7:58 PM Richard Biener <[email protected]> wrote:
> >
> > Ah, OK. So the issue is that without swapping we can build
> > the operand from scalars because we "late" fail here, but
> > after swapping we fail early during vect_build_slp_tree_1
> > and in that case we never build the operand from scalars.
> > Instead we now build the swapped parent operand from scalars.
> >
> > It seems that we might instead want change the original patch
> > so that we preserve the build from scalars when we build
> > operand zero but, if we could have swapped and the just
> > built child is external (aka built from scalars), we make
> > sure matches[] is still correctly populated so we can
> > try vect_build_slp_tree_1 on the swapped stmt set to see
> > if we'd wreck and only when not discard the extern promoted
> > child and try with swapping?
> >
> > That would basically implement the original change in a different,
> > more local way.
> >
> > Richard.
Sorry for the delay responding ...
> I tried the "preserve the build from scalars" direction, but it gets a
> bit complicated since I have to keep/restore the failed child state.
>
> So I tried another way, which close to your earlier patch.
>
> probe the swapped stmts with vect_build_slp_tree_1,
> if that fails again, then skip swap and allow child external directly.
>
> Does this look OK ?
Hmm, so this probes ahead that we can likely successfully build
the swapped operand zero.
Of course the first vect_build_slp_tree_1 is redundant (we'll
do it again during vect_build_slp_tree before swapping).
So what I had in mind was sth like (just a quick sketch, not even
compile tested):
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index ff11b392305..c1d82a8907e 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -3018,17 +3018,22 @@ out:
}
}
- old_swap_distance = least_upthread_swappable_op_distance;
- if (can_swap_nonmatching)
- least_upthread_swappable_op_distance = 1;
- else if (least_upthread_swappable_op_distance != -1U)
- least_upthread_swappable_op_distance++;
child = vect_build_slp_tree (vinfo, oprnd_info->def_stmts,
&this_max_nunits,
matches, limit,
&this_tree_size, bst_map);
- least_upthread_swappable_op_distance = old_swap_distance;
- if (child != NULL)
+
+ slp_tree saved_child = NULL;
+ if (child
+ && is_a <bb_vec_info> (vinfo)
+ && SLP_TREE_DEF_TYPE (child) == vect_external_def
+ && oprnd_info->first_dt != vect_external_def
+ && oprnd_info->first_dt != vect_constant_def
+ && can_swap_nonmatching)
+ /* Check if matches[] from above is still populated correctly.
+ I think it is. */
+ saved_child = child;
+ else if (child != NULL)
{
oprnd_info->def_stmts = vNULL;
children.safe_push (child);
@@ -3094,6 +3099,14 @@ out:
children.safe_push (child);
continue;
}
+ if (saved_child)
+ {
+ /* Now un-swap stmts. matches[] should be still available
+ from above. Important so we get operand 1 back. */
+ oprnd_info->def_stmts = vNULL;
+ children.safe_push (saved_child);
+ continue;
+ }
}
fail:
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 2250f6f74a1..b60c53cd5d2 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -3013,6 +3013,51 @@ out:
> if (j != 0 && !stmt_can_swap[j])
> can_swap_nonmatching = false;
> }
> +
> + if (can_swap_nonmatching)
> + {
> + /* Keep the delayed swap retry only if the swapped immediate
> + operand-zero node still matches at this level. */
> + unsigned char *probe_swap = XALLOCAVEC (unsigned char,
> group_size);
> + bool *probe_matches = XALLOCAVEC (bool, group_size);
> + bool probe_two_operators = false;
> + tree probe_vectype = NULL_TREE;
> + poly_uint64 probe_max_nunits = this_max_nunits;
> + bool op0_matches
> + = vect_build_slp_tree_1 (vinfo, probe_swap,
> + oprnds_info[0]->def_stmts,
> + group_size, &probe_max_nunits,
> + probe_matches, &probe_two_operators,
> + &probe_vectype);
> + if (!op0_matches && !probe_matches[0])
> + can_swap_nonmatching = false;
> +
> + if (can_swap_nonmatching)
> + {
> + vec<stmt_vec_info> probe_stmts = vNULL;
> + probe_stmts.create (group_size);
> + for (j = 0; j < group_size; ++j)
> + {
> + bool swap_lane
> + = (op0_matches
> + ? j != 0
> + : !probe_matches[j]);
> + probe_stmts.quick_push (swap_lane
> + ? oprnds_info[1]->def_stmts[j]
> + : oprnds_info[0]->def_stmts[j]);
> + }
> +
> + probe_two_operators = false;
> + probe_vectype = NULL_TREE;
> + probe_max_nunits = this_max_nunits;
> + if (!vect_build_slp_tree_1 (vinfo, probe_swap,
> + probe_stmts, group_size,
> + &probe_max_nunits,
> probe_matches,
> + &probe_two_operators,
> &probe_vectype))
> + can_swap_nonmatching = false;
> + probe_stmts.release ();
> + }
> + }
> }
>
> old_swap_distance = least_upthread_swappable_op_distance;
>
>
>
> --
> Regards,
> Zhongyao
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald; (HRB 36809, AG Nuernberg)
