Message ID | 20230110210106.1457686-15-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | None | expand |
On 1/10/23 13:01, Adhemerval Zanella wrote: > From: Richard Henderson <richard.henderson@linaro.org> > > While alpha has the more important string functions in assembly, > there are still a few for find the generic routines are used. > > Use the CMPBGE insn, via the builtin, for testing of zeros. Use a > simplified expansion of __builtin_ctz when the insn isn't available. > > Checked on alpha-linux-gnu. > --- > sysdeps/alpha/string-fzb.h | 52 +++++++++++++++++ > sysdeps/alpha/string-fzi.h | 113 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 165 insertions(+) > create mode 100644 sysdeps/alpha/string-fzb.h > create mode 100644 sysdeps/alpha/string-fzi.h > > diff --git a/sysdeps/alpha/string-fzb.h b/sysdeps/alpha/string-fzb.h > new file mode 100644 > index 0000000000..e3934ba413 > --- /dev/null > +++ b/sysdeps/alpha/string-fzb.h > @@ -0,0 +1,52 @@ > +/* Zero byte detection; boolean. Alpha 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_FZB_H > +#define _STRING_FZB_H 1 > + > +#include <sys/cdefs.h> > +#include <string-optype.h> > + > +/* Note that since CMPBGE creates a bit mask rather than a byte mask, > + we cannot simply provide a target-specific string-fza.h. */ > + > +/* Determine if any byte within X is zero. This is a pure boolean test. */ > + > +static __always_inline _Bool > +has_zero (op_t x) > +{ > + return __builtin_alpha_cmpbge (0, x) != 0; > +} > + > +/* Likewise, but for byte equality between X1 and X2. */ > + > +static __always_inline _Bool > +has_eq (op_t x1, op_t x2) > +{ > + return has_zero (x1 ^ x2); > +} > + > +/* Likewise, but for zeros in X1 and equal bytes between X1 and X2. */ > + > +static __always_inline _Bool > +has_zero_eq (op_t x1, op_t x2) > +{ > + return has_zero (x1) | has_eq (x1, x2); > +} > + > +#endif /* _STRING_FZB_H */ > diff --git a/sysdeps/alpha/string-fzi.h b/sysdeps/alpha/string-fzi.h > new file mode 100644 > index 0000000000..bc2f0bdc91 > --- /dev/null > +++ b/sysdeps/alpha/string-fzi.h > @@ -0,0 +1,113 @@ > +/* string-fzi.h -- zero byte detection; indices. Alpha 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_FZI_H > +#define _STRING_FZI_H > + > +#include <limits.h> > +#include <string-optype.h> > + > +/* Note that since CMPBGE creates a bit mask rather than a byte mask, > + we cannot simply provide a target-specific string-fza.h. */ > + > +/* A subroutine for the index_zero functions. Given a bitmask C, > + return the index of the first bit set in memory order. */ > + > +static __always_inline unsigned int > +index_first (unsigned long int c) > +{ > +#ifdef __alpha_cix__ > + return __builtin_ctzl (c); > +#else > + c = c & -c; > + return (c & 0xf0 ? 4 : 0) + (c & 0xcc ? 2 : 0) + (c & 0xaa ? 1 : 0); > +#endif > +} > + > +/* Similarly, but return the (memory order) index of the last bit > + that is non-zero. Note that only the least 8 bits may be nonzero. */ > + > +static __always_inline unsigned int > +index_last (unsigned long int x) > +{ > +#ifdef __alpha_cix__ > + return __builtin_clzl (x) ^ 63; > +#else > + unsigned r = 0; > + if (x & 0xf0) > + r += 4; > + if (x & (0xc << r)) > + r += 2; > + if (x & (0x2 << r)) > + r += 1; > + return r; > +#endif > +} > + > +/* Given a word X that is known to contain a zero byte, return the > + index of the first such within the word in memory order. */ > + > +static __always_inline unsigned int > +index_first_zero (op_t x) > +{ > + return index_first (__builtin_alpha_cmpbge (0, x)); The header split has drifted somewhat since the original: string-fza.h is missing and check_mask() seems misplaced. In particular, from strchrnul, + op_t mask = check_mask (find_zero_eq_all (word, repeated_c), s_int); + if (mask != 0) + return (char *) str + index_first (mask); We're using the generic find_zero_eq_all, then (correctly, for the moment) the generic check_mask, and then incorrectly the alpha-specific index_first. We want to use an alpha-specific find_zero_eq_all (using cmpbge), an alpha-specific check_mask (shifting by bits, not bytes), and an alpha-specific index_first (searching only 8 low bits, not a full ctz). I wonder if it makes sense to introduce a target-specific find_t, making it clear that it's not the same type as op_t (for alpha, int instead of unsigned long (uint8_t having it's own issues on ancient alpha ev5)). I think check_mask (possibly renamed shift_find?) should belong in string-fza.h to match the implementation therein, or within a new header (possibly along with find_t?). Anyway, even the current set of headers would work, so long as Alpha implements all of them. r~
On 10/01/23 23:45, Richard Henderson wrote: > On 1/10/23 13:01, Adhemerval Zanella wrote: >> From: Richard Henderson <richard.henderson@linaro.org> >> >> While alpha has the more important string functions in assembly, >> there are still a few for find the generic routines are used. >> >> Use the CMPBGE insn, via the builtin, for testing of zeros. Use a >> simplified expansion of __builtin_ctz when the insn isn't available. >> >> Checked on alpha-linux-gnu. >> --- >> sysdeps/alpha/string-fzb.h | 52 +++++++++++++++++ >> sysdeps/alpha/string-fzi.h | 113 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 165 insertions(+) >> create mode 100644 sysdeps/alpha/string-fzb.h >> create mode 100644 sysdeps/alpha/string-fzi.h >> >> diff --git a/sysdeps/alpha/string-fzb.h b/sysdeps/alpha/string-fzb.h >> new file mode 100644 >> index 0000000000..e3934ba413 >> --- /dev/null >> +++ b/sysdeps/alpha/string-fzb.h >> @@ -0,0 +1,52 @@ >> +/* Zero byte detection; boolean. Alpha 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_FZB_H >> +#define _STRING_FZB_H 1 >> + >> +#include <sys/cdefs.h> >> +#include <string-optype.h> >> + >> +/* Note that since CMPBGE creates a bit mask rather than a byte mask, >> + we cannot simply provide a target-specific string-fza.h. */ >> + >> +/* Determine if any byte within X is zero. This is a pure boolean test. */ >> + >> +static __always_inline _Bool >> +has_zero (op_t x) >> +{ >> + return __builtin_alpha_cmpbge (0, x) != 0; >> +} >> + >> +/* Likewise, but for byte equality between X1 and X2. */ >> + >> +static __always_inline _Bool >> +has_eq (op_t x1, op_t x2) >> +{ >> + return has_zero (x1 ^ x2); >> +} >> + >> +/* Likewise, but for zeros in X1 and equal bytes between X1 and X2. */ >> + >> +static __always_inline _Bool >> +has_zero_eq (op_t x1, op_t x2) >> +{ >> + return has_zero (x1) | has_eq (x1, x2); >> +} >> + >> +#endif /* _STRING_FZB_H */ >> diff --git a/sysdeps/alpha/string-fzi.h b/sysdeps/alpha/string-fzi.h >> new file mode 100644 >> index 0000000000..bc2f0bdc91 >> --- /dev/null >> +++ b/sysdeps/alpha/string-fzi.h >> @@ -0,0 +1,113 @@ >> +/* string-fzi.h -- zero byte detection; indices. Alpha 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_FZI_H >> +#define _STRING_FZI_H >> + >> +#include <limits.h> >> +#include <string-optype.h> >> + >> +/* Note that since CMPBGE creates a bit mask rather than a byte mask, >> + we cannot simply provide a target-specific string-fza.h. */ >> + >> +/* A subroutine for the index_zero functions. Given a bitmask C, >> + return the index of the first bit set in memory order. */ >> + >> +static __always_inline unsigned int >> +index_first (unsigned long int c) >> +{ >> +#ifdef __alpha_cix__ >> + return __builtin_ctzl (c); >> +#else >> + c = c & -c; >> + return (c & 0xf0 ? 4 : 0) + (c & 0xcc ? 2 : 0) + (c & 0xaa ? 1 : 0); >> +#endif >> +} >> + >> +/* Similarly, but return the (memory order) index of the last bit >> + that is non-zero. Note that only the least 8 bits may be nonzero. */ >> + >> +static __always_inline unsigned int >> +index_last (unsigned long int x) >> +{ >> +#ifdef __alpha_cix__ >> + return __builtin_clzl (x) ^ 63; >> +#else >> + unsigned r = 0; >> + if (x & 0xf0) >> + r += 4; >> + if (x & (0xc << r)) >> + r += 2; >> + if (x & (0x2 << r)) >> + r += 1; >> + return r; >> +#endif >> +} >> + >> +/* Given a word X that is known to contain a zero byte, return the >> + index of the first such within the word in memory order. */ >> + >> +static __always_inline unsigned int >> +index_first_zero (op_t x) >> +{ >> + return index_first (__builtin_alpha_cmpbge (0, x)); > > The header split has drifted somewhat since the original: string-fza.h is missing and check_mask() seems misplaced. > > In particular, from strchrnul, > > > + op_t mask = check_mask (find_zero_eq_all (word, repeated_c), s_int); > + if (mask != 0) > + return (char *) str + index_first (mask); > > > We're using the generic find_zero_eq_all, then (correctly, for the moment) the generic check_mask, and then incorrectly the alpha-specific index_first. > > We want to use an alpha-specific find_zero_eq_all (using cmpbge), an alpha-specific check_mask (shifting by bits, not bytes), and an alpha-specific index_first (searching only 8 low bits, not a full ctz). Right, I had to consult the alpha manual to remind that cmpbge sets the lower 8 bits of the target register. And indeed running on qemu-users triggers a lot of issues. > > I wonder if it makes sense to introduce a target-specific find_t, making it clear that it's not the same type as op_t (for alpha, int instead of unsigned long (uint8_t having it's own issues on ancient alpha ev5)). I think it makes sense, I will add this change. > > I think check_mask (possibly renamed shift_find?) should belong in string-fza.h to match the implementation therein, or within a new header (possibly along with find_t?). It also make sense. > > Anyway, even the current set of headers would work, so long as Alpha implements all of them. A alpha-specific string-fza.h is in fact quite simple, since we do not need to implement all the function. I will update the patch with your suggestions.
diff --git a/sysdeps/alpha/string-fzb.h b/sysdeps/alpha/string-fzb.h new file mode 100644 index 0000000000..e3934ba413 --- /dev/null +++ b/sysdeps/alpha/string-fzb.h @@ -0,0 +1,52 @@ +/* Zero byte detection; boolean. Alpha 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_FZB_H +#define _STRING_FZB_H 1 + +#include <sys/cdefs.h> +#include <string-optype.h> + +/* Note that since CMPBGE creates a bit mask rather than a byte mask, + we cannot simply provide a target-specific string-fza.h. */ + +/* Determine if any byte within X is zero. This is a pure boolean test. */ + +static __always_inline _Bool +has_zero (op_t x) +{ + return __builtin_alpha_cmpbge (0, x) != 0; +} + +/* Likewise, but for byte equality between X1 and X2. */ + +static __always_inline _Bool +has_eq (op_t x1, op_t x2) +{ + return has_zero (x1 ^ x2); +} + +/* Likewise, but for zeros in X1 and equal bytes between X1 and X2. */ + +static __always_inline _Bool +has_zero_eq (op_t x1, op_t x2) +{ + return has_zero (x1) | has_eq (x1, x2); +} + +#endif /* _STRING_FZB_H */ diff --git a/sysdeps/alpha/string-fzi.h b/sysdeps/alpha/string-fzi.h new file mode 100644 index 0000000000..bc2f0bdc91 --- /dev/null +++ b/sysdeps/alpha/string-fzi.h @@ -0,0 +1,113 @@ +/* string-fzi.h -- zero byte detection; indices. Alpha 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_FZI_H +#define _STRING_FZI_H + +#include <limits.h> +#include <string-optype.h> + +/* Note that since CMPBGE creates a bit mask rather than a byte mask, + we cannot simply provide a target-specific string-fza.h. */ + +/* A subroutine for the index_zero functions. Given a bitmask C, + return the index of the first bit set in memory order. */ + +static __always_inline unsigned int +index_first (unsigned long int c) +{ +#ifdef __alpha_cix__ + return __builtin_ctzl (c); +#else + c = c & -c; + return (c & 0xf0 ? 4 : 0) + (c & 0xcc ? 2 : 0) + (c & 0xaa ? 1 : 0); +#endif +} + +/* Similarly, but return the (memory order) index of the last bit + that is non-zero. Note that only the least 8 bits may be nonzero. */ + +static __always_inline unsigned int +index_last (unsigned long int x) +{ +#ifdef __alpha_cix__ + return __builtin_clzl (x) ^ 63; +#else + unsigned r = 0; + if (x & 0xf0) + r += 4; + if (x & (0xc << r)) + r += 2; + if (x & (0x2 << r)) + r += 1; + return r; +#endif +} + +/* Given a word X that is known to contain a zero byte, return the + index of the first such within the word in memory order. */ + +static __always_inline unsigned int +index_first_zero (op_t x) +{ + return index_first (__builtin_alpha_cmpbge (0, x)); +} + +/* Similarly, but perform the test for byte equality between X1 and X2. */ + +static __always_inline unsigned int +index_first_eq (op_t x1, op_t x2) +{ + return index_first_zero (x1 ^ x2); +} + +/* Similarly, but perform the search for zero within X1 or + equality between X1 and X2. */ + +static __always_inline unsigned int +index_first_zero_eq (op_t x1, op_t x2) +{ + return index_first (__builtin_alpha_cmpbge (0, x1) + | __builtin_alpha_cmpbge (0, x1 ^ x2)); +} + +/* Similarly, but perform the search for zero within X1 or + inequality between X1 and X2. */ + +static __always_inline unsigned int +index_first_zero_ne (op_t x1, op_t x2) +{ + return index_first (__builtin_alpha_cmpbge (0, x1) + | (__builtin_alpha_cmpbge (0, x1 ^ x2) ^ 0xFF)); +} + +/* Similarly, but search for the last zero within X. */ + +static __always_inline unsigned int +index_last_zero (op_t x) +{ + return index_last (__builtin_alpha_cmpbge (0, x)); +} + +static __always_inline unsigned int +index_last_eq (op_t x1, op_t x2) +{ + return index_last_zero (x1 ^ x2); +} + +#endif /* _STRING_FZI_H */
From: Richard Henderson <richard.henderson@linaro.org> While alpha has the more important string functions in assembly, there are still a few for find the generic routines are used. Use the CMPBGE insn, via the builtin, for testing of zeros. Use a simplified expansion of __builtin_ctz when the insn isn't available. Checked on alpha-linux-gnu. --- sysdeps/alpha/string-fzb.h | 52 +++++++++++++++++ sysdeps/alpha/string-fzi.h | 113 +++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 sysdeps/alpha/string-fzb.h create mode 100644 sysdeps/alpha/string-fzi.h