Hi Robin,

> -----Original Message-----
> From: Robin Dapp <[email protected]>
> Sent: 23 September 2026 16:36
> To: Tamar Christina <[email protected]>; [email protected]
> Cc: nd <[email protected]>; [email protected]; Robin Dapp
> <[email protected]>
> Subject: Re: [patch 1/14]middle-end: add new vector control flow IFNs and
> optabs
> 
> Hi Tamar,
> 
> Glad to see this series :)
> 
> I'm still trying to wrap my head around the changes as well as the
> status before, and what we need for RVV in order to make proper use of
> it.

No worries, I'm not sure what good codegen or RVV looks like so feedback
is always appreciated.

> 
> Also, I'm getting the impression that this is the first time we could
> use two separate else values (one for the tail, one for the inactive
> elements).  But let's not go there now, I don't want to make people
> detest RVV even more :)
> 
> One main point I'd still like to raise, though.  If that has been
> discussed already, feel free to ignore:
> 
> Right now both the mask as well as the len path mask the comparison
> (where the m & cmp comes from that you want to optimize) and not the
> following "reduction" (= COUNT_ACTIVE) that feeds the branch.  For RVV
> that's surely not the best solution and I'd rather have a plain vector
> comparison, implicitly length masked, as input for
> LEN_COUNT_ACTIVE/LEN_MASK_INDEX_FIRST so that the m & cmp never
> even

Yeah, the idea atm is that the compare itself reduces to a known state so that
COUNT_ACTIVE has a more defined input and output.  I'll get back to this
later responding to another point.

> gets emitted.  For RVV that approach should always better, for SVE I'm
> not so sure.  I could imagine that the predicate handling for ncopies >
> 1 gets more expensive with that suggestion.
> 
> Would masking COUNT_ACTIVE work for SVE, at least for the ncopies = 1
> case or am I missing something fundamental?  Or maybe it just doesn't
> scale beyond ncopies = 1?  But wouldn't some of the extra work we need
> to do (like the TER and duplicate use) in this series vanish if that
> were a viable approach?
> 

COUNT_ACTIVE doesn't work today when ncopies > 1 because the exits are
reduced using exclusive OR.  The vectorizer has a limitation that we can't
introduce new cfg on the early exits. So with ncopies > 1 we know one element
is active but we don't know which one.

The thing Artemiy is working on is not using an ORR reduction in such cases,
and instead we reduce each copy individually so we know which exit was taken.
We'd still not need COUNT_ACTIVE to take an additional value however since
The IV computation code will deal with the offset as part of the PHI node with
the vector "scalar" IV.

So at the moment I'm having a hard time with why you'd want this, and what the
benefit would be vs normal combine patterns.  For instance even with SVE we
transform x + COUNT_ACTIVE further into e.g. "INC_ACTIVE" in RTL..

> One way or another, I was just curious and, of course, I'm not asking
> you to change the RVV code path.  I'll do that myself it there's still
> time during stage 1.
> 

The patch explicitly doesn't deal with ncopies > 1, along with reductions
and stores since this grew things considerably.  To support them I think we
need to do more during materialization.

> So to my understanding, right now with trapping math excluded, we don't
> need a masked vec_cmp for RVV because we could just compare without
> length, ignoring tail elements, then popcount the result and branch
> dependent on it.  With trapping math the cond_len variant is needed but
> won't be optimized anyway.

Not entirely, it's also needed for partial loops where the mask is required so
we don't access or store elements beyond what e loaded.  This is particularly
important for targets where the else value of a load is not zero. 

Another reason I added the masked vec_cmp is because at least for SVE
compares can set flags.  And because of that we introduced in GCC 15
vec_cbranch_{any/all} which allow a target to distinguish between when the
compare is setting a flag or when the non-flags results of the compare is 
needed.

By doing cmp & mask, we rely on combine to the compare into a masked compare,
which is great but then CSE cannot eliminate duplicate compares since there's
no CSE after combine.

So cond_vec_cmp is intended to not rely on combine to push in the masks, and as
a side-effect also allows the expand patterns to be much simpler than before.

I have chosen to do this quite late instead of through the vectorizer because
passes such as cunroll can deal with & on VLA compares, but wouldn't be able
to deal with the new IFN. i.e. we'd lose many GIMPLE optimizations by 
transforming
it too early.

Do you have an example of why you'd want to do the emission so early?

> 
> Apart from that, I have mostly bikeshedding to contribute, with the RVV
> usage in mind.
> 
> > +@cindex @code{IFN_COUNT_ACTIVE} internal function
> > +@item @samp{IFN_COUNT_ACTIVE}
> > +The @code{IFN_COUNT_ACTIVE} internal function directly maps to the
> > +@code{count_active} instruction pattern.  It returns the number of active
> > +elements in its argument.
> > +
> > +@cindex @code{IFN_COND_COUNT_ACTIVE} internal function
> > +@item @samp{IFN_COND_COUNT_ACTIVE}
> > +The @code{IFN_COND_COUNT_ACTIVE} internal function directly maps to
> the
> > +@code{cond_count_active} instruction pattern.  It returns the number of
> > +active elements in its second argument under the mask in its first
> > +argument.
> > +
> > +@cindex @code{IFN_COND_LEN_COUNT_ACTIVE} internal function
> > +@item @samp{IFN_COND_LEN_COUNT_ACTIVE}
> > +The @code{IFN_COND_LEN_COUNT_ACTIVE} internal function directly
> maps to the
> > +@code{cond_len_count_active} instruction pattern.  It is like
> > +@code{IFN_COND_COUNT_ACTIVE}, but also ignores elements whose
> index is
> > +greater than or equal to the sum of the length and bias arguments.
> > +
> > +@cindex @code{IFN_INDEX_FIRST_ACTIVE} internal function
> > +@item @samp{IFN_INDEX_FIRST_ACTIVE}
> > +The @code{IFN_INDEX_FIRST_ACTIVE} internal function directly maps to
> the
> > +@code{index_first_active} instruction pattern.  It returns the index of the
> > +first active element in its argument.  The result is undefined if the
> > +predicate has no active elements.
> 
> Could we define the result as -1?  RVV's vfirst returns -1 if no bits
> are set and that would help to rely on it.
> 

No, since the undefined is to deal with targes which don't have a native 
instruction
for this optimally. For instance adv. Simd does not have a single instruction to
do so, and instead relies on CTZ, which is undefined for 0.  So having to force 
it to
-1 requires such targets to artificially have additional instructions they 
could never
optimize away.

The vectorizer however knows that this case can never happen, you won't take the
branch on an early exit if not at least one element is active, and this 
information allows
for more optimal codegen.  If we are to assume -1 then the IFN isn't needed and
targets can just implement TAKE_EXCL + COUNT_ACTIVE.

> 
> I know I mentioned naming before, sorry, but I got confused about
> "active" again.  The functions here refer to the set bits in a
> vector/mask as active while there can be fewer active lanes than set
> bits.  So arguably, we should rather call them INDEX_FIRST_SET etc. but
> I'm not going to ask you change everything unless there's consensus.

I'm explicitly avoiding talking about bits set rather than lanes, because for
SVE bits set do not indicate a lane being active. SVE as an architecture says
that only the bottom bit in a mask is checked. And so on e.g. a vector
boolean mask for a .h instruction (i.e. short elements) both `0x10` and `0x00`
mean the lane is inactive.  That's why none of these optabs or instructions
talk about bits or sets. They just reason about lanes.

> 
> The one I mentioned before, EXTRACT_FIRST_ACTIVE is still ok I guess,
> because it actually extracts an active lane.  But the IFNs operating on
> predicates don't.  I just realized RVV's insns already follow the
> set-naming (like vmsbf vector mask set before first).  Believe it or
> not, that's not where I'm coming from :)
> 

Yeah, I was trying to avoid associations with a bit pattern, due to the
above.  I also think as an abstraction, active lanes map better to SLP.

But happy to change If I'm in the minority :)

> Usually we add len ifns (as well as mask ifns) to internal_fn_len_index
> and internal_fn_mask_index but as we can't re-use
> add_mask_else_and_len_args due to optab operand order, it won't help
> anyway.  And I really hope we can untangle all of this eventually when
> we have an RTL vector predicate.
> 

One Issue I've faced is that genuinely many of the CFG control IFN don't
work with an else other than zero.  That leads to an unfortunate number
of churn to use them since as you say I can't use the existing helpers.

This is becoming a bigger issue as we keep adding CFG support and needs
more looking at.  I thought to bring it up at Cauldron.

> > +
> > +@cindex @code{IFN_COND_INDEX_FIRST_ACTIVE} internal function
> > +@item @samp{IFN_COND_INDEX_FIRST_ACTIVE}
> > +The @code{IFN_COND_INDEX_FIRST_ACTIVE} internal function directly
> maps to the
> > +@code{cond_index_first_active} instruction pattern.  It returns the index 
> > of
> > +the first active element in its second argument under the mask in its first
> > +argument.  The result is undefined if the selected predicate has no active
> > +elements.
> 
> Maybe a short note here that these IFNs take a predicate/mask type as
> input?

Will do! That information is with the optabs, I struggled to figure out what to
put where.  Historically we've only documented the optabs but recently there's
been appetite to document the IFNs too.  But this introduces this duplicates in
information.

Thanks for the feedback!

Tamar.
> 
> --
> Regards
>  Robin

Reply via email to