Re: [PATCH 2/2] riscv: vectorised mem* and str* functions

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 01/02/23 12:33, Jeff Law via Libc-alpha wrote:
> 
> 
> On 2/1/23 02:52, Sergei Lewis wrote:
>> Initial implementations of memchr, memcmp, memcpy, memmove, memset, strchr,
>> strcmp, strcpy, strlen, strncmp, strncpy, strnlen, strrchr, strspn
>> targeting the riscv "V" extension, version 1.0
>>
>> The vectorised implementations assume VLENB of at least 128 and at least 32
>> registers (as mandated by the "V" extension spec). They also assume that
>> VLENB is a power of two which is no larger than the page size, and (as
>> vectorised code in glibc for other platforms does) that it is safe to read
>> past null terminators / buffer ends provided one does not cross a page
>> boundary.
>>
>> Signed-off-by: Sergei Lewis <slewis@rivosinc.com>
>> ---
>>   sysdeps/riscv/rv64/rvv/Implies     |   2 +
>>   sysdeps/riscv/rv64/rvv/memchr.S    | 127 +++++++++++++++++++
>>   sysdeps/riscv/rv64/rvv/memcmp.S    |  93 ++++++++++++++
>>   sysdeps/riscv/rv64/rvv/memcpy.S    | 154 +++++++++++++++++++++++
>>   sysdeps/riscv/rv64/rvv/memmove.c   |  22 ++++
>>   sysdeps/riscv/rv64/rvv/memset.S    |  89 ++++++++++++++
>>   sysdeps/riscv/rv64/rvv/strchr.S    |  92 ++++++++++++++
>>   sysdeps/riscv/rv64/rvv/strchrnul.c |  22 ++++
>>   sysdeps/riscv/rv64/rvv/strcmp.S    | 108 +++++++++++++++++
>>   sysdeps/riscv/rv64/rvv/strcpy.S    |  72 +++++++++++
>>   sysdeps/riscv/rv64/rvv/strcspn.c   |  22 ++++
>>   sysdeps/riscv/rv64/rvv/strlen.S    |  67 ++++++++++
>>   sysdeps/riscv/rv64/rvv/strncmp.S   | 104 ++++++++++++++++
>>   sysdeps/riscv/rv64/rvv/strncpy.S   |  96 +++++++++++++++
>>   sysdeps/riscv/rv64/rvv/strnlen.S   |  81 +++++++++++++
>>   sysdeps/riscv/rv64/rvv/strrchr.S   |  88 ++++++++++++++
>>   sysdeps/riscv/rv64/rvv/strspn.S    | 189 +++++++++++++++++++++++++++++
> Does this need to be revamped given the recent push to do more with generic code and target specific hooks for mem* and str*?
> 

I should be doable, although I think it might require some more hooks since
afaiu RISCV vector instruction does not have the concept of address update
with his vector approach (as my generic string routines does by using the
'vector' type op_t).  So I am not sure it is a well fit for RISCV vec
approach.


> Shouldn't the implementations be in a multiarch directory?  I would fully expect we're going to need both a vector and scalar implementation selected by an ifunc.

If so, we will need both IFUNC support on riscv (which as Florian pointed out
it is not default); and a way to discover at runtime the processor/kernel
capabilities.  It does not seem that RISCV has the expected hwcap support,
the UAPI only define COMPAT_HWCAP_ISA_* that also does not seems to match
the vector or bitmanip extensions.  Does it have a instruction to query for
such information, something like cpuid (x86) or midr_elX (aarch64)?

It would be interesting to add a ifunc variant for bitmanip as well.

> 
> I'm happy to pass along the current bits from VRULL which put that infrastructure in place.  I just haven't had the time to look at revamping their assembly implementations for the new generic+hooks scheme.

I just sent an updated version [1] where I added bitmanip optimization [2].
So now with a recent gcc (I tested with gcc 13 with upstream qemu), the
string routines should use ctz/clz/orc.b.  I did not add support for
xthread [3], but it should be doable.

Once I get this upstream, I have a WIP to revamp the memmove/memcpy/memset/memcmp
as well.  At least for memcmp it should use bitmanip as well.
 
[1] https://patchwork.sourceware.org/project/glibc/list/?series=16622
[2] https://patchwork.sourceware.org/project/glibc/patch/20230201170406.303978-21-adhemerval.zanella@linaro.org/
[3] https://lore.kernel.org/all/20220906122243.1243354-1-christoph.muellner@vrull.eu/