Tamar Christina <tamar.christ...@arm.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford <richard.sandif...@arm.com>
>> Sent: Friday, July 26, 2024 10:24 AM
>> To: Tamar Christina <tamar.christ...@arm.com>
>> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; Richard Earnshaw
>> <richard.earns...@arm.com>; Marcus Shawcroft
>> <marcus.shawcr...@arm.com>; ktkac...@gcc.gnu.org
>> Subject: Re: [PATCH]AArch64: check for vector mode in get_mask_mode
>> [PR116074]
>>
>> Tamar Christina <tamar.christ...@arm.com> writes:
>> > Hi All,
>> >
>> > For historical reasons AArch64 has TI mode vector types but does not
>> > consider
>> > TImode a vector mode.
>> >
>> > What's happening in the PR is that get_vectype_for_scalar_type is returning
>> > vector(1) TImode for a TImode scalar. This then fails when we call
>> > targetm.vectorize.get_mask_mode (vecmode).exists (&) on the TYPE_MODE.
>> >
>> > I've checked other usages of get_mask_mode and none of them have anything
>> that
>> > would prevent this same issue from happening. It only happens that
>> > normally
>> > the vectorizer rejects the vector(1) type early, but in this case we get
>> > further because the COND_EXPR hasn't been analyzed yet for a type.
>> >
>> > I believe get_mask_mode shouldn't fault, and so this adds the check for
>> > vector
>> > mode in the hook and returns nothing if it's not. I did not add this to
>> > the
>> > generic function because I believe this is an AArch64 quirk.
>>
>> Feels to me like the problem is higher up. The hook says:
>>
>> Return the mode to use for a vector mask that holds one boolean
>> result for each element of vector mode @var{mode}. ...
>>
>> So the caller is breaking the interface if it is passing a non-vector mode.
>
> But my point is that it's AArch64 that creates a vector type from a
> non-vector mode.
> I can easily fix my own call site. But it's weird that AArch64 has a
> TYPE_MODE (vectorype) != VECTOR_MODE.
Vector types without vector modes aren't unusual in themselves.
It's how many "simple" vector types are represented on non-vector ISAs.
And on aarch64:
typedef unsigned char foo __attribute__((vector_size(2)));
is a vector type with HImode. I don't think the hook is expected to
deal with those.
The vectoriser also directly supports vector types with integer modes
in limited cases, via vect_can_vectorize_without_simd_p.
Thanks,
Richard
>
> So I still think this is an AArch64 bug.
>
> Tamar
>>
>> Thanks,
>> Richard
>>
>> >
>> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >
>> > Ok for master?
>> >
>> > Thanks,
>> > Tamar
>> >
>> > gcc/ChangeLog:
>> >
>> > PR target/116074
>> > * config/aarch64/aarch64.cc (aarch64_get_mask_mode): Check vector
>> mode.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > PR target/116074
>> > * g++.target/aarch64/pr116074.C: New test.
>> >
>> > ---
>> >
>> > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> > index
>> 355ab97891cf0a7d487fa4c69ae23a5f75897851..045ac0e09b0eaa14935db392
>> 4798402c9dd1947c 100644
>> > --- a/gcc/config/aarch64/aarch64.cc
>> > +++ b/gcc/config/aarch64/aarch64.cc
>> > @@ -1870,6 +1870,9 @@ aarch64_sve_pred_mode (machine_mode mode)
>> > static opt_machine_mode
>> > aarch64_get_mask_mode (machine_mode mode)
>> > {
>> > + if (!VECTOR_MODE_P (mode))
>> > + return opt_machine_mode ();
>> > +
>> > unsigned int vec_flags = aarch64_classify_vector_mode (mode);
>> > if (vec_flags & VEC_SVE_DATA)
>> > return aarch64_sve_pred_mode (mode);
>> > diff --git a/gcc/testsuite/g++.target/aarch64/pr116074.C
>> b/gcc/testsuite/g++.target/aarch64/pr116074.C
>> > new file mode 100644
>> > index
>> 0000000000000000000000000000000000000000..54cf561510c460499a816a
>> b6a84603fc20a5f1e5
>> > --- /dev/null
>> > +++ b/gcc/testsuite/g++.target/aarch64/pr116074.C
>> > @@ -0,0 +1,24 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-additional-options "-O3" } */
>> > +
>> > +int m[40];
>> > +
>> > +template <typename k> struct j {
>> > + int length;
>> > + k *e;
>> > + void operator[](int) {
>> > + if (length)
>> > + __builtin___memcpy_chk(m, m+3, sizeof (k), -1);
>> > + }
>> > +};
>> > +
>> > +j<j<int>> o;
>> > +
>> > +int *q;
>> > +
>> > +void ao(int i) {
>> > + for (; i > 0; i--) {
>> > + o[1];
>> > + *q = 1;
>> > + }
>> > +}
