This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 0/3] sparc M7 optimized memcpy/memset
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Fri, 3 Nov 2017 21:51:12 -0200
- Subject: Re: [PATCH 0/3] sparc M7 optimized memcpy/memset
- Authentication-results: sourceware.org; auth=none
- References: <1506542999-97895-1-git-send-email-patrick.mcgehearty@oracle.com> <20170927.214103.2236958753795210322.davem@davemloft.net> <1f543a82-5816-11f1-d615-a5b4a07c8392@oracle.com> <5de85da5-b994-1619-a682-7d1fcd2179f6@linaro.org> <c3b177a8-f20e-b5c7-16dd-9b4440b31bd2@oracle.com> <da5e9e31-4323-0415-4d35-d956d5f9546b@linaro.org> <4b693056-34e6-f4be-4161-59503de32e8a@oracle.com>
On 02/11/2017 13:17, Patrick McGehearty wrote:
> On 10/26/2017 2:39 PM, Adhemerval Zanella wrote:
>>
>> On 19/10/2017 14:46, Patrick McGehearty wrote:
>>
>>>> Adhemerval asked that the ifunc selector code use C instead of assembly.
>>>> He has prepared changes to replace all assembly uses of ifunc with C.
>>> The existing ifunc code in that directory uses assembly.
>>> Adhemerval's changes were not available when I submitted the patch.
>>> As far as I understand, they are still not available in the main branch.
>>> Making those changes increases risk for my memcpy/memset optimizations.
>>> I consider such work, while worthy in its own right, to be out of
>>> scope for my patch set.
>>>
>>> Respectively,
>>> - Patrick McGehearty
>> I just sent a patchset to refactor all remaining IFUNC resolver still
>> in assembly to C, including all the missing sparc ones.
>>
>> Also, I adjusted your patches to my refactor in personal branch [2] and
>> I also split the patch in memcpy/memmove and memset/bzero. It simplifies
>> a lot new ifunc inclusions, for instance the memcpy part is just:
>>
>> ---
>> diff --git a/sysdeps/sparc/sparc64/multiarch/ifunc-memcpy.h b/sysdeps/sparc/sparc64/multiarch/ifunc-memcpy.h
>> index 46f3795..dbdad2d 100644 (file)
>> --- a/sysdeps/sparc/sparc64/multiarch/ifunc-memcpy.h
>> +++ b/sysdeps/sparc/sparc64/multiarch/ifunc-memcpy.h
>> @@ -19,6 +19,7 @@
>> #include <ifunc-init.h>
>> +extern __typeof (REDIRECT_NAME) OPTIMIZE (niagara7) attribute_hidden;
>> extern __typeof (REDIRECT_NAME) OPTIMIZE (niagara4) attribute_hidden;
>> extern __typeof (REDIRECT_NAME) OPTIMIZE (niagara2) attribute_hidden;
>> extern __typeof (REDIRECT_NAME) OPTIMIZE (niagara1) attribute_hidden;
>> @@ -28,6 +29,8 @@ extern __typeof (REDIRECT_NAME) OPTIMIZE (ultra1) attribute_hidden;
>> static inline void *
>> IFUNC_SELECTOR (int hwcap)
>> {
>> + if (hwcap & HWCAP_SPARC_ADP)
>> + return OPTIMIZE (niagara7);
>> if (hwcap & HWCAP_SPARC_CRYPTO)
>> return OPTIMIZE (niagara4);
>> if (hwcap & HWCAP_SPARC_N2)
>> ---
>>
>> So if you could help with any review I will be thankful. I would expect
>> the memcpy/memmove and memset/bzero refactor to be straightforward.
>>
>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/ifunc-c-sparc-m7
> I've reviewed Adhemerval's changes including building
> and running on my sparc s7 system. They look good to me.
> Only issue I see is a naming difference in sysdeps/sparc/sparc64/multiarch
> between:
> ifunc-memmove.c
> memcpy.c
> mempcpy.c
> memset.c
>
> Perhaps ifunc-memmove.c should be named memmove.c for consistency
> with memset.c, mempcpy.c and memcpy.c? Or all named ifunc-mem*.c?
Indeed this naming is a mistake and I will correct it before send it upstream
(sysdeps/sparc/sparc64/multiarch/ifunc-memmove.c should be indded
sysdeps/sparc/sparc64/multiarch/memmove.c).
It also shown a missing #endif at the end the file, thanks.