On Wed, Jul 1, 2026 at 7:56 PM Richard Biener <[email protected]> wrote:
> Hmm, the testcase is a bit complicated since there's loop
> vectorization as well. I have tried the following alternative
> patch but it doesn't make a difference for me? But in essence
> what you are testing is whether both operand0 and operand1 appear
> unform but with different enough operation so that swapping would
> make it non-uniform. So I'd have expected the following to cover
> that case as well, but maybe I misunderstood. I think we want to
> avoid creating another "do stmts match" computation, re-using
> vect_build_slp_tree_1 would be more reasonable for this (we could
> think of refactoring that a bit, of course).
>
> That said, I do see how the heuristic can be bad, and peeking at
> operand1 looks reasonable.
>
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 2250f6f74a1..c529f9c3f11 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -3013,6 +3013,18 @@ out:
> if (j != 0 && !stmt_can_swap[j])
> can_swap_nonmatching = false;
> }
> + unsigned char *swapt = XALLOCAVEC (unsigned char, group_size);
> + poly_uint64 max_nunitst;
> + bool *matchest = XALLOCAVEC (bool, group_size);
> + bool two_operatorst = false;
> + tree vectypet = NULL_TREE;
> + if (can_swap_nonmatching
> + && vect_build_slp_tree_1 (vinfo, swapt,
> + oprnds_info[1]->def_stmts,
> + group_size, &max_nunitst,
> + matchest, &two_operatorst,
> + &vectypet))
> + can_swap_nonmatching = false;
> }
>
> old_swap_distance = least_upthread_swappable_op_distance;
The reason seems to be that vect_build_slp_tree_1 returns false for
operand1 here. The operand1 subtree also cannot be built as an
internal SLP node, because it hits the same kind of BIT_FIELD_REF
mismatch:
Build SLP for stmp_rsum00_88.79_303 =
BIT_FIELD_REF <vect__15.78_302, 64, 0>;
Build SLP for stmp_isum00_89.75_296 =
BIT_FIELD_REF <vect__18.74_295, 64, 0>;
Build SLP failed: different BIT_FIELD_REF arguments in
stmp_isum00_89.75_296 = BIT_FIELD_REF <vect__18.74_295, 64, 0>;
I agree that adding another small "do stmts match" check is a bit odd.
But I don't think making vect_build_slp_tree_1 return true here would
be good either,
and I haven't found a better way.
Maybe need a wider refactoring of the swap heuristic.
Do that "stmts match" in the initial swap decision instead of relying
on later retry.
--
Regards,
Zhongyao
