[v11,20/29] riscv: Add string-fza.h and string-fzi.h

  It uses the bitmanip extension to optimize index_fist and index_last
with clz/ctz (using generic implementation that routes to compiler
builtin) and orc.b to check null bytes.

Checked the string test on riscv64 user mode.
---
 sysdeps/riscv/string-fza.h | 70 ++++++++++++++++++++++++++++++++++
 sysdeps/riscv/string-fzi.h | 77 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+)
 create mode 100644 sysdeps/riscv/string-fza.h
 create mode 100644 sysdeps/riscv/string-fzi.h
  

On 2/1/23 07:03, Adhemerval Zanella wrote:
> +static __always_inline unsigned int
> +index_first (find_t c)
> +{
> +  if (c & 0x80U)
> +    return 0;
> +  if (c & 0x8000U)
> +    return 1;
> +  if (c & 0x800000U)
> +    return 2;
> +
> +  if (sizeof (op_t) == 4)
> +    return 3;
> +
> +  if (c & 0x80000000U)
> +    return 3;
> +  if (c & 0x8000000000UL)
> +    return 4;
> +  if (c & 0x800000000000UL)
> +    return 5;
> +  if (c & 0x80000000000000UL)
> +    return 6;
> +  return 7;

There is a -mbig-endian switch to gcc, even if that isn't the normal configuration.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
  

On Wed, Feb 1, 2023 at 11:05 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> It uses the bitmanip extension to optimize index_fist and index_last
> with clz/ctz (using generic implementation that routes to compiler
> builtin) and orc.b to check null bytes.
>
> Checked the string test on riscv64 user mode.
> ---
>  sysdeps/riscv/string-fza.h | 70 ++++++++++++++++++++++++++++++++++
>  sysdeps/riscv/string-fzi.h | 77 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 147 insertions(+)
>  create mode 100644 sysdeps/riscv/string-fza.h
>  create mode 100644 sysdeps/riscv/string-fzi.h
>
> diff --git a/sysdeps/riscv/string-fza.h b/sysdeps/riscv/string-fza.h
> new file mode 100644
> index 0000000000..9c7a6efba2
> --- /dev/null
> +++ b/sysdeps/riscv/string-fza.h
> @@ -0,0 +1,70 @@
> +/* Zero byte detection; basics.  RISCV version.
> +   Copyright (C) 2023 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/>.  */
> +
> +#ifndef _RISCV_STRING_FZA_H
> +#define _RISCV_STRING_FZA_H 1
> +
> +#ifdef __riscv_zbb
> +/* With bitmap extension we can use orc.b to find all zero bytes.  */
> +# include <string-misc.h>
> +# include <string-optype.h>
> +
> +/* The functions return a byte mask.  */
> +typedef op_t find_t;
> +
> +/* This function returns 0xff for each byte that is zero in X.  */
> +static __always_inline find_t
> +find_zero_all (op_t x)
> +{
> +  find_t r;
> +  asm ("orc.b %0, %1" : "=r" (r) : "r" (x));
> +  return ~r;
> +}
> +
> +/* This function returns 0xff for each byte that is equal between X1 and
> +   X2.  */
> +static __always_inline find_t
> +find_eq_all (op_t x1, op_t x2)
> +{
> +  return find_zero_all (x1 ^ x2);
> +}
> +
> +/* Identify zero bytes in X1 or equality between X1 and X2.  */
> +static __always_inline find_t
> +find_zero_eq_all (op_t x1, op_t x2)
> +{
> +  return find_zero_all (x1) | find_eq_all (x1, x2);
> +}
> +
> +/* Identify zero bytes in X1 or inequality between X1 and X2.  */
> +static __always_inline find_t
> +find_zero_ne_all (op_t x1, op_t x2)
> +{
> +  return find_zero_all (x1) | ~find_eq_all (x1, x2);
> +}
> +
> +/* Define the "inexact" versions in terms of the exact versions.  */
> +# define find_zero_low         find_zero_all
> +# define find_eq_low           find_eq_all
> +# define find_zero_eq_low      find_zero_eq_all
> +# define find_zero_ne_low      find_zero_ne_all
> +#else
> +#include <sysdeps/generic/string-fza.h>
> +#endif
> +
> +#endif /* _RISCV_STRING_FZA_H  */
> diff --git a/sysdeps/riscv/string-fzi.h b/sysdeps/riscv/string-fzi.h
> new file mode 100644
> index 0000000000..3cde113afb
> --- /dev/null
> +++ b/sysdeps/riscv/string-fzi.h
> @@ -0,0 +1,77 @@
> +/* Zero byte detection; indexes.  RISCV version.
> +   Copyright (C) 2023 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/>.  */
> +
> +#ifndef _STRING_RISCV_FZI_H
> +#define _STRING_RISCV_FZI_H 1
> +
> +#ifdef __riscv_zbb
> +# include <sysdeps/generic/string-fzi.h>
> +#else
> +/* Without bitmap clz/ctz extensions, it is faster to direct test the bits
> +   instead of calling compiler auxiliary functions.  */
> +# include <string-optype.h>
> +
> +static __always_inline unsigned int
> +index_first (find_t c)
> +{
> +  if (c & 0x80U)
> +    return 0;
> +  if (c & 0x8000U)
> +    return 1;
> +  if (c & 0x800000U)
> +    return 2;
> +
> +  if (sizeof (op_t) == 4)
> +    return 3;
> +
> +  if (c & 0x80000000U)
> +    return 3;
> +  if (c & 0x8000000000UL)
> +    return 4;
> +  if (c & 0x800000000000UL)
> +    return 5;
> +  if (c & 0x80000000000000UL)
> +    return 6;
> +  return 7;
> +}
> +
> +static __always_inline unsigned int
> +index_last (find_t c)
> +{
> +  if (sizeof (op_t) == 8)
> +    {
> +      if (c & 0x8000000000000000UL)
> +       return 7;
> +      if (c & 0x80000000000000UL)
> +       return 6;
> +      if (c & 0x800000000000UL)
> +       return 5;
> +      if (c & 0x8000000000UL)
> +       return 4;
> +    }
> +  if (c & 0x80000000U)
> +    return 3;
> +  if (c & 0x800000U)
> +    return 2;
> +  if (c & 0x8000U)
> +    return 1;
> +  return 0;
> +}
> +#endif
> +
> +#endif /* STRING_FZI_H */
> --
> 2.34.1
>

When applying:
Applying: riscv: Add string-fza.h and string-fzi.h
.git/rebase-apply/patch:115: trailing whitespace.
   instead of calling compiler auxiliary functions.  */
warning: 1 line adds whitespace errors.
  

On 01/02/23 14:53, Richard Henderson wrote:
> On 2/1/23 07:03, Adhemerval Zanella wrote:
>> +static __always_inline unsigned int
>> +index_first (find_t c)
>> +{
>> +  if (c & 0x80U)
>> +    return 0;
>> +  if (c & 0x8000U)
>> +    return 1;
>> +  if (c & 0x800000U)
>> +    return 2;
>> +
>> +  if (sizeof (op_t) == 4)
>> +    return 3;
>> +
>> +  if (c & 0x80000000U)
>> +    return 3;
>> +  if (c & 0x8000000000UL)
>> +    return 4;
>> +  if (c & 0x800000000000UL)
>> +    return 5;
>> +  if (c & 0x80000000000000UL)
>> +    return 6;
>> +  return 7;
> 
> There is a -mbig-endian switch to gcc, even if that isn't the normal configuration.

I though about adding BE support, however we explicit do not support it
(sysdeps/riscv/bits/endianness.h) so I think we can assume LE for now.

> 
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
  

On 2/2/23 02:30, Adhemerval Zanella Netto wrote:
>> There is a -mbig-endian switch to gcc, even if that isn't the normal configuration.
> 
> I though about adding BE support, however we explicit do not support it
> (sysdeps/riscv/bits/endianness.h) so I think we can assume LE for now.

Fair.

Separate subject for cleanup, I suppose, implementing endianness.h generically in terms of 
compiler pre-defined macro __BYTE_ORDER__ et al.


r~
  

On 2/2/23 05:30, Adhemerval Zanella Netto wrote:
> 
> 
> On 01/02/23 14:53, Richard Henderson wrote:
>> On 2/1/23 07:03, Adhemerval Zanella wrote:
>>> +static __always_inline unsigned int
>>> +index_first (find_t c)
>>> +{
>>> +  if (c & 0x80U)
>>> +    return 0;
>>> +  if (c & 0x8000U)
>>> +    return 1;
>>> +  if (c & 0x800000U)
>>> +    return 2;
>>> +
>>> +  if (sizeof (op_t) == 4)
>>> +    return 3;
>>> +
>>> +  if (c & 0x80000000U)
>>> +    return 3;
>>> +  if (c & 0x8000000000UL)
>>> +    return 4;
>>> +  if (c & 0x800000000000UL)
>>> +    return 5;
>>> +  if (c & 0x80000000000000UL)
>>> +    return 6;
>>> +  return 7;
>>
>> There is a -mbig-endian switch to gcc, even if that isn't the normal configuration.
> 
> I though about adding BE support, however we explicit do not support it
> (sysdeps/riscv/bits/endianness.h) so I think we can assume LE for now.
I wouldn't bother with BE until someone comes asking for it.

Jeff
  

On 2/4/23 06:31, Jeff Law wrote:
> 
> 
> On 2/2/23 05:30, Adhemerval Zanella Netto wrote:
>>
>>
>> On 01/02/23 14:53, Richard Henderson wrote:
>>> On 2/1/23 07:03, Adhemerval Zanella wrote:
>>>> +static __always_inline unsigned int
>>>> +index_first (find_t c)
>>>> +{
>>>> +  if (c & 0x80U)
>>>> +    return 0;
>>>> +  if (c & 0x8000U)
>>>> +    return 1;
>>>> +  if (c & 0x800000U)
>>>> +    return 2;
>>>> +
>>>> +  if (sizeof (op_t) == 4)
>>>> +    return 3;
>>>> +
>>>> +  if (c & 0x80000000U)
>>>> +    return 3;
>>>> +  if (c & 0x8000000000UL)
>>>> +    return 4;
>>>> +  if (c & 0x800000000000UL)
>>>> +    return 5;
>>>> +  if (c & 0x80000000000000UL)
>>>> +    return 6;
>>>> +  return 7;
>>>
>>> There is a -mbig-endian switch to gcc, even if that isn't the normal configuration.
>>
>> I though about adding BE support, however we explicit do not support it
>> (sysdeps/riscv/bits/endianness.h) so I think we can assume LE for now.
> I wouldn't bother with BE until someone comes asking for it.

My only point is there's a compiler switch, and this function is incorrect in that 
context.  One could either #error or not use this specialization.


r~