> On Nov 1, 2024, at 10:06, Kito Cheng <kito.ch...@gmail.com> wrote:
>
> Generally LGTM, just a few minor comments for the C-ish stuff...
>
>> +
>> + /* At least one more version other than the default. */
>> + unsigned int num_versions = fndecls->length ();
>> + gcc_assert (num_versions >= 2);
>> +
>> + struct function_version_info
>> + {
>> + tree version_decl;
>> + int prio;
>> + struct riscv_feature_bits features;
>> + } *function_versions;
>> +
>> + function_versions = (struct function_version_info *)
>> + XNEWVEC (struct function_version_info, (num_versions));
>
> Use std::vector instead
>
>> +
>> + unsigned int actual_versions = 0;
>> +
>> + for (tree version_decl : *fndecls)
>> + {
>> + function_versions[actual_versions].version_decl = version_decl;
>> + // Get attribute string, parse it and find the right features.
>> + parse_features_for_version (version_decl,
>> +
>> function_versions[actual_versions].features,
>> + function_versions[actual_versions].prio);
>> + actual_versions++;
>> + }
>> +
>> +
>> + auto compare_feature_version_info = [](const void *p1, const void *p2)
>> + {
>> + const function_version_info v1 = *(const function_version_info *)p1;
>> + const function_version_info v2 = *(const function_version_info *)p2;
>> + return - compare_fmv_features (v1.features, v2.features,
>> + v1.prio, v2.prio);
>> + };
>> +
>> + /* Sort the versions according to descending order of dispatch priority.
>> */
>> + qsort (function_versions, actual_versions,
>> + sizeof (struct function_version_info),
>> compare_feature_version_info);
>
> Use std::sort instead, I
Thanks for these review opinions. Except for the comments, there
is one problem that needs to be resolved before the next revision:
the gp problem when calling IFUNC resolver.
See that thread on GLIBC patch.
Link:
https://patchwork.sourceware.org/project/glibc/patch/tencent_71d182fbda6e8e57b80731dd218d8d5c7...@qq.com/
Through the discussion, a possible solution might be to refuse the
relax for the IFUNC resolvers and its callees. Thus, we don’t require
gp to access global variables like __riscv_feature_bits.
I came up with a solution that adds an attribute like
__attribute__((norelax)) to functional like -mno-relax for a specific
function to both the FMV resolver gimple function and the libgcc
source code which will be called from FMV resolver.
Would this solution be acceptable?
Thanks,
Yangyu Chen
