Message ID | eec88dfcec8092033aac7c3b30268f437e6ad1ea.1601569371.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | glibc-hwcaps support | expand |
On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: > At this point, only the "atomics" subdirectory is available, > for libraries built using LSE atomics. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > sysdeps/aarch64/dl-hwcaps-subdirs.c | 31 +++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 sysdeps/aarch64/dl-hwcaps-subdirs.c > > diff --git a/sysdeps/aarch64/dl-hwcaps-subdirs.c b/sysdeps/aarch64/dl-hwcaps-subdirs.c > new file mode 100644 > index 0000000000..fd6325024e > --- /dev/null > +++ b/sysdeps/aarch64/dl-hwcaps-subdirs.c > @@ -0,0 +1,31 @@ > +/* Architecture-specific glibc-hwcaps subdirectories. aarch64 version. > + Copyright (C) 2020 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 <dl-hwcaps.h> > +#include <ldsodefs.h> > + > +const char _dl_hwcaps_subdirs[] = "atomics"; > + > +int32_t > +_dl_hwcaps_subdirs_active (void) > +{ > + if (GLRO (dl_hwcap) & HWCAP_ATOMICS) > + return 1; > + > + return 0; > +} > Ok.
* Adhemerval Zanella via Libc-alpha: > On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: >> At this point, only the "atomics" subdirectory is available, >> for libraries built using LSE atomics. > > LGTM, thanks. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Thanks. I didn't want to skip AArch64, but it's really more of a stub implementation: >> +const char _dl_hwcaps_subdirs[] = "atomics"; Does this still make sense with GCC defaulting to -moutline-atomics for ARMv8a? That's what I wonder. Eventually, we need to define some levels for AArch64, and I'm not sure to what extent they would align with the official 8.X versions. To me, they look more like a bouquet you can choose from. Thanks, Florian
On 14/10/2020 11:08, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: >>> At this point, only the "atomics" subdirectory is available, >>> for libraries built using LSE atomics. >> >> LGTM, thanks. >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > Thanks. I didn't want to skip AArch64, but it's really more of a stub > implementation: > >>> +const char _dl_hwcaps_subdirs[] = "atomics"; > > Does this still make sense with GCC defaulting to -moutline-atomics for > ARMv8a? That's what I wonder. > > Eventually, we need to define some levels for AArch64, and I'm not sure > to what extent they would align with the official 8.X versions. To me, > they look more like a bouquet you can choose from. I am not sure how ARM maintainers would like to handle it, either by defining based on ARMv8.x revisions, a subsets of HWCAP capabilities, or by not defining anything. For now I think the atomic makes sense because of -moutline-atomics, although it is essentially ARMv8.1.
The 10/14/2020 11:15, Adhemerval Zanella via Libc-alpha wrote: > On 14/10/2020 11:08, Florian Weimer wrote: > > * Adhemerval Zanella via Libc-alpha: > >> On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: > >>> At this point, only the "atomics" subdirectory is available, > >>> for libraries built using LSE atomics. > >> > >> LGTM, thanks. > >> > >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > > Thanks. I didn't want to skip AArch64, but it's really more of a stub > > implementation: > > > >>> +const char _dl_hwcaps_subdirs[] = "atomics"; > > > > Does this still make sense with GCC defaulting to -moutline-atomics for > > ARMv8a? That's what I wonder. > > > > Eventually, we need to define some levels for AArch64, and I'm not sure > > to what extent they would align with the official 8.X versions. To me, > > they look more like a bouquet you can choose from. > > I am not sure how ARM maintainers would like to handle it, either by > defining based on ARMv8.x revisions, a subsets of HWCAP capabilities, > or by not defining anything. > > For now I think the atomic makes sense because of -moutline-atomics, > although it is essentially ARMv8.1. the atomics path logic was added in glibc 2.28 in commit 397c54c1afa531242602fe3ac7bb47eff0e909f9 (see the reasoning in the commit message). since then this is public api that users may rely on (although likely there are not many users..) so i dont want to remove it (and yes it's not very helpful now that outline-atomics is the default in gcc). otoh i did not review this patch when it was posted because i wanted the return value to not be a signed int when it is a bit mask.
On 14/10/2020 11:37, Szabolcs Nagy wrote: > The 10/14/2020 11:15, Adhemerval Zanella via Libc-alpha wrote: >> On 14/10/2020 11:08, Florian Weimer wrote: >>> * Adhemerval Zanella via Libc-alpha: >>>> On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: >>>>> At this point, only the "atomics" subdirectory is available, >>>>> for libraries built using LSE atomics. >>>> >>>> LGTM, thanks. >>>> >>>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >>> >>> Thanks. I didn't want to skip AArch64, but it's really more of a stub >>> implementation: >>> >>>>> +const char _dl_hwcaps_subdirs[] = "atomics"; >>> >>> Does this still make sense with GCC defaulting to -moutline-atomics for >>> ARMv8a? That's what I wonder. >>> >>> Eventually, we need to define some levels for AArch64, and I'm not sure >>> to what extent they would align with the official 8.X versions. To me, >>> they look more like a bouquet you can choose from. >> >> I am not sure how ARM maintainers would like to handle it, either by >> defining based on ARMv8.x revisions, a subsets of HWCAP capabilities, >> or by not defining anything. >> >> For now I think the atomic makes sense because of -moutline-atomics, >> although it is essentially ARMv8.1. > > the atomics path logic was added in glibc 2.28 in > commit 397c54c1afa531242602fe3ac7bb47eff0e909f9 > (see the reasoning in the commit message). > > since then this is public api that users may rely > on (although likely there are not many users..) > > so i dont want to remove it (and yes it's not > very helpful now that outline-atomics is the > default in gcc). > > otoh i did not review this patch when it was > posted because i wanted the return value to > not be a signed int when it is a bit mask. > Ok fair enough, do you plan to send an updated version Florian or the rest of the set is ok as-is to review?
* Szabolcs Nagy: > The 10/14/2020 11:15, Adhemerval Zanella via Libc-alpha wrote: >> On 14/10/2020 11:08, Florian Weimer wrote: >> > * Adhemerval Zanella via Libc-alpha: >> >> On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: >> >>> At this point, only the "atomics" subdirectory is available, >> >>> for libraries built using LSE atomics. >> >> >> >> LGTM, thanks. >> >> >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> > >> > Thanks. I didn't want to skip AArch64, but it's really more of a stub >> > implementation: >> > >> >>> +const char _dl_hwcaps_subdirs[] = "atomics"; >> > >> > Does this still make sense with GCC defaulting to -moutline-atomics for >> > ARMv8a? That's what I wonder. >> > >> > Eventually, we need to define some levels for AArch64, and I'm not sure >> > to what extent they would align with the official 8.X versions. To me, >> > they look more like a bouquet you can choose from. >> >> I am not sure how ARM maintainers would like to handle it, either by >> defining based on ARMv8.x revisions, a subsets of HWCAP capabilities, >> or by not defining anything. >> >> For now I think the atomic makes sense because of -moutline-atomics, >> although it is essentially ARMv8.1. > > the atomics path logic was added in glibc 2.28 in > commit 397c54c1afa531242602fe3ac7bb47eff0e909f9 > (see the reasoning in the commit message). > > since then this is public api that users may rely > on (although likely there are not many users..) > > so i dont want to remove it (and yes it's not > very helpful now that outline-atomics is the > default in gcc). This patch adds glibc-hwcaps/atomics as a new subdirectory. The whole series does not remove the legacy directories. Based on what you wrote, I should drop this patch, right? Thanks, Florian
The 10/14/2020 16:44, Florian Weimer wrote: > * Szabolcs Nagy: > > > The 10/14/2020 11:15, Adhemerval Zanella via Libc-alpha wrote: > >> On 14/10/2020 11:08, Florian Weimer wrote: > >> > * Adhemerval Zanella via Libc-alpha: > >> >> On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: > >> >>> At this point, only the "atomics" subdirectory is available, > >> >>> for libraries built using LSE atomics. > >> >> > >> >> LGTM, thanks. > >> >> > >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > >> > > >> > Thanks. I didn't want to skip AArch64, but it's really more of a stub > >> > implementation: > >> > > >> >>> +const char _dl_hwcaps_subdirs[] = "atomics"; > >> > > >> > Does this still make sense with GCC defaulting to -moutline-atomics for > >> > ARMv8a? That's what I wonder. > >> > > >> > Eventually, we need to define some levels for AArch64, and I'm not sure > >> > to what extent they would align with the official 8.X versions. To me, > >> > they look more like a bouquet you can choose from. > >> > >> I am not sure how ARM maintainers would like to handle it, either by > >> defining based on ARMv8.x revisions, a subsets of HWCAP capabilities, > >> or by not defining anything. > >> > >> For now I think the atomic makes sense because of -moutline-atomics, > >> although it is essentially ARMv8.1. > > > > the atomics path logic was added in glibc 2.28 in > > commit 397c54c1afa531242602fe3ac7bb47eff0e909f9 > > (see the reasoning in the commit message). > > > > since then this is public api that users may rely > > on (although likely there are not many users..) > > > > so i dont want to remove it (and yes it's not > > very helpful now that outline-atomics is the > > default in gcc). > > This patch adds glibc-hwcaps/atomics as a new subdirectory. The whole > series does not remove the legacy directories. > > Based on what you wrote, I should drop this patch, right? ah i missed that legacy is handled independently. in that case, yes please drop this.
* Adhemerval Zanella: > Ok fair enough, do you plan to send an updated version Florian > or the rest of the set is ok as-is to review? The rest of the set is ready for review. There were some conflicts with the adjustments based on the comments so far. I have rebased the fw/glibc-hwcaps branch on sourceware to reflect what I currently have locally. Thanks, Florian
diff --git a/sysdeps/aarch64/dl-hwcaps-subdirs.c b/sysdeps/aarch64/dl-hwcaps-subdirs.c new file mode 100644 index 0000000000..fd6325024e --- /dev/null +++ b/sysdeps/aarch64/dl-hwcaps-subdirs.c @@ -0,0 +1,31 @@ +/* Architecture-specific glibc-hwcaps subdirectories. aarch64 version. + Copyright (C) 2020 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 <dl-hwcaps.h> +#include <ldsodefs.h> + +const char _dl_hwcaps_subdirs[] = "atomics"; + +int32_t +_dl_hwcaps_subdirs_active (void) +{ + if (GLRO (dl_hwcap) & HWCAP_ATOMICS) + return 1; + + return 0; +}