Re: [PATCH v3] PowerPC: Influence cpu/arch hwcap features via GLIBC_TUNABLES.

[Peter Bergner <bergner@linux.ibm.com>]

On 7/6/23 8:16 AM, Adhemerval Zanella Netto wrote:
> On 06/07/23 09:25, bmahi496--- via Libc-alpha wrote:
>>  struct cpu_features
>>  {
>>    bool use_cached_memopt;
>> +  unsigned long int hwcap;
>> +  unsigned long int hwcap2;
>> +};
>> +
>> +static const struct
>> +{
>> +  const char *name;
>> +  int mask;
>> +  bool id;
>> +} hwcap_tunables[] = {
>> +   /* AT_HWCAP tunable masks.  */
>> +   { "4xxmac",           PPC_FEATURE_HAS_4xxMAC,                 0 },
> 
> This creates one extra dynamic relocation per entry:
> 
> powerpc64le-linux-gnu-base$ powerpc64le-linux-gnu-readelf -a elf/ld.so
> [...]
> Relocation section '.relr.dyn' at offset 0xc68 contains 3 entries:
>   10 offsets
> [...]
> 
> powerpc64le-linux-gnu-patch$ powerpc64le-linux-gnu-readelf -a elf/ld.so
> [...]
> Relocation section '.relr.dyn' at offset 0xc68 contains 5 entries:
>   53 offsets
> [...]

Good catch!  Especially since the plan is to add space for hwcap3 and hwcap4
(possibly more) in the near future.  We don't need the extra hwcaps yet, but
we want to reserve the space in the TCB for them now, where __builtin_cpu_supports()
uses them, so that when we need them in the future, the space is already
there to use in distro glibcs.




> Which I think we should avoid since is a small slow down on every program
> invocation.  You can either define the name with a predefine size that
> fits for every name (say 32), but this will waste a some of space.  Or
> you can specify the hwcap_tunables struct as pointing to an offset:
> 
>   static const char hwcap_names[] = 
>     "4xxmac\0"
>     "altivec\0"
>    [...]
> 
>   static const struct
>   {
>     unsigned short off;
>     int mask;
>     bool id;
>   } hwcap_tunables[] = {
>     { 0,    PPC_FEATURE_HAS_4xxMAC,                 0 },
>     { 7,    PPC_FEATURE_HAS_ALTIVEC,                0 },
>     [...]
>   }
> And then you check the name as:
> 
>   for (i = 0; array_length (hwcap_tunables); ++i)
>     {
>       const char *hwcap_name = hwcap_names + hwcap_tunables[i].off;
>       [...]
>     }
> 
> The drowback is to get the offsets right it would require some preprocessor
> phase (something like we do for the signal and errno list).

I don't think we need the offset in the struct, since Mahesh's loop is
already calculating the strlen of hwcap_names[].name, so we can just
update the pointer as we go.  Ala...

  const char *hwcap_name = &hwcap_names[0];
  for (i = 0; array_length (hwcap_tunables); ++i)
    {
      size_t tunable_len = STRLEN_DEFAULT (hwcap_name);

      /* Check the tunable name on the supported list.  */
      if (tunable_len == feature_len
          && MEMCMP_DEFAULT (feature, hwcap_name, feature_len) == 0)
        {
           ...
        }
      hwcap_name += tunable_len + 1;
    }


Peter