Re: [PATCH 0/3] sparc M7 optimized memcpy/memset

[Patrick McGehearty <patrick dot mcgehearty at oracle dot com>]

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?

- Patrick McGehearty