Messages in this thread | | | Date | Tue, 13 Aug 2019 16:50:58 -0700 (PDT) | Subject | Re: [PATCH 1/2] riscv: Add memmove string operation. | From | Palmer Dabbelt <> |
| |
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@lst.de>
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.
| |