Message ID | 1515588482-15744-4-git-send-email-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | Improve generic string routines | expand |
On 01/10/2018 04:47 AM, Adhemerval Zanella wrote: > +/* Create a mask with high bit of each byte being 1, and the low 7 bits > + being all the opposite of the input 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 pointer alignment > + of 3 the function returns 0x7f7f7f0000000000 (input meant to > + be 0xffffff0000000000) for BE and 0x00000000007f7f7f for LE (input > + meant to be 0x0000000000ffffff). */ > +static inline op_t > +highbit_mask (op_t m) > +{ > + return m & ~repeat_bytes (0x80); > +} The first line of the comment does not match the implementation, which would have to be something like "return (m ^ repeat_bytes (0x7f)) | repeat_bytes (0x80);" to match the comment. Also, I suggest replacing "~repeat_bytes (0x80)" with "repeat_bytes (0x7f)" as the latter is a bit simpler.
On 10/01/2018 21:24, Paul Eggert wrote: > On 01/10/2018 04:47 AM, Adhemerval Zanella wrote: >> +/* Create a mask with high bit of each byte being 1, and the low 7 bits >> + being all the opposite of the input 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 pointer alignment >> + of 3 the function returns 0x7f7f7f0000000000 (input meant to >> + be 0xffffff0000000000) for BE and 0x00000000007f7f7f for LE (input >> + meant to be 0x0000000000ffffff). */ >> +static inline op_t >> +highbit_mask (op_t m) >> +{ >> + return m & ~repeat_bytes (0x80); >> +} > > The first line of the comment does not match the implementation, which would have to be something like "return (m ^ repeat_bytes (0x7f)) | repeat_bytes (0x80);" to match the comment. Also, I suggest replacing "~repeat_bytes (0x80)" with "repeat_bytes (0x7f)" as the latter is a bit simpler. Thanks, I will change it locally.
On Wed, 10 Jan 2018, Adhemerval Zanella wrote: > +/* 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 Is there a missing "for BE" at the end of this line? -- Joseph S. Myers joseph@codesourcery.com
On 11/01/2018 11:29, Joseph Myers wrote: > On Wed, 10 Jan 2018, Adhemerval Zanella wrote: > >> +/* 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 > > Is there a missing "for BE" at the end of this line? > I applied the following patch based on Paul's suggestion: diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h index 6231798..98e7852 100644 --- a/sysdeps/generic/string-maskoff.h +++ b/sysdeps/generic/string-maskoff.h @@ -47,18 +47,17 @@ repeat_bytes (unsigned char c_in) return ((op_t)-1 / 0xff) * c_in; } -/* Create a mask with high bit of each byte being 1, and the low 7 bits - being all the opposite of the input 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 pointer alignment - of 3 the function returns 0x7f7f7f0000000000 (input meant to - be 0xffffff0000000000) for BE and 0x00000000007f7f7f for LE (input - meant to be 0x0000000000ffffff). */ +/* 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 (0x80); + return m & repeat_bytes (0x7f); } #endif /* STRING_MASKOFF_H */
diff --git a/sysdeps/generic/string-maskoff.h b/sysdeps/generic/string-maskoff.h new file mode 100644 index 0000000..6231798 --- /dev/null +++ b/sysdeps/generic/string-maskoff.h @@ -0,0 +1,64 @@ +/* Mask off bits. Generic C version. + Copyright (C) 2018 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 <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; +} + +/* Create a mask with high bit of each byte being 1, and the low 7 bits + being all the opposite of the input 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 pointer alignment + of 3 the function returns 0x7f7f7f0000000000 (input meant to + be 0xffffff0000000000) for BE and 0x00000000007f7f7f for LE (input + meant to be 0x0000000000ffffff). */ +static inline op_t +highbit_mask (op_t m) +{ + return m & ~repeat_bytes (0x80); +} + +#endif /* STRING_MASKOFF_H */