Re: [PATCH] powerpc64: Add optimized strcpy and stpcpy for POWER10

[MAHESH BODAPATI <bmahi496@linux.ibm.com>]

On 21/06/24 3:10 am, Paul E Murphy wrote:
>
>
> On 6/16/24 6:49 AM, bmahi496@linux.ibm.com wrote:
>
>> +/* Implements the function
>> +
>> +   char * [r3] strcpy (char *dest [r3], const char *src [r4])
>> +
>> +   or
>> +
>> +   char * [r3] stpcpy (char *dest [r3], const char *src [r4])
>> +
>> +   if USE_AS_STPCPY is defined.
>> +
>> +   The implementation can load bytes past a NULL terminator, but only
>> +   up to the next 16B/64B boundary, so it never crosses a page.  */
>
> I think this comment is confusing.  Maybe it suffices to say that it 
> never reads across a page boundary, but may read beyond the NUL 
> terminator.
>
>

I will update the comment.


>> +
>> +
>> +#define LXVP(xtp,dq,ra)        \
>> +    .long(((6)<<(32-6))        \
>> +          | ((((xtp)-32)>>1)<<(32-10)) \
>> +          | ((1)<<(32-11))         \
>> +          | ((ra)<<(32-16))        \
>> +          | dq)
>> +
>> +/* Load 4 quadwords, merge into one VR for speed and check for NULLs
>> +   and branch to label if NULL is found.  */
>> +#define CHECK_64B(offset,addr,label)      \
>> +    LXVP(v4+32,offset,addr);        \
>> +    LXVP(v6+32,offset+32,addr);     \
>
> Have you benchmarked using lxv vs lxvp here?  If the performance 
> difference is marginal, I think this implementation could be used for 
> P9 too.
>
>

No, I didn't check. I will do the bench-marking with lxv and update.


>> +    vminub    v14,v4,v5;        \
>> +    vminub    v15,v6,v7;        \
>> +    vminub    v16,v14,v15;          \
>> +    vcmpequb. v0,v16,v18;           \
>> +    beq     cr6,$+12;           \
>> +    li      r7,offset;          \
>> +    b       L(label);           \
>> +    stxv    32+v5,(offset+0)(r11);      \
>> +    stxv    32+v4,(offset+16)(r11);     \
>> +    stxv    32+v7,(offset+32)(r11);     \
>> +    stxv    32+v6,(offset+48)(r11)
>
> Otherwise, this LGTM with trivial comments addressed.