[v5,03/17] Add string-maskoff.h generic header

  Macros to operate on unaligned access for string operations:

  - create_mask: create a mask based on pointer alignment to sets up
    non-zero bytes before the beginning of the word so a following
    operation (such as find zero) might ignore these bytes.

  - highbit_mask: create a mask with high bit of each byte being 1,
    and the low 7 bits being all the opposite of the input.

These macros are meant to be used on optimized vectorized string
implementations.
---
 sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 sysdeps/generic/string-maskoff.h
  

On Mon, Sep 19, 2022 at 04:59:06PM -0300, Adhemerval Zanella via Libc-alpha wrote:
> Macros to operate on unaligned access for string operations:
> 
>   - create_mask: create a mask based on pointer alignment to sets up
>     non-zero bytes before the beginning of the word so a following
>     operation (such as find zero) might ignore these bytes.
> 
>   - highbit_mask: create a mask with high bit of each byte being 1,
>     and the low 7 bits being all the opposite of the input.

I really appreciate the effort you've put into documenting the purpose
of each function! It's really awesome to reach such nice coments. Thank
you for that. I've gone through this to review the implementation and
the descriptions. I think it needs a little more tweaking.

> These macros are meant to be used on optimized vectorized string
> implementations.
> ---
>  sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 sysdeps/generic/string-maskoff.h
> 
> diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h
> new file mode 100644
> index 0000000000..831647bda6
> --- /dev/null
> +++ b/sysdeps/generic/string-maskoff.h
> @@ -0,0 +1,73 @@
> +/* Mask off bits.  Generic C version.
> +   Copyright (C) 2022 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_MASKOFF_H
> +#define _STRING_MASKOFF_H 1
> +
> +#include <endian.h>
> +#include <limits.h>
> +#include <stdint.h>
> +#include <string-optype.h>
> +
> +/* Provide a mask based on the pointer alignment that sets up non-zero
> +   bytes before the beginning of the word.  It is used to mask off
> +   undesirable bits from an aligned read from an unaligned pointer.
> +   For instance, on a 64 bits machine with a pointer alignment of

s/bits/-bit/g

While it is technically correct English to say "A 64-bits machine", this
is not the normative usage.

I suggest we use the normative "64-bit machine." We can talk about 64
bits, and alignment as bits etc.

> +   3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000
> +   (meaning to mask off the initial 3 bytes).  */

Missing "for BE" ?

> +static inline op_t
> +create_mask (uintptr_t i)
> +{
> +  i = i % sizeof (op_t);

OK. Wrap the value.

> +  if (__BYTE_ORDER == __LITTLE_ENDIAN)
> +    return ~(((op_t)-1) << (i * CHAR_BIT));
> +  else
> +    return ~(((op_t)-1) >> (i * CHAR_BIT));

OK. Shift.

> +}
> +
> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits

s/an/a/g
s/bits/-bit/g

> +   machine with input as 0xce the functions returns 0xcececececececece.  */
> +static inline op_t
> +repeat_bytes (unsigned char c_in)
> +{
> +  return ((op_t)-1 / 0xff) * c_in;
> +}

How does the compiler do here on the various architectures to produce
the deposit/expand instructions that could be used for this operation?

aarch64 gcc trunk:
        ldrb    r3, [r7, #7]    @ zero_extendqisi2
        mov     r2, #16843009
        mul     r3, r2, r3

x86_64 gcc trunk:
        movzx   eax, BYTE PTR [rbp-4]
        imul    eax, eax, 16843009

s390x gcc12:
	ic      %r1,167(%r11)
        lhi     %r2,255
        nr      %r1,%r2
        ms      %r1,.L4-.L3(%r5)
        llgfr   %r1,%r1

Looks OK, and the static inline will get optimized with the rest of
the operations.

> +
> +/* Based on mask created by 'create_mask', mask off the high bit of each

s/on/on a/g

> +   byte in the mask.  It is used to mask off undesirable bits from an
> +   aligned read from an unaligned pointer, and also taking care to avoid

s/and/while/g

> +   match possible bytes meant to be matched.  For instance, on a 64 bits

Suggest:
matching possible bytes not meant to be matched.

s/bits/-bits/g

> +   machine with a mask created from a pointer with an alignment of 3
> +   (0x0000000000ffffff) the function returns 0x7f7f7f0000000000 for BE
> +   and 0x00000000007f7f7f for LE.  */
> +static inline op_t
> +highbit_mask (op_t m)
> +{
> +  return m & repeat_bytes (0x7f);

OK.

> +}
> +
> +/* Return the address of the op_t word containing the address P.  For
> +   instance on address 0x0011223344556677 and op_t with size of 8,
> +   it returns 0x0011223344556670.  */

Could you expand on this a bit more? It's a bit opaque what we might use
this for (I have some ideas).

> +static inline op_t *
> +word_containing (char const *p)
> +{
> +  return (op_t *) (p - (uintptr_t) p % sizeof (op_t));
> +}
> +
> +#endif /* _STRING_MASKOFF_H  */
> -- 
> 2.34.1
>
  

On 20/09/22 08:43, Carlos O'Donell wrote:
> On Mon, Sep 19, 2022 at 04:59:06PM -0300, Adhemerval Zanella via Libc-alpha wrote:
>> Macros to operate on unaligned access for string operations:
>>
>>   - create_mask: create a mask based on pointer alignment to sets up
>>     non-zero bytes before the beginning of the word so a following
>>     operation (such as find zero) might ignore these bytes.
>>
>>   - highbit_mask: create a mask with high bit of each byte being 1,
>>     and the low 7 bits being all the opposite of the input.
> 
> I really appreciate the effort you've put into documenting the purpose
> of each function! It's really awesome to reach such nice coments. Thank
> you for that. I've gone through this to review the implementation and
> the descriptions. I think it needs a little more tweaking.
> 
>> These macros are meant to be used on optimized vectorized string
>> implementations.
>> ---
>>  sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
>>  1 file changed, 73 insertions(+)
>>  create mode 100644 sysdeps/generic/string-maskoff.h
>>
>> diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h
>> new file mode 100644
>> index 0000000000..831647bda6
>> --- /dev/null
>> +++ b/sysdeps/generic/string-maskoff.h
>> @@ -0,0 +1,73 @@
>> +/* Mask off bits.  Generic C version.
>> +   Copyright (C) 2022 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_MASKOFF_H
>> +#define _STRING_MASKOFF_H 1
>> +
>> +#include <endian.h>
>> +#include <limits.h>
>> +#include <stdint.h>
>> +#include <string-optype.h>
>> +
>> +/* Provide a mask based on the pointer alignment that sets up non-zero
>> +   bytes before the beginning of the word.  It is used to mask off
>> +   undesirable bits from an aligned read from an unaligned pointer.
>> +   For instance, on a 64 bits machine with a pointer alignment of
> 
> s/bits/-bit/g
> 
> While it is technically correct English to say "A 64-bits machine", this
> is not the normative usage.
> 
> I suggest we use the normative "64-bit machine." We can talk about 64
> bits, and alignment as bits etc.

Alright.

> 
>> +   3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000
>> +   (meaning to mask off the initial 3 bytes).  */
> 
> Missing "for BE" ?

Ack.

> 
>> +static inline op_t
>> +create_mask (uintptr_t i)
>> +{
>> +  i = i % sizeof (op_t);
> 
> OK. Wrap the value.
> 
>> +  if (__BYTE_ORDER == __LITTLE_ENDIAN)
>> +    return ~(((op_t)-1) << (i * CHAR_BIT));
>> +  else
>> +    return ~(((op_t)-1) >> (i * CHAR_BIT));
> 
> OK. Shift.
> 
>> +}
>> +
>> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits
> 
> s/an/a/g
> s/bits/-bit/g

Ack.

> 
>> +   machine with input as 0xce the functions returns 0xcececececececece.  */
>> +static inline op_t
>> +repeat_bytes (unsigned char c_in)
>> +{
>> +  return ((op_t)-1 / 0xff) * c_in;
>> +}
> 
> How does the compiler do here on the various architectures to produce
> the deposit/expand instructions that could be used for this operation?
> 
> aarch64 gcc trunk:
>         ldrb    r3, [r7, #7]    @ zero_extendqisi2
>         mov     r2, #16843009
>         mul     r3, r2, r3
> 
> x86_64 gcc trunk:
>         movzx   eax, BYTE PTR [rbp-4]
>         imul    eax, eax, 16843009
> 
> s390x gcc12:
> 	ic      %r1,167(%r11)
>         lhi     %r2,255
>         nr      %r1,%r2
>         ms      %r1,.L4-.L3(%r5)
>         llgfr   %r1,%r1
> 
> Looks OK, and the static inline will get optimized with the rest of
> the operations.
> 
>> +
>> +/* Based on mask created by 'create_mask', mask off the high bit of each
> 
> s/on/on a/g

Ack.

> 
>> +   byte in the mask.  It is used to mask off undesirable bits from an
>> +   aligned read from an unaligned pointer, and also taking care to avoid
> 
> s/and/while/g

Ack.

> 
>> +   match possible bytes meant to be matched.  For instance, on a 64 bits
> 
> Suggest:
> matching possible bytes not meant to be matched.
> 
> s/bits/-bits/g

Ack.

> 
>> +   machine with a mask created from a pointer with an alignment of 3
>> +   (0x0000000000ffffff) the function returns 0x7f7f7f0000000000 for BE
>> +   and 0x00000000007f7f7f for LE.  */
>> +static inline op_t
>> +highbit_mask (op_t m)
>> +{
>> +  return m & repeat_bytes (0x7f);
> 
> OK.
> 
>> +}
>> +
>> +/* Return the address of the op_t word containing the address P.  For
>> +   instance on address 0x0011223344556677 and op_t with size of 8,
>> +   it returns 0x0011223344556670.  */
> 
> Could you expand on this a bit more? It's a bit opaque what we might use
> this for (I have some ideas).

Maybe: 

/* Return the word aligned address containing the address P.  For instance for
   the address 0x0011223344556677 with op_t with size of 8, it returns
   0x0011223344556670.  */

> 
>> +static inline op_t *
>> +word_containing (char const *p)
>> +{
>> +  return (op_t *) (p - (uintptr_t) p % sizeof (op_t));
>> +}
>> +
>> +#endif /* _STRING_MASKOFF_H  */
>> -- 
>> 2.34.1
>>
>
  

On Mon, Sep 19, 2022 at 12:59 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Macros to operate on unaligned access for string operations:
>
>   - create_mask: create a mask based on pointer alignment to sets up
>     non-zero bytes before the beginning of the word so a following
>     operation (such as find zero) might ignore these bytes.
>
>   - highbit_mask: create a mask with high bit of each byte being 1,
>     and the low 7 bits being all the opposite of the input.
>
> These macros are meant to be used on optimized vectorized string
> implementations.
> ---
>  sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 sysdeps/generic/string-maskoff.h
>
> diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h
> new file mode 100644
> index 0000000000..831647bda6
> --- /dev/null
> +++ b/sysdeps/generic/string-maskoff.h
> @@ -0,0 +1,73 @@
> +/* Mask off bits.  Generic C version.
> +   Copyright (C) 2022 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_MASKOFF_H
> +#define _STRING_MASKOFF_H 1
> +
> +#include <endian.h>
> +#include <limits.h>
> +#include <stdint.h>
> +#include <string-optype.h>
> +
> +/* Provide a mask based on the pointer alignment that sets up non-zero
> +   bytes before the beginning of the word.  It is used to mask off
> +   undesirable bits from an aligned read from an unaligned pointer.
> +   For instance, on a 64 bits machine with a pointer alignment of
> +   3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000
> +   (meaning to mask off the initial 3 bytes).  */
> +static inline op_t
> +create_mask (uintptr_t i)
> +{
> +  i = i % sizeof (op_t);
> +  if (__BYTE_ORDER == __LITTLE_ENDIAN)
> +    return ~(((op_t)-1) << (i * CHAR_BIT));
> +  else
> +    return ~(((op_t)-1) >> (i * CHAR_BIT));
> +}
> +
> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits
> +   machine with input as 0xce the functions returns 0xcececececececece.  */
> +static inline op_t
> +repeat_bytes (unsigned char c_in)
> +{
> +  return ((op_t)-1 / 0xff) * c_in;
> +}
> +
> +/* Based on mask created by 'create_mask', mask off the high bit of each
> +   byte in the mask.  It is used to mask off undesirable bits from an
> +   aligned read from an unaligned pointer, and also taking care to avoid
> +   match possible bytes meant to be matched.  For instance, on a 64 bits
> +   machine with a mask created from a pointer with an alignment of 3
> +   (0x0000000000ffffff) the function returns 0x7f7f7f0000000000 for BE
> +   and 0x00000000007f7f7f for LE.  */
> +static inline op_t
> +highbit_mask (op_t m)
> +{
> +  return m & repeat_bytes (0x7f);
> +}
> +
> +/* Return the address of the op_t word containing the address P.  For
> +   instance on address 0x0011223344556677 and op_t with size of 8,
> +   it returns 0x0011223344556670.  */
> +static inline op_t *
> +word_containing (char const *p)
> +{
> +  return (op_t *) (p - (uintptr_t) p % sizeof (op_t));

This can just be (p & (-sizeof(p)) I think.
Other than that look goods.
> +}
> +
> +#endif /* _STRING_MASKOFF_H  */
> --
> 2.34.1
>
  

On 1/5/23 23:49, Noah Goldstein via Libc-alpha wrote:
> On Mon, Sep 19, 2022 at 12:59 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> Macros to operate on unaligned access for string operations:
>>
>>    - create_mask: create a mask based on pointer alignment to sets up
>>      non-zero bytes before the beginning of the word so a following
>>      operation (such as find zero) might ignore these bytes.
>>
>>    - highbit_mask: create a mask with high bit of each byte being 1,
>>      and the low 7 bits being all the opposite of the input.
>>
>> These macros are meant to be used on optimized vectorized string
>> implementations.
>> ---
>>   sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
>>   1 file changed, 73 insertions(+)
>>   create mode 100644 sysdeps/generic/string-maskoff.h
>>
>> diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h
>> new file mode 100644
>> index 0000000000..831647bda6
>> --- /dev/null
>> +++ b/sysdeps/generic/string-maskoff.h
>> @@ -0,0 +1,73 @@
>> +/* Mask off bits.  Generic C version.
>> +   Copyright (C) 2022 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_MASKOFF_H
>> +#define _STRING_MASKOFF_H 1
>> +
>> +#include <endian.h>
>> +#include <limits.h>
>> +#include <stdint.h>
>> +#include <string-optype.h>
>> +
>> +/* Provide a mask based on the pointer alignment that sets up non-zero
>> +   bytes before the beginning of the word.  It is used to mask off
>> +   undesirable bits from an aligned read from an unaligned pointer.
>> +   For instance, on a 64 bits machine with a pointer alignment of
>> +   3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000
>> +   (meaning to mask off the initial 3 bytes).  */
>> +static inline op_t
>> +create_mask (uintptr_t i)
>> +{
>> +  i = i % sizeof (op_t);
>> +  if (__BYTE_ORDER == __LITTLE_ENDIAN)
>> +    return ~(((op_t)-1) << (i * CHAR_BIT));
>> +  else
>> +    return ~(((op_t)-1) >> (i * CHAR_BIT));
>> +}
>> +
>> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits
>> +   machine with input as 0xce the functions returns 0xcececececececece.  */
>> +static inline op_t

Hi Adhemerval and Noah,

I don't know what is the minimum C version for compiling glibc, but if you can 
ignore C89, I would propose something:

'static inline' should be restricted to .c files, since if the compiler decides 
to not inline and you have it in a header, you end up with multiple static 
definitions for the same code.

In headers, I use C99 inline, which doesn't emit any object code when the 
compiler decides to not inline.  Then in a .c file, you add a prototype using 
'extern inline', and the compiler will emit code there, exactly once.

Even if you have to support C89, I'd use [[gnu::always_inline]] together with 
'static inline', to make sure that the compiler doesn't do nefarious stuff.

Cheers,

Alex

-- 
<http://www.alejandro-colomar.es/>
  

On 05/01/23 19:49, Noah Goldstein wrote:
> On Mon, Sep 19, 2022 at 12:59 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> Macros to operate on unaligned access for string operations:
>>
>>   - create_mask: create a mask based on pointer alignment to sets up
>>     non-zero bytes before the beginning of the word so a following
>>     operation (such as find zero) might ignore these bytes.
>>
>>   - highbit_mask: create a mask with high bit of each byte being 1,
>>     and the low 7 bits being all the opposite of the input.
>>
>> These macros are meant to be used on optimized vectorized string
>> implementations.
>> ---
>>  sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
>>  1 file changed, 73 insertions(+)
>>  create mode 100644 sysdeps/generic/string-maskoff.h
>>
>> diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h
>> new file mode 100644
>> index 0000000000..831647bda6
>> --- /dev/null
>> +++ b/sysdeps/generic/string-maskoff.h
>> @@ -0,0 +1,73 @@
>> +/* Mask off bits.  Generic C version.
>> +   Copyright (C) 2022 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_MASKOFF_H
>> +#define _STRING_MASKOFF_H 1
>> +
>> +#include <endian.h>
>> +#include <limits.h>
>> +#include <stdint.h>
>> +#include <string-optype.h>
>> +
>> +/* Provide a mask based on the pointer alignment that sets up non-zero
>> +   bytes before the beginning of the word.  It is used to mask off
>> +   undesirable bits from an aligned read from an unaligned pointer.
>> +   For instance, on a 64 bits machine with a pointer alignment of
>> +   3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000
>> +   (meaning to mask off the initial 3 bytes).  */
>> +static inline op_t
>> +create_mask (uintptr_t i)
>> +{
>> +  i = i % sizeof (op_t);
>> +  if (__BYTE_ORDER == __LITTLE_ENDIAN)
>> +    return ~(((op_t)-1) << (i * CHAR_BIT));
>> +  else
>> +    return ~(((op_t)-1) >> (i * CHAR_BIT));
>> +}
>> +
>> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits
>> +   machine with input as 0xce the functions returns 0xcececececececece.  */
>> +static inline op_t
>> +repeat_bytes (unsigned char c_in)
>> +{
>> +  return ((op_t)-1 / 0xff) * c_in;
>> +}
>> +
>> +/* Based on mask created by 'create_mask', mask off the high bit of each
>> +   byte in the mask.  It is used to mask off undesirable bits from an
>> +   aligned read from an unaligned pointer, and also taking care to avoid
>> +   match possible bytes meant to be matched.  For instance, on a 64 bits
>> +   machine with a mask created from a pointer with an alignment of 3
>> +   (0x0000000000ffffff) the function returns 0x7f7f7f0000000000 for BE
>> +   and 0x00000000007f7f7f for LE.  */
>> +static inline op_t
>> +highbit_mask (op_t m)
>> +{
>> +  return m & repeat_bytes (0x7f);
>> +}
>> +
>> +/* Return the address of the op_t word containing the address P.  For
>> +   instance on address 0x0011223344556677 and op_t with size of 8,
>> +   it returns 0x0011223344556670.  */
>> +static inline op_t *
>> +word_containing (char const *p)
>> +{
>> +  return (op_t *) (p - (uintptr_t) p % sizeof (op_t));
> 
> This can just be (p & (-sizeof(p)) I think.

Indeed it is simpler.

> Other than that look goods.

Thanks.
  

On 05/01/23 20:26, Alejandro Colomar wrote:
> 
> 
> On 1/5/23 23:49, Noah Goldstein via Libc-alpha wrote:
>> On Mon, Sep 19, 2022 at 12:59 PM Adhemerval Zanella via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>> Macros to operate on unaligned access for string operations:
>>>
>>>    - create_mask: create a mask based on pointer alignment to sets up
>>>      non-zero bytes before the beginning of the word so a following
>>>      operation (such as find zero) might ignore these bytes.
>>>
>>>    - highbit_mask: create a mask with high bit of each byte being 1,
>>>      and the low 7 bits being all the opposite of the input.
>>>
>>> These macros are meant to be used on optimized vectorized string
>>> implementations.
>>> ---
>>>   sysdeps/generic/string-maskoff.h | 73 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 73 insertions(+)
>>>   create mode 100644 sysdeps/generic/string-maskoff.h
>>>
>>> diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h
>>> new file mode 100644
>>> index 0000000000..831647bda6
>>> --- /dev/null
>>> +++ b/sysdeps/generic/string-maskoff.h
>>> @@ -0,0 +1,73 @@
>>> +/* Mask off bits.  Generic C version.
>>> +   Copyright (C) 2022 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_MASKOFF_H
>>> +#define _STRING_MASKOFF_H 1
>>> +
>>> +#include <endian.h>
>>> +#include <limits.h>
>>> +#include <stdint.h>
>>> +#include <string-optype.h>
>>> +
>>> +/* Provide a mask based on the pointer alignment that sets up non-zero
>>> +   bytes before the beginning of the word.  It is used to mask off
>>> +   undesirable bits from an aligned read from an unaligned pointer.
>>> +   For instance, on a 64 bits machine with a pointer alignment of
>>> +   3 the function returns 0x0000000000ffffff for LE and 0xffffff0000000000
>>> +   (meaning to mask off the initial 3 bytes).  */
>>> +static inline op_t
>>> +create_mask (uintptr_t i)
>>> +{
>>> +  i = i % sizeof (op_t);
>>> +  if (__BYTE_ORDER == __LITTLE_ENDIAN)
>>> +    return ~(((op_t)-1) << (i * CHAR_BIT));
>>> +  else
>>> +    return ~(((op_t)-1) >> (i * CHAR_BIT));
>>> +}
>>> +
>>> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits
>>> +   machine with input as 0xce the functions returns 0xcececececececece.  */
>>> +static inline op_t
> 
> Hi Adhemerval and Noah,
> 
> I don't know what is the minimum C version for compiling glibc, but if you can ignore C89, I would propose something:
> 
> 'static inline' should be restricted to .c files, since if the compiler decides to not inline and you have it in a header, you end up with multiple static definitions for the same code.
> 
> In headers, I use C99 inline, which doesn't emit any object code when the compiler decides to not inline.  Then in a .c file, you add a prototype using 'extern inline', and the compiler will emit code there, exactly once.
> 
> Even if you have to support C89, I'd use [[gnu::always_inline]] together with 'static inline', to make sure that the compiler doesn't do nefarious stuff.

Although we build glibc with -std=gnu11 we also uses -fgnu89-inline, but regardless of
the inline mode 'multiple definitions' it is usually not an issue.  We do use
__always_inline in performant code, so it should be ok to use it here as well (although
I would expect that compiler will always inline these short functions).