Re: [PATCH v3 05/18] string: Improve generic strlen

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 11/01/2018 15:21, Paul Eggert wrote:
> On 01/10/2018 04:47 AM, Adhemerval Zanella wrote:
>> +  /* Align pointer to sizeof op_t.  */
>> +  const uintptr_t s_int = (uintptr_t) str;
>> +  const op_t *word_ptr = (const op_t*) (s_int & -sizeof (op_t));
> 
> I see this sort of code used in multiple places (often with different implementations), and suggest packaging it up into an inline function. Also, you might consider implementing it this way, which is a bit simpler (it's the method used in your generic strcmp):
> 
>   op_t *
>   word_containing (char const *p)
>   {
>      return (op_t *) (p - (uintptr_t) p % sizeof (op_t));
>   }
> 
> This generates the same code with gcc -O2, and minimizes the usage of pointers as integers.

Thanks, I have added this function suggestion to string-maskoff.h and used on
the generic implementations.

> 
> Similarly, in other code I suggest using char * instead of uintptr_t, as much as possible. This works just as well with ordinary addition, and will simplify GCC's job if we ever build with -fcheck-pointer-bounds.
> 
>> +  while (1)
>>       {
>> -      /* 64-bit version of the magic.  */
>> -      /* Do the shift in two steps to avoid a warning if long has 32 bits.  */
>> -      himagic = ((himagic << 16) << 16) | himagic;
>> -      lomagic = ((lomagic << 16) << 16) | lomagic;
>> +      if (has_zero (word))
>> +    break;
>> +      word = *++word_ptr;
>>       }
> 
> This would be a bit simpler:
> 
>   while (! has_zero (word))
>     word = *++word_ptr;
> 

Thanks, I have applied it locally as well.