> Am 26.07.2024 um 11:29 schrieb Tamar Christina <tamar.christ...@arm.com>:
>
>
>>
>> -----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.
>
> So I still think this is an AArch64 bug.
Note the middle-end creates such vector types as well, though usually not with
TImode. Generally I’d consider using the
Scalar mode for vector(1) appropriate.
Richard
> 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;
>>> + }
>>> +}
