Spencer Abson <spencer.ab...@arm.com> writes:
> On Tue, Jun 10, 2025 at 08:04:06PM +0100, Richard Sandiford wrote:
>> Spencer Abson <spencer.ab...@arm.com> writes:
>> > On Fri, Jun 06, 2025 at 03:52:12PM +0100, Richard Sandiford wrote:
>> >> Spencer Abson <spencer.ab...@arm.com> writes:
>> >> > @@ -8165,20 +8169,25 @@
>> >> >  ;;
>> >> >  ;; For unpacked vectors, it doesn't really matter whether SEL uses the
>> >> >  ;; the container size or the element size.  If SEL used the container 
>> >> > size,
>> >> > -;; it would ignore undefined bits of the predicate but would copy the
>> >> > -;; upper (undefined) bits of each container along with the defined 
>> >> > bits.
>> >> > -;; If SEL used the element size, it would use undefined bits of the 
>> >> > predicate
>> >> > -;; to select between undefined elements in each input vector.  Thus 
>> >> > the only
>> >> > -;; difference is whether the undefined bits in a container always come 
>> >> > from
>> >> > -;; the same input as the defined bits, or whether the choice can vary
>> >> > +;; it would ignore bits of the predicate that can be undefined, but 
>> >> > would copy
>> >> > +;; the upper (undefined) bits of each container along with the defined 
>> >> > bits.
>> >> > +;; If SEL used the element size, it would use bits from the predicate 
>> >> > that can
>> >> > +;; be undefined to select between undefined elements in each input 
>> >> > vector.
>> >> > +;; Thus the only difference is whether the undefined bits in a 
>> >> > container always
>> >> > +;; come from the same input as the defined bits, or whether the choice 
>> >> > can vary
>> >> 
>> >> It looks like the main change here is to replace "undefined bits of the
>> >> predicate" with "bits of the predicate that can be undefined".  Could
>> >> you go into more detail about the distinction?  It seems to be saying
>> >> that the upper bits in each predicate are sometimes defined and
>> >> sometimes not.
>> >> 
>> >> As I see it, the "level of undefinedness" is really the same for the
>> >> predicates and data vectors.  Within each container element, the bits
>> >> that hold the element value are defined/significant and the other bits
>> >> are undefined/insignificant/have arbitrary values.  The same thing
>> >> goes for the upper bits in each predicate element: the low bit is
>> >> defined/significant and the other bits are undefined/insignificant/have
>> >> arbitrary values.  They might by chance be zeros when zeros are
>> >> convenient or ones when ones are convenient, but the semantics don't
>> >> guarantee anything either way.
>> >
>> > Yes, I agree.  Sorry, my change was not very clear.
>> >
>> > What I was trying to reflect is that, for example, selecting between
>> > a pair of VNx4HF using VNx8BI is now a recognised insn.  However, any
>> > bits of a VNx8BI that are not significant to a VNx4BI are don't-care
>> > wrt the result.
>> >
>> > I meant that they 'can be undefined' in that they are as good as
>> > undefined, for the purpose of SEL.  Maybe a better change would
>> > have been to reword this paragraph in favour of 'don't-care' rather
>> > than 'undefined' when describing the upper bits of each predicate
>> > element?
>> 
>> Ah, I see.  In that case, how about a bigger edit:
>> 
>> ;; For unpacked vectors, it doesn't really matter whether SEL uses the
>> ;; the container size or the element size.  If SEL used the container size,
>> ;; it would would copy the upper (undefined) bits of each container along
>> ;; with the corresponding defined bits.  If SEL used the element size,
>> ;; it would use separate predicate bits to select between the undefined
>> ;; elements in each input vector; these seperate predicate bits might
>> ;; themselves be undefined, depending on the mode of the predicate.
>> ;;
>> ;; Thus the only difference is whether the undefined bits in a container
>> ;; always come from the same input as the defined bits, or whether the
>> ;; choice can vary independently of the defined bits.
>
> Yeah, that's much clearer.
>
> Thanks for this review, I'll apply your suggestions and commit the
> patches with minor changes - with the vectorizer test change applied
> across the series where applicable.
>
> Unless I run into something unexpected, it looks like I may only need
> to repost this patch (11/14) and patch 04, does that sound right to
> you?

Yeah, that sounds right.

Thanks,
Richard

Reply via email to