Re: [PATCH 2/3] aarch64: libgcc: add prototypes in cpuinfo

[Richard Sandiford]

Christophe Lyon <christophe.l...@linaro.org> writes:
> On Fri, 4 Oct 2024 at 10:00, Kyrylo Tkachov <ktkac...@nvidia.com> wrote:
>>
>>
>>
>> > On 3 Oct 2024, at 21:44, Christophe Lyon <christophe.l...@linaro.org> 
>> > wrote:
>> >
>> > External email: Use caution opening links or attachments
>> >
>> >
>> > Add prototypes for __init_cpu_features_resolver and
>> > __init_cpu_features to avoid warnings due to -Wmissing-prototypes.
>> >
>> >        libgcc/
>> >        * config/aarch64/cpuinfo.c (__init_cpu_features_resolver): Add
>> >        prototype.
>> >        (__init_cpu_features): Likewise.
>> > ---
>> > libgcc/config/aarch64/cpuinfo.c | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/libgcc/config/aarch64/cpuinfo.c 
>> > b/libgcc/config/aarch64/cpuinfo.c
>> > index 4b94fca8695..c62a7453e8e 100644
>> > --- a/libgcc/config/aarch64/cpuinfo.c
>> > +++ b/libgcc/config/aarch64/cpuinfo.c
>> > @@ -418,6 +418,7 @@ __init_cpu_features_constructor(unsigned long hwcap,
>> >   setCPUFeature(FEAT_INIT);
>> > }
>> >
>> > +void __init_cpu_features_resolver(unsigned long, const __ifunc_arg_t *);
>> > void
>> > __init_cpu_features_resolver(unsigned long hwcap, const __ifunc_arg_t 
>> > *arg) {
>> >   if (__aarch64_cpu_features.features)
>> > @@ -425,6 +426,7 @@ __init_cpu_features_resolver(unsigned long hwcap, 
>> > const __ifunc_arg_t *arg) {
>> >   __init_cpu_features_constructor(hwcap, arg);
>> > }
>> >
>> > +void __init_cpu_features(void);
>> > void __attribute__ ((constructor))
>> > __init_cpu_features(void) {
>> >   unsigned long hwcap;
>>
>> I thought the intent of the missing-prototypes warning is to warn about 
>> missing prototypes in a header file primarily.
>
> Indeed, that's my understanding too....
>
>> Should these prototypes go into gcc/common/config/aarch64/cpuinfo.h instead?
> In that case, compilation of gcc/config/aarch64/aarch64.c fails because:
> gcc/common/config/aarch64/cpuinfo.h:96:56: error: ‘__ifunc_arg_t’ does
> not name a type
> and it does not seem obvious to expose this type in aarch64.c

Yeah, I agree the prototype shouldn't be visible to GCC code.

> IIUC, these functions never have their prototypes exposed/used, and
> I'm not even sure how __init_cpu_features is called: in
> dispatch_function_versions(), I only see a reference to
> __init_cpu_features_resolver?

The main way it gets called is automatically via the constructor.
Making it public, and making it resilient against multiple calls,
seems like a useful safety net in case the normal constructor doesn't
get run early enough.

Given all that, I think declaring them in the .c is probably the
best approach.  The patch LGTM FWIW.

Thanks,
Richard