Re: [RFC 2.0] Implementing hwcap2

[Roland McGrath <roland at hack dot frob dot com>]

> I'm presuming that the kernel will assign '26' as AT_HWCAP2.  This may
> change when the feature is actually added.

When the exact bits are predicated on kernel changes, we don't commit
changes presuming such bit values to libc until the changes are in the
primary kernel tree (and preferably an -rc tag so it's that much more
likely the kernel folks won't change their minds before release).

> There isn't a usage of hwcap2 in this patch.  There will be a later
> patch which adds some feature definitions to powerpc/hwcap.h that
> utilize hwcap2.

It's fine to separate the change that way.

> I'm not sure how useful tst-getauxval is until there is something it can
> return from HWCAP2 that's not in HWCAP.  This would only be relevant to
> architectures which have features defined in HWCAP2... which would
> presumably only be PowerPC to start with.

It's hard to see how it's useful at all in its present form.
It tests that getauxval doesn't crash or hang.  Do we really
care to have a test that doesn't do any correctness checking?

> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -22,7 +22,7 @@
>  #include <sys/mman.h>
>  #include <dl-cache.h>
>  #include <dl-procinfo.h>
> -
> +#include <stdint.h>
>  #include <_itoa.h>
>  
>  #ifndef _DL_PLATFORMS_COUNT

This and the other files where this was the only change seem wrong to me.

If the file itself uses the name uint64_t and doesn't already include some
header that's specified to define it, then the file is already wrong and
the clean-up should be a separate (earlier) change from this one.  If the
file itself doesn't use that name, then it shouldn't need the header.

ldsodefs.h should include <stdint.h> since it uses uint64_t in its
declarations.  That being the case, nothing that just uses dl_hwcap
(even if it uses the uint64_t name along with it) should need its own
<stdint.h> include just because of that.

> @@ -211,8 +212,12 @@ _dl_aux_init (ElfW(auxv_t) *av)
>  	GL(dl_phnum) = av->a_un.a_val;
>  	break;
>        case AT_HWCAP:
> -	GLRO(dl_hwcap) = (unsigned long int) av->a_un.a_val;
> +	GLRO(dl_hwcap) |= (unsigned long int) av->a_un.a_val;
>  	break;
> +      case AT_HWCAP2:
> +	GLRO(dl_hwcap) |= ((uint64_t) av->a_un.a_val) << 32;
> +	break;

This presupposes that dl_hwcap starts out zero.  That is probably already
the case, but the presumption is new and merits a comment.


Thanks,
Roland