diff mbox series

[20/28] aarch64: Add glibc-hwcaps support

Message ID eec88dfcec8092033aac7c3b30268f437e6ad1ea.1601569371.git.fweimer@redhat.com
State Dropped
Headers show
Series glibc-hwcaps support | expand

Commit Message

Florian Weimer Oct. 1, 2020, 4:33 p.m. UTC
At this point, only the "atomics" subdirectory is available,
for libraries built using LSE atomics.
---
 sysdeps/aarch64/dl-hwcaps-subdirs.c | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 sysdeps/aarch64/dl-hwcaps-subdirs.c

Comments

Adhemerval Zanella Oct. 14, 2020, 1:46 p.m. UTC | #1
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.
Florian Weimer Oct. 14, 2020, 2:08 p.m. UTC | #2
* 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
Adhemerval Zanella Oct. 14, 2020, 2:15 p.m. UTC | #3
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.
Szabolcs Nagy Oct. 14, 2020, 2:37 p.m. UTC | #4
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.
Adhemerval Zanella Oct. 14, 2020, 2:43 p.m. UTC | #5
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?
Florian Weimer Oct. 14, 2020, 2:44 p.m. UTC | #6
* 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
Szabolcs Nagy Oct. 14, 2020, 3:09 p.m. UTC | #7
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.
Florian Weimer Oct. 14, 2020, 3:13 p.m. UTC | #8
* 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 mbox series

Patch

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;
+}