On Fri, Jun 28, 2024 at 2:16 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Fri, Jun 28, 2024 at 11:06 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> >
> >
> > > Am 28.06.2024 um 10:27 schrieb Richard Sandiford
> > > <richard.sandif...@arm.com>:
> > >
> > > 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 is it necessary that only one bit is set for SVE?
> >
> > >>> 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);
> > >
> > > ?
> >
> > Note the path is also necessary for avx512 and gcn mask modes which are
> > integer modes.
> >
> > > 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.)
> >
> > That’s a good question and also related to GCC vector extension which can
> > result in both BLKmode and integer modes to be used. But I’m not sure how
> > we expose masks to the middle end here. A too large vector bool could be
> > lowered to AVX512 mode. Maybe we should simply reject interpret/encode of
> > BLKmode vectors and make sure to never assign integer modes to vector bools
> > (if the target didn’t specify that mode)?
> >
> > I guess some test coverage would be nice here.
>
> To continue on that, we do not currently have a way to capture a
> vector comparison output
> but the C++ frontend has vector ?:
>
> typedef int v8si __attribute__((vector_size(32)));
>
> void foo (v8si *a, v8si *b, v8si *c)
> {
> *c = *a < *b ? (v8si){-1,-1,-1,-1,-1,-1,-1,-1 } : (v8si){0,0,0,0,0,0,0,0};
> }
>
> with SSE2 we get a <signed-boolean:32> temporary, with AVX512 enabled
> that becomes <singed-boolean:1>. When we enlarge the vector to size 128
> then even with AVX512 enabled I see <signed-boolean:32> here and
> vector lowering decomposes that to scalar (also with AVX or SSE, so maybe
> just a missed optimization). But note that to decompose this into two
> AVX512 vectors the temporary would have to change from <signed-boolean:32>
> elements to <signed-boolean:1>.
>
> The not supported vector bool types have BLKmode sofar.
>
> But for example on i?86-linux with -mno-sse (like -march=i586) for
>
> typedef short v2hi __attribute__((vector_size(4)));
>
> void foo (v2hi *a, v2hi *b, v2hi *c)
> {
> *c = *a < *b ? (v2hi){-1,-1} : (v2hi){0,0};
> }
>
> we get a SImode vector <signed-boolean:16> as I feared. That means
> <signed-boolean:8> (the BITS_PER_UNIT case) can be ambiguous
> between SVE (bool for a 8byte data vector) and emulated vectors
> ("word-mode" vectors; for 1byte data vectors).
>
> And without knowing that SVE would have used VnBImode given that
> AVX512 uses an integer mode.
>
> Aside from the too large vector and AVX512 issue above I think we can use
> MODE_VECTOR_BOOL || TYPE_PRECISION == 1 and for the latter we
> can assert the mode is a scalar integer mode (AVX512 or GCN)?So like the attached? Richard.
p
Description: Binary data
