Kyrylo Tkachov <kyrylo.tkac...@arm.com> writes:
> Hi all,
>
> Besides the builtins in aarch64-simd-builtins.def there are a number of
> builtins defined in aarch64-builtins.c itself.
> They could also benefit from the attributes generated by
> aarch64_get_attributes.
> However aarch64_get_attributes and its helpers are only set up to handle a
> aarch64_simd_builtin_datum.
>
> This patch changes these functions to instead take a flag and mode value that
> are extracted from
> aarch64_simd_builtin_datum.flags and aarch64_simd_builtin_datum.mode anyway.
> Then the various builtin init functions in aarch64-builtins.c can pass down
> their own FLAG_* flags
> that they want to derive attributes from.
> I'm not too happy with changing the helper functions in this way, but it
> seemed like the least bad option.
>
> Richard, does this look like an ok approach to you?
Looks good to me. Just some very minor things:
I guess this is personal preference, but in:
> + unsigned int flags = FLAG_FP;
> + tree attrs = aarch64_get_attributes (flags, d->mode);
it seemed odd to separate out the flags variable instead of just having:
tree attrs = aarch64_get_attributes (FLAG_FP, d->mode);
Long line in:
> + tree fndecl = aarch64_general_add_builtin (d->name, ftype, d->fcode,
> attrs);
For;
> @@ -906,14 +906,13 @@ aarch64_init_simd_builtin_scalar_types (void)
> "__builtin_aarch64_simd_udi");
> }
>
> -/* Return a set of FLAG_* flags that describe what the function could do,
> +/* Adjust the set of FLAG_* flags derived from FLAGS
> + that describe what the function with result MODE could do,
I think this is easier to parse without the s/Return a/Adjust the/.
Also, s/the function/a function/ seems better, now that we're not passing
a specific function. Same for the other comments.
Thanks,
Richard
