Re: [PATCH 1/2] riscv: Add memmove string operation.

From: Palmer Dabbelt
Date: Tue Aug 13 2019 - 19:51:09 EST


On Mon, 12 Aug 2019 08:04:46 PDT (-0700), Christoph Hellwig wrote:
On Wed, Aug 07, 2019 at 03:19:14PM +0800, Nick Hu wrote:
There are some features which need this string operation for compilation,
like KASAN. So the purpose of this porting is for the features like KASAN
which cannot be compiled without it.

KASAN's string operations would replace the original string operations and
call for the architecture defined string operations. Since we don't have
this in current kernel, this patch provides the implementation.

This porting refers to the 'arch/nds32/lib/memmove.S'.

This looks sensible to me, although my stringop asm is rather rusty,
so just an ack and not a real review-by:

Acked-by: Christoph Hellwig <hch@xxxxxx>

FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.

Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.