[RFC PATCH 16/19] riscv: Add accelerated strcmp routines

[Adhemerval Zanella Netto]

On 31/03/23 02:06, Jeff Law wrote:
> 
> 
> On 2/7/23 07:15, Christoph Müllner wrote:
> 
>>      > diff --git a/sysdeps/riscv/multiarch/strcmp_zbb_unaligned.S
>>      > b/sysdeps/riscv/multiarch/strcmp_zbb_unaligned.S
> [ ... ]
> 
> 
>>      > +
>>      > +ENTRY_ALIGN (STRCMP, 6)
>>      > +       /* off...delta from src1 to src2.  */
>>      > +       sub     off, src2, src1
>>      > +       li      m1, -1
>>      > +       andi    tmp, off, SZREG-1
>>      > +       andi    align1, src1, SZREG-1
>>      > +       bnez    tmp, L(misaligned8)
>>      > +       bnez    align1, L(mutual_align)
>>      > +
>>      > +       .p2align 4
>>      > +L(loop_aligned):
>>      > +       REG_L   data1, 0(src1)
>>      > +       add     tmp, src1, off
>>      > +       addi    src1, src1, SZREG
>>      > +       REG_L   data2, 0(tmp)
> 
> So any thoughts on reducing the alignment?  Based on the data I've seen we very rarely ever take the branch to L(loop_aligned).    So aligning this particular label is of dubious value to begin with.  As it stands we have to emit 3 full sized nops to achieve the requested alignment and they can burn most of an issue cycle.
> 
> While it's highly dependent on pipeline state, there's a reasonable chance of shaving a cycle by reducing the alignment to p2align 3.
> 
> I haven't done much with analyzing the rest of the code as it just hasn't been hot in any of the cases I've looked at.
> 
> I'd be comfortable with this going in as-is or with the alignment adjustment.  Obviously wiring it up via ifunc is dependent upon settling the kernel->glibc interface.
> 
> Jeff

Is this implementation really better than new generic one [1]? With a target
with zbb support, the generic word comparison should use orc.b instruction [2].
And the final comparison, once with the last word or the mismatch word is found,
should use clz/ctz instruction [3] (result also in branchless code, albeit
I have not check if better than the snippet this implementation uses).

The generic implementation also has the advantage of use word instruction
on unaligned case, where this implementation does a naive byte per byte
check.

So maybe a better option would to optimize further the generic implementation.
One option might be to parametrize the final_cmp so you can use the branchless
trick (if it indeed is better than generic code).  Another option that the 
generic implementation does not explore is manual loop unrolling, as done by 
multiple assembly implementations.

[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=string/strcmp.c;h=11ec8bac816b630417ccbfeba70f9eab6ec37874;hb=HEAD
[2] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/riscv/string-fza.h;h=4429653a001de09730cfa83325b29556a8afb5ed;hb=HEAD
[3] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/generic/string-fzi.h;h=2deecefc236833abffbee886851d75e7ecf66755;hb=HEAD