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

["Paul A. Clarke" <pc@us.ibm.com>]

On Thu, Oct 01, 2020 at 06:33:29PM +0200, Florian Weimer via Libc-alpha wrote:
> The "power10" and "power9" subdirectories are selected.
> ---
>  .../powerpc/powerpc64/le/dl-hwcaps-subdirs.c  | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c
> 
> 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?

> +_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?
The bits are also inverse the the relative offset in the string,
which could be confusing (index 0 is the 2nd bit and index 1 is the 1st).

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.

> +
> +  return 0;
> +}

PC