Richard Biener <richard.guent...@gmail.com> writes:
> On Fri, Jun 28, 2024 at 8:01 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
>>
>> On Fri, Jun 28, 2024 at 3:15 AM liuhongt <hongtao....@intel.com> wrote:
>> >
>> > for the testcase in the PR115406, here is part of the dump.
>> >
>> > char D.4882;
>> > vector(1) <signed-boolean:8> _1;
>> > vector(1) signed char _2;
>> > char _5;
>> >
>> > <bb 2> :
>> > _1 = { -1 };
>> >
>> > When assign { -1 } to vector(1} {signed-boolean:8},
>> > Since TYPE_PRECISION (itype) <= BITS_PER_UNIT, so it set each bit of dest
>> > with each vector elemnet. But i think the bit setting should only apply for
>> > TYPE_PRECISION (itype) < BITS_PER_UNIT. .i.e for vector(1).
>> > <signed-boolean:16>, it will be assigned as -1, instead of 1.
>> > Is there any specific reason vector(1) <signed-boolean:8> is handled
>> > differently from vector<1> <signed-boolean:16>?
>> >
>> > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
>> > Ok for trunk?
>>
>> I agree that <= BITS_PER_UNIT is suspicious, but the bit-precision
>> code should work for 8 bit
>> entities as well, it seems we only set the LSB of each element in the
>> "mask". ISTR that SVE
>> masks can have up to 8 bit elements (for 8 byte data elements), so
>> maybe that's why
>> <= BITS_PER_UNIT.
Yeah.
>> So maybe instead of just setting one bit in
>>
>> ptr[bit / BITS_PER_UNIT] |= 1 << (bit % BITS_PER_UNIT);
>>
>> we should set elt_bits bits, aka (without testing)
>>
>> ptr[bit / BITS_PER_UNIT] |= (1 << elt_bits - 1) << (bit
>> % BITS_PER_UNIT);
>>
>> ?
>
> Alternatively
>
> if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (expr))
> && TYPE_PRECISION (itype) <= BITS_PER_UNIT)
>
> should be amended with
>
> && GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (expr))) != MODE_VECTOR_INT
How about:
if (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (expr))) == MODE_VECTOR_BOOL)
{
gcc_assert (TYPE_PRECISION (itype) <= BITS_PER_UNIT);
?
Is it OK for TYPE_MODE to affect tree-level semantics though, especially
since it can change with the target attribute? (At least TYPE_MODE_RAW
would be stable.)
> maybe. Still for the possibility of vector(n) <signed-boolean:16>
> mask for a int128 element vector
> we'd have 16bit mask elements, encoding that differently would be
> inconsistent as well
> (but of course 16bit elements are not handled by the code right now).
Yeah, 16-bit predicate elements aren't a thing for SVE, so we've not
had to add support for them.
Richard
