> 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..
IMHO the current RVV or length-based approach for early break has it
backwards. VCOND_MASK_LEN is a vehicle to convert a length into a mask
with built-in zeroing of tail lanes. In my opinion that's not necessary
and we should be able to work with the length directly. Right now, not
doing this limits our early-break usage unnecessarily. Conceptually,
VCOND_MASK_LEN cannot be fully fixed because of RVV ISA restrictions.
Today, we already combine (popcount (VCOND_MASK_LEN)) into
"len_popcount" (under the restrictions above) so I'm suggesting to emit
the latter directly in order to avoid the restrictions.
> 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.
I think what I'm proposing for RVV would work for ncopies > 1 in the
same way, reducing every copy individually. But I haven't implemented
it yet.
>> 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.
Here I'd disagree to some degree and instead argue that it's actually
not needed. In partial vectorization we mask load and store and,
normally, let the rest of the loop operate at full length. Exceptions
to this rule are tail-observing operations like reductions, trapping
ops, etc. Those we always mask.
Now, for early break I believe we have the choice where or what to mask:
(1) The compare itself.
(2) The "reduction" (COUNT_ACTIVE) after the compare.
(3) The "branch".
For SVE (1) and (2) are the same instruction (and the same operation in
gimple because flags aren't exposed) and we can't even choose what to
mask. For RVV all three are separate insns and we have more freedom.
Now if we mask the reduction (2), there is no need for zeroing because
it will only look at the active values and the tail stays unobserved.
That way, RVV early-break handling wouldn't need to jump through as many
hoops as the SVE approach.
The current approach of masking (1) (or maybe 1 and 2) makes sense for
SVE because the operations cannot be separated anyway. Or maybe they
could, synthetically, but then we'd have a costly re-test. Also,
zeroing is free.
> 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?
Ideal RVV code for ncopies = 1 would look like:
# ...
# loop body
vmslt v0, v2, v3 # v0 = v2 < v3, unmasked
vfirst a0, v0 # COND_LEN_INDEX_FIRST_ACTIVE, masked
bltz a0 # branch if < 0
# scalar "tail" for the mismatched element
# ...
For ncopies > 1 we'd have/want ncopies * vfirst, so one per copy.
Probably the same direction Artemiy is heading?
Out of interest and as a future question, not for this series: All the
mask duplication, TER, optabs, isel changes are a churn as well and feel
very tailored to just a few early-break operations. What would need
changing to handle this more gracefully? You mentioned RTL
representation issues with flags, that should generally be solvable?
I also think we can do more in the gimple pipeline by stripping cond/len
and reassembling after simplification.
>> 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.
And presumably for AdvSIMD the -1 sequence would be too expensive? But
it's in the exit block, right? Anyway, then I suppose I need to define
a separate IFN for RVV. Being able to rely on -1 makes things much
easier. We can discuss this once I have the rest in place.
>> 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.
I think that might also be a consequence of the whole approach being
"mask focused". Under my RVV interpretation they would be "reductions"
(i.e. observers of tail elements) and must be explicitly masked
themselves. See my response to 7/14.
--
Regards
Robin
