Re: [PATCH 18/28] powerpc64le: Add glibc-hwcaps support

[Florian Weimer via Libc-alpha <libc-alpha@sourceware.org>]

* Paul A. Clarke:

>> diff --git a/sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c b/sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c
>> new file mode 100644
>> index 0000000000..496daf0fa0
>> --- /dev/null
>> +++ b/sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c
>> @@ -0,0 +1,34 @@
>> +/* Architecture-specific glibc-hwcaps subdirectories.  powerpc64le 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[] = "power10:power9";
>> +
>> +int32_t
>
> Is 32 bits enough?  Any reason not to make this 64 bits?

It's enough for now for all the targets that have glibc-hwcaps support
so far.  It's a strictly internal interface, so it's easy to change
later.

We also need to consider the impact on library path processing.  Even
just 20 additional open & stat calls for each LD_LIBRARY_PATH entry
might be quite visible in process startup benchmarks.  So I'm not sure
if we will ever use those 32 bits.

>> +_dl_hwcaps_subdirs_active (void)
>> +{
>> +  if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1)
>> +    return 3;
>> +
>> +  if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00)
>> +    return 1;
>
> Is there some way to tie these magic numbers closer to their meaning?

Would you please have a look at the the x86-64 implementation and check
if it is closer to your liking?

> Perhaps something like (not tested):
> --
> const char * const _dl_hwcaps_subdirs[] = {
> #define _DL_HWCAPS_SUBDIR_POWER10_BIT 0x2 /* or 1 to preserve same order.  */
> 	"power10",
> #define _DL_HWCAPS_SUBDIR_POWER9_BIT 0x1 /* or 2.  */
> 	"power9"
> };
>
> int32_t
> _dl_hwcaps_subdirs_active (void)
> {
>   int32_t result = 0;
>
>   if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1)
>     result |= _DL_HWCAPS_SUBDIR_POWER10_BIT;
>
>   if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00)
>     result |= _DL_HWCAPS_SUBDIR_POWER9_BIT;
>
>   return result;
> }
> --
>
> Of course, that would require changes to the code that parses
> _dl_hwcaps_subdirs.

I chose the current approach to avoid relocations and memory allocations
for processing hwcaps settings (e.g. from the ld.so command line).

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill