> Ok, but why would you want to use COUNT_ACTIVE which is there for
> non-masked and non-len targets instead of COND_LEN_COUNT_ACTIVE
> which does the exact thing you mentioned above?
I'm reading some frustration in your message about being misunderstood
and me not having read/grasped all the proposed concepts. Point taken,
apologies :) I don't think I misunderstood entirely, though, even if I
mixed some things up.
(I meant COND_LEN_COUNT_ACTIVE, I didn't want to use COUNT_ACTIVE for
RVV here.)
>> 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.
>
> That's not quite true. SVE compares are side-effectful in more than just fp
> exceptions. They also set CC flags. And it's the reason the example code
> in the cover letter has
>
> cmphi p6.b, p7/z, z31.b, #5
> b.none .L8
> splice z31.b, p6, z31.b, z31.b
> fmov w0, s31
> ret
>
> notice the branch after the compare, because the compares themselves
> set the zero flag. Without masking, this would never be correct, this is
> why SVE does not have "unpredicated" compares. Because GIMPLE does
> not carry this information, the "cbranch" today re-sets the flags using a
> PTEST.
Yes, I should have clarified that I wasn't talking about switching SVE
to masking (2), but just RVV. Hence my question later regarding SVE and
what would be missing at the various passes to make an earlier IFN
emission possible. But as mentioned, that was a future-facing question.
> Also in SVE (1) and (2) are not the same instruction as you say.
> COUNT_ACTIVE
> is an mask reduction, And SVE would use COND_COUNT_ACTIVE because the
> SVE instruction CNTP requires both the loop mask and the result mask [1]
> This is used in the cases where
>
> If (<condition>)
> return i;
>
> Or another computation involving the loop invariant. So (1) and (2) are
> unrelated.
> The codegen here is
>
> cmp_mask = cmp & loop_mask
> if (cmp_mask != 0)
> {
> partial_mask = . COND_TAKE_EXCL_ACTIVE (loop_mask, cmp_mask);
> index = .COND_COUNT_ACTIVE (loop_mask, partial_mask);
> return x + index;
> }
>
> where x is the scalar IV the vectorizer is maintaining. COUNT_ACTIVE* and
> friends
> are about manipulating the loop IV. Both here and in Alfie's patches.
I think I was confusing flag-setting and predicate-producing in the
context of the branch. So yes, that (1) (2) (3) was more misleading
than helpful.
> Now masking (3) is not possible, because Richi has stated repeatedly in the
> past
> not to want an IFN within a cbranch in GIMPLE. i.e. VECTOR_BOOLEAN semantics
> on comparisons are the same for every target in GIMPLE.
Yep, I didn't want to argue for masking (3), just show that the
intermediate reduction is explicit in RVV and can be masked right away
to simplify things (for RVV).
> This was the point of cond_[len]_vec_cbranch_{any/all}, at transition out of
> GIMPLE
> it would explicitly mask and len your branch. So I never understood why you
> said it
> wasn't useful for RVV, you're arguing for the exact semantics here :)
The reason they were not useful for RVV is not their semantics
(they are fine) but how we get there. For RVV we'd combine
VCOND_MASK_LEN with the vector cbranch but that's already too late
because VCOND_MASK_LEN has converted the len to a mask with the
restriction that comes with it. VCOND_MASK_LEN is the issue, nothing
else, but in the current scheme I don't see a way of doing anything
else. That's why I'm suggesting what I'm suggesting.
> This is again the idea behind the new compare IFNs, to make it so that by the
> time
> You leave GIMPLE all early break control statements have an explicit len.
Yeah, here again, their semantics are fine, but the way we get to them
is similar to before. Instead, we need a length-controlled mask
reduction (be it vfirst or vcpop) right away and I don't think we'd be
hampered by the same issues SVE is (needing to duplicate for single use
etc.) if we'd have that. I'm not saying there won't be any issues, but
the status quo for RVV is not without issues either.
>> 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
>> # ...
>>
>
> That's not what the optabs are supposed to be or do,
> COND_LEN_INDEX_FIRST_ACTIVE
> is an IV extraction optab.
Yes, but the semantics are very close IMHO. With the -1 adjustment it
could do what I intended (see below), so I don't consider the suggestion
too outrageous :) I understand you rather not want to go there for the
reasons you stated and the -1 is useless for the way you anticipate
COND_LEN_INDEX_FIRST_ACTIVE to be used. So let's just define another one
for the RVV use?
>
> For instance
>
> If (<condition>)
> return i;
>
> gets transformed into for SVE
>
> cmp_mask = cmp & loop_mask
> if (cmp_mask != 0)
> return x + .COND_INDEX_FIRST_ACTIVE (loop_mask, cmp_mask);
>
> In SVE2 this maps to FIRSTP,
>
> https://support.arm.com/documentation/ddi0487/mb/-Part-C-The-AArch64-Instruction-Set/-Chapter-C8-SVE-Instruction-Descriptions/-C8-2-Alphabetical-list-of-SVE-instructions/-C8-2-192-FIRSTP
>
> and COND_INDEX_FIRST_ACTIVE is an optimization over using
>
> partial_mask = . COND_TAKE_EXCL_ACTIVE (loop_mask, cmp_mask);
> index = .COND_COUNT_ACTIVE (loop_mask, partial_mask);
>
> And this is the exact reason that COND_INDEX_FIRST_ACTIVE is undefined
> when the cmp_mask is empty, vs the combination of TAKE_EXCL + COUNT_ACTIVE
> which is
> defined to be num_lanes.
Yeah, firstp and vfirst are almost identical. For RVV the index of the
first active element (and -1 if there is none) can double as both input
for the branch as well as the scalar index for the exit block.
Of course COND_LEN_COUNT_ACTIVE works as well for the branch, but then
we'd need an extra "reduction" vfirst = IFN_OTHER_NAME :) in the exit
block when we could have gotten the value in the loop already. I
understand that it can never be -1 under your proposed scheme _in_ the
exit block because it's the very condition to not reach the exit block.
(IFN_COND_LEN_OTHER_NAME of course...)
> . COND_INDEX_FIRST_ACTIVE and friends are an optimization that can only be
> applied
> by the vectorizer which generates the branch. The vectorizer knows that
> because the
> result is used within an early exit edge, that if that branch is taken the
> mask would never
> but empty. You cannot do this in RTL (well not easily, without a new RTL
> pass) because
> you'd have to walk the loop and identify which exit is an early exit and
> which is the main
> exit. Something you cannot tell anymore after the vectorizer, since the
> vectorizer can pick
> a main exit *other* than the natural loop latch. That's the
> EARLY_BREAK_PEELED cases
> the code deals with.
>
> And because operations that map to COND_INDEX_FIRST_ACTIVE typically define
> the empty
> case to be -1, this also means in RTL you can't blindly combine the mask
> partition + count operation.
>
[..]
> The last 3 operations are dead. The csel condition would never be
> true. But it cannot be eliminated
> by any RTL optimizer. For that to happen it needs to know that the BB the
> instruction is in
> is guaranteed that there is always at least one bit set. This is not the
> same as nonzero_bits
> because this is a mask, and we simply don't know which bit may be non-zero.
> We just know
> there is one.
>
> So I don't understand why -1 makes it easier for RVV, it would quite
> literally never be used.
Right, if it's just used in the exit block, indeed, the -1 is
unreachable. As above, for RVV I'd like IFN_OTHER_NAME to double as
sentinel for "nothing found" as well as the index of the match. Then
we'd only have the vfirst inside the loop and the exit block could
re-use the value for the full index calculation.
So all in all, for RVV, ideally we'd get
a < b # unmasked cmp
ix = IFN_COND_LEN_OTHER_NAME # returns -1 for no match and index otherwise
if (ix == -1)
goto loop;
# do something with ix (or not)
So purely for RVV, not for SVE which obviously has different tradeoffs:
Do you see this approach (unmasked comparison, masked reduction) in
conflict with anything done and suggested for SVE and the other archs?
IFN_COND_LEN_OTHER_NAME I'd want to emit right away in the vectorizer.
Obviously we'd need some tweaks here and there and essentially the code
paths would fork a little.
Maybe it would be easier to argue based on the code once I have it. But
I didn't want to start before your series is in. I guess my main point
still is that I don't think we need the LEN variant of COND_VEC_CMP as
well as their optimizations.
--
Regards
Robin
