Re: [PATCH] powerpc: strcmp optimization for power9

[Carlos Eduardo Seo <cseo at linux dot vnet dot ibm dot com>]

On 11/14/16 9:29 AM, Rajalakshmi Srinivasaraghavan wrote:
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile

OK

> diff --git a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c

OK

> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcmp-power9.S b/sysdeps/powerpc/powerpc64/multiarch/strcmp-power9.S

OK

> new file mode 100644
> index 0000000..0a09e5b
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/multiarch/strcmp-power9.S
> @@ -0,0 +1,40 @@
> +/* Optimized strcmp implementation for POWER9/PPC64.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <sysdep.h>
> +
> +#undef EALIGN
> +#define EALIGN(name, alignt, words)				\

I noticed that all of our implementations for multiarch/<function>.S
have this 'alignt' typo. It doesn't affect functionality since the typo
is present in all usages of the variable, but it'd be nice to clean this
up in the future.

Apart from this, OK

> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcmp.c b/sysdeps/powerpc/powerpc64/multiarch/strcmp.c

OK

> diff --git a/sysdeps/powerpc/powerpc64/power9/strcmp.S b/sysdeps/powerpc/powerpc64/power9/strcmp.S
> new file mode 100644
> index 0000000..cc24296
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/power9/strcmp.S
> @@ -0,0 +1,279 @@
> +/* Optimized strcmp implementation for PowerPC64/POWER9.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#ifdef __LITTLE_ENDIAN__
> +#include <sysdep.h>
> +
> +/* Implements the function
> +
> +   int [r3] strcmp (const char *s1 [r3], const char *s2 [r4])
> +
> +   The implementation uses unaligned doubleword access for first 32 bytes
> +   as in POWER8 path and uses vectorised loops after that.  */

s/path/patch

> +
> +/* Macros are defined below for these newer instructions in order
> +   to maintain compatibility.  */

It's probably good to leave a TODO here to update these to the actual
instructions when the minimum binutils is upgraded to 2.27, like you did
below for .machine power7.

The rest of the implementation looks OK, but I'll double check in the
simulator and get back to you.

-- 
Carlos Eduardo Seo
Software Engineer - Linux on Power Toolchain
cseo@linux.vnet.ibm.com