On Thu, 27 Nov 2025, Jakub Jelinek wrote:
> On Thu, Nov 27, 2025 at 02:50:11PM +0100, Richard Biener wrote:
> > --- a/gcc/tree-vect-stmts.cc
> > +++ b/gcc/tree-vect-stmts.cc
> > @@ -4222,9 +4222,12 @@ vectorizable_simd_clone_call (vec_info *vinfo,
> > stmt_vec_info stmt_info,
> > poly_uint64 vf = loop_vinfo ? LOOP_VINFO_VECT_FACTOR (loop_vinfo) : 1;
> > unsigned group_size = SLP_TREE_LANES (slp_node);
> > unsigned int badness = 0;
> > + unsigned int badness_inbranch = 0;
> > struct cgraph_node *bestn = NULL;
> > + struct cgraph_node *bestn_inbranch = NULL;
> > if (!cost_vec)
> > - bestn = cgraph_node::get (simd_clone_info[0]);
> > + bestn = ((loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
> > + ? data.clone_inbranch : data.clone);
> > else
> > for (struct cgraph_node *n = node->simd_clones; n != NULL;
> > n = n->simdclone->next_clone)
> > @@ -4355,14 +4358,19 @@ vectorizable_simd_clone_call (vec_info *vinfo,
> > stmt_vec_info stmt_info,
> > SIMD_CLONE_ARG_TYPE_MASK);
> > /* Penalize using a masked SIMD clone in a non-masked loop, that is
> > not in a branch, as we'd have to construct an all-true mask. */
> > - if (!loop_vinfo || !LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
> > - this_badness += 64;
> > + this_badness += 64;
>
> Shouldn't there be also this_badness and this_badness_inbranch, the latter
> set to the former before the if containing this and so this affecting just
> badness and not badness_inbranch?
I didn't want to alter any costing with this patch. But now looking
the above seems to be oddly redundant with the earlier
if (n->simdclone->inbranch)
this_badness += 8192;
since with the patch we now try to find the best notinbranch and
the best inbranch clone (with the former eventually being inbranch
as well, if required by the call being a .MASK_CALL).
I don't think we need this_badness_inbranch, and having the general
penalty of inbranch clones should make sure that bestn is notinbranch
if possible?
The logic with the above hunk in the patch was that we're going to
use the inbranch clone only iff LOOP_VINFO_FULLY_MASKED_P, so it's
OK to always apply this penalty (if that very penalty there makes
sense at all, which I did not question when writing the patch).
Richard.
> > }
> > if (bestn == NULL || this_badness < badness)
> > {
> > bestn = n;
> > badness = this_badness;
> > }
> > + if (n->simdclone->inbranch
> > + && (bestn_inbranch == NULL || this_badness < badness_inbranch))
> > + {
> > + bestn_inbranch = n;
> > + badness_inbranch = this_badness;
> > + }
> > }
> >
>
> Jakub
>
>
--
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)
