[EXT] [PATCH 1/1] riscv: Add Zbkb optimized repeat_bytes helper

[Pincheng Wang]

On 2025/9/12 1:03, Peter Bergner wrote:
> On 9/11/25 10:48 AM, Pincheng Wang wrote:
>> +/* Miscellaneous functions used in string implementations.  Generic C version.
>> +   Copyright (C) 2023-2025 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
> 
> Others can correct me if I'm wrong, but since this is a new file, I think
> you should be using "2025" for the copyright year, rather than "2023-2025".
> 
> 

You're right, this is a new file. I copied the header from 
sysdeps/generic/string-misc.h and missed adjusting the years. I'll 
change it to the right form and submit a V2 patch.

> 
> 
>> +/* Setup an word with each byte being c_in.  For instance, on a 64 bits
>> +   machine with input as 0xce the functions returns 0xcececececececece.  */
>> +static __always_inline op_t
>> +repeat_bytes (unsigned char c_in)
>> +{
>> +  op_t c = c_in;
>> +#if defined __riscv_zbkb && __riscv_xlen == 64
>> +  /* 8bit -> 16bit -> 32bit -> 64bit pattern replication */
>> +  __asm__ ("packh %0, %1, %1\n\t"
>> +           "packw %0, %0, %0\n\t"
>> +           "pack %0, %0, %0"
>> +           : "=r"(c) : "r"(c));
>> +#elif defined __riscv_zbkb && __riscv_xlen == 32
>> +  /* 8bit -> 16bit -> 32bit pattern replication */
>> +  __asm__ ("packh %0, %1, %1\n\t"
>> +           "pack %0, %0, %0"
>> +           : "=r"(c) : "r"(c));
>> +#else
>> +  c = ((op_t)-1 / 0xff) * c_in;
>> +#endif
>> +  return c;
>> +}
> 
> It seems this inline asm creates a lot of unneeded data dependencies,
> which may affect low end cores???  Maybe rewrite it to remove those using
> some temp variables and a separate output variable?
> 
> ...or maybe people don't think we should care?
> 
> Peter
> 

Good point. My current sequence does reuse the same register and create 
a dependency chain. For low-end/short pipeline cores that can matter. In 
V2 I'll rewrite this inline asm, use separate temporaries and a distinct 
output to reduce false deps and give the scheduler freedom. Below is one 
possible approach for reference:

RV64:
static __always_inline op_t
repeat_bytes (unsigned char c_in)
{
   op_t out, t16, t32;
#if defined __riscv_zbkb && __riscv_xlen == 64
   __asm__ (
     "packh %1, %3, %3\n\t"   /* t16  = replicate 8b -> 16b */
     "packw %2, %1, %1\n\t"   /* t32  = replicate 16b -> 32b */
     "pack  %0, %2, %2"       /* out  = replicate 32b -> 64b */
     : "=&r"(out), "=&r"(t16), "=&r"(t32)
     : "r"((op_t)c_in)
   );
   return out;
#else
   return ((op_t)-1 / 0xff) * c_in;
#endif
}

RV32:
static __always_inline op_t
repeat_bytes (unsigned char c_in)
{
   op_t out, t16;
#if defined __riscv_zbkb && __riscv_xlen == 32
   __asm__ (
     "packh %1, %2, %2\n\t"   /* t16  = replicate 8b -> 16b */
     "pack  %0, %1, %1"       /* out  = replicate 16b -> 32b */
     : "=&r"(out), "=&r"(t16)
     : "r"((op_t)c_in)
   );
   return out;
#else
   return ((op_t)-1 / 0xff) * c_in;
#endif
}

I'll benchmark both versions (current single-reg vs. temp-based) with 
benchtests; I'm also trying to get runs on real hardware in addition to 
QEMU to confirm there's no regression on simpler cores.

Thanks again for the helpful suggestions!

Best,
Wang