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

[Pincheng Wang]

在 2025/9/12 1:21, 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

Hi Peter,

Thanks again for your feedback!

I recently ran some additional performance tests for this patch on real 
hardware to better understand the potential impact of data dependencies. 
Specifically, I compared three version of `repeat_bytes`:

op_t origin_repeat_bytes(unsigned char c_in) {
   return ((op_t)-1 / 0xff) * c_in);
}

op_t zbkb_v1_repeat_bytes(unsigned char c_in) {
   op_t c = c_in;
   __asm__ volatile (
     "packh %0, %1, %1\n\t"
     "pack %0, %0, %0"
     : "=r"(c) : "r"(c));
   return c;
}

op_t zbkb_v2_repeat_bytes(unsigned char c_in) {
   op_t out, t16;
   __asm__ volatile (
     "packh %0, %2, %2\n\t"
     "pack %1, %0, %0"
     : "=&r"(t16), "=&r"(out)
     : "r"((op_t)c_in));
   return out;
}

The test were performed on a Raspberry Pi Pico 2, which uses a Hazard 3 
three-stage RV32IMACZb* core-likely representative of the "low-end, 
short pipeline" cores you mentioned. Each function was executed 
100,000,000 times, which execution time measured via hardware timer:
- Origin version: 12,001,363 µs (100%)
- Data-dependent version: 10,666,671 µs (88.88%)
- Dependency-free version: 10,000,003 µs (83.32%)

For completeness, here is the compiler-generated assembly for each version:

1000012e <origin_repeat_bytes>:
1000012e: 00851793   slli a5,a0,0x8
10000132: 953e       add a0,a0,a5
10000134: 01051793   slli a5,a0,0x10
10000138: 953e       add a0,a0,a5
1000013a: 8082       ret

1000013c <zbkb_v1_repeat_bytes>:
1000013c: 08a57533   packh a0,a0,a0
10000140: 08a54533   pack a0,a0,a0
10000144: 8082       ret

10000146 <zbkb_v2_repeat_bytes>:
10000146: 87aa       mv a5,a0
10000148: 08f7f733   packh a4,a5,a5
1000014c: 08e74533   pack a0,a4,a4
10000150: 8082       ret

The "dependency-free" version actually has more instructions and 
register usage. My current guess is taht for such a simple chain of 
dependencies, the pipeline's forwarding paths are already effective, so 
software-side elimination of the dependency may not provide significant 
benefit, and could slightly increase pressure on register allocation or so.

Unfortunately, I do not currently have RISC-V hardware suitable for 
running a full glibc build, but I think this focused test still provides 
useful insight. I would appreciate futher input from you and the 
community on which implementation would be preferable.

Thanks again for your helpful suggestions!

Regards,
Pincheng Wang