On Fri, 28 Nov 2025, Richard Biener wrote:
> 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).
So I'm stuck here without further clarification of why you think
that we need this_badness_inbranch? Is the original patch OK?
Thanks,
Richard.
> 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)
