"H.J. Lu" <hjl.to...@gmail.com> writes:
> On Thu, Mar 19, 2020 at 4:10 AM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> In this PR we had a 512-bit VECTOR_TYPE whose mode is XImode
>> (an integer mode used for four 128-bit vectors). When trying
>> to expand a zero constant for it, we hit code in expand_expr_real_1
>> that tries to use the associated integer type instead. The code used
>> type_for_mode (XImode, 1) to get this integer type.
>>
>> However, the c-family implementation of type_for_mode checks for
>> any registered built-in type that matches the mode and has the
>> right signedness. This meant that it could return a built-in
>> vector type when given an integer mode (particularly if, as here,
>> the vector type isn't supported by the current subtarget and so
>> TYPE_MODE != TYPE_MODE_RAW). The expand code would then cycle
>> endlessly trying to use this "new" type instead of the original
>> vector type.
>>
>> The search loop is probably too lax in other ways -- e.g. it could
>> return records that just happen to have the right mode -- but this
>> seems like a safe, incremental improvement.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>>
>> Richard
>>
>>
>> 2020-03-18 Richard Sandiford <richard.sandif...@arm.com>
>>
>> gcc/c-family/
>> PR middle-end/94072
>> * c-common.c (c_common_type_for_mode): Before using a registered
>> built-in type, check that the vectorness of the type matches
>> the vectorness of the mode.
>>
>> gcc/testsuite/
>> PR middle-end/94072
>> * gcc.target/aarch64/pr94072.c: New test.
>> ---
>> gcc/c-family/c-common.c | 11 +++++++----
>> gcc/testsuite/gcc.target/aarch64/pr94072.c | 9 +++++++++
>> 2 files changed, 16 insertions(+), 4 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94072.c
>>
>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
>> index 25020bf1415..8e5a9243827 100644
>> --- a/gcc/c-family/c-common.c
>> +++ b/gcc/c-family/c-common.c
>> @@ -2387,10 +2387,13 @@ c_common_type_for_mode (machine_mode mode, int
>> unsignedp)
>> }
>>
>> for (t = registered_builtin_types; t; t = TREE_CHAIN (t))
>> - if (TYPE_MODE (TREE_VALUE (t)) == mode
>> - && !!unsignedp == !!TYPE_UNSIGNED (TREE_VALUE (t)))
>> - return TREE_VALUE (t);
>> -
>> + {
>> + tree type = TREE_VALUE (t);
>> + if (TYPE_MODE (type) == mode
>> + && VECTOR_TYPE_P (type) == VECTOR_MODE_P (mode)
>> + && !!unsignedp == !!TYPE_UNSIGNED (type))
>> + return type;
>> + }
>> return NULL_TREE;
>> }
>>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr94072.c
>> b/gcc/testsuite/gcc.target/aarch64/pr94072.c
>> new file mode 100644
>> index 00000000000..2aa72eb7a16
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr94072.c
>> @@ -0,0 +1,9 @@
>> +/* { dg-options "-msve-vector-bits=512" } */
>> +
>> +#pragma GCC target "+nosve"
>> +
>> +void
>> +foo (void)
>> +{
>> + (int __attribute__ ((__vector_size__ (64)))){};
>> +}
>
> Shouldn't this test also be enabled for AVX512F?
The test already worked on 512-bit vector targets (including 512-bit SVE).
The problem was when the SVE length was set to 512 bits but SVE itself
wasn't enabled.
There are already more extensive tests for:
int __attribute__ ((__vector_size__ (64)))
and similar vectors in gcc.dg and gcc.c-torture, so I don't think
making this generic would really add much.
Thanks,
Richard
