Kyrylo Tkachov <kyrylo.tkac...@arm.com> writes:
> Hi Jakub,
>
>> -----Original Message-----
>> From: Jakub Jelinek <ja...@redhat.com>
>> Sent: 13 March 2021 20:34
>> To: Richard Sandiford <richard.sandif...@arm.com>; Richard Earnshaw
>> <richard.earns...@arm.com>; Kyrylo Tkachov <kyrylo.tkac...@arm.com>
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: [PATCH] aarch64: Fix up
>> aarch64_simd_clone_compute_vecsize_and_simdlen [PR99542]
>>
>> Hi!
>>
>> As the patch shows, there are several bugs in
>> aarch64_simd_clone_compute_vecsize_and_simdlen.
>> One is that unlike for function declarations that aren't definitions
>> it completely ignores argument types. Such decls don't have
>> DECL_ARGUMENTS,
>> but we can walk TYPE_ARG_TYPES instead, like the i386 backend does or like
>> the simd cloning code in the middle end does too.
>>
>> Another problem is that it checks types of uniform arguments. That is
>> unnecessary, uniform arguments are passed the way it normally is, it is
>> a scalar argument rather than vector, so there is no reason not to support
>> uniform argument of different size, or long double, structure etc.
>>
>> Fixed thusly, bootstrapped/regtested on aarch64-linux, ok for trunk?
>
> I don't have much experience in this part of GCC and I think you're best
> placed to make a judgement on the implementation of this hook.
> It's okay from my perspective (it doesn't look like it would break any SVE
> logic), but if you'd like to wait for a review from Richard I won't object.
Yeah, it looks good to me too, sorry for the slow response.
IMO:
tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl));
bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE);
for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i = 0;
t && t != void_list_node; t = TREE_CHAIN (t), i++)
{
tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t);
shows we have an interface problem here -- we shouldn't need something this
convoluted to walk through an argument list. But that's not your problem
or a problem with the patch.
Thanks,
Richard
