Re: [PATCH] powerpc: Add optimized strlen for POWER10

[Matheus Castanho <msc@linux.ibm.com>]

Paul E Murphy <murphyp@linux.ibm.com> writes:

> On 4/16/21 4:24 PM, Matheus Castanho via Libc-alpha wrote:
>
>> diff --git a/sysdeps/powerpc/powerpc64/le/power10/strlen.S b/sysdeps/powerpc/powerpc64/le/power10/strlen.S
>> new file mode 100644
>> index 0000000000..5218a2277c
>> --- /dev/null
>> +++ b/sysdeps/powerpc/powerpc64/le/power10/strlen.S
>> @@ -0,0 +1,219 @@
>> +/* Optimized strlen implementation for POWER10 LE.
>> +   Copyright (C) 2021 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
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +#include <sysdep.h>
>> +
>> +#ifndef STRLEN
>> +# define STRLEN __strlen
>> +# define DEFINE_STRLEN_HIDDEN_DEF 1
>> +#endif
>> +
>> +#define VEXTRACTBM(rt,vrb)	 \
>> +	.long(((4)<<(32-6))	 \
>> +	      | ((rt)<<(32-11))	 \
>> +	      | ((8)<<(32-16))	 \
>> +	      | ((vrb)<<(32-21)) \
>> +	      | 1602)
>> +
>> +#define LXVP(xtp,dq,ra)		   \
>> +	.long(((6)<<(32-6))		   \
>> +	      | ((((xtp)-32)>>1)<<(32-10)) \
>> +	      | ((1)<<(32-11))		   \
>> +	      | ((ra)<<(32-16))		   \
>> +	      | dq)
>
> Can you add a comment to these noting the minimum version of binutils
> which supports these instructions?
>
>> +
>> +#define CHECK16(reg,offset,addr,label)	\
>> +	lxv	  reg+32,offset(addr);	\
>> +	vcmpequb. reg,reg,v18;		\
>> +	bne	  cr6,L(label);
>
> Maybe rename reg to vreg for clarity?
>
> With the trivial comments above addressed, I think this patch is OK.

Thanks for the review. Just posted v2 [1] with the suggested fixes.

[1] https://sourceware.org/pipermail/libc-alpha/2021-April/125307.html

--
Matheus Castanho