[PATCH v6 3/3] RISC-V: add riscv vector support for memcpy

Palmer Dabbelt palmer@rivosinc.com
Fri Feb 14 17:22:08 GMT 2025


On Fri, 14 Feb 2025 04:59:22 PST (-0800), aswaterman@gmail.com wrote:
> On Fri, Feb 14, 2025 at 3:24 AM Andrew Waterman <aswaterman@gmail.com> wrote:
>>
>> This is not what a generic RVV memcpy routine should look like.  It
>> should look like this:
>> https://github.com/riscvarchive/riscv-v-spec/blob/2f68ef7256d6ec53e4d2bd7cb12862f406d64e34/example/memcpy.s#L7
>>
>> The version you are proposing looks like it is optimized for a
>> particular microarchitecture.  That's fine, but it is a
>> de-optimization for other microarchitectures.  The generic version I
>> reference contains fewer branches, fewer static instructions, and
>> fewer dynamic instructions, for any memcpy size > 0.  (And 0 is not a
>> case we should be aggressively optimizing for.)
>
> My previous message isn't entirely accurate.  The proposed
> implementation does elide a couple ALU ops in the short-vector case,
> but it does so at the cost of another branch.  It remains the case
> that the reference code from the spec will be better for most
> implementations.

So I think we need to see some numbers from some actual HW in order to 
reason about this stuff?  These string routines are the sort of place 
where uarch-specific special cases can be useful, but without numbers 
we're just going to pile up special cases that don't help.

> Separately, there is a subtle bug in the proposed code; the bgt should
> be bgtu, otherwise lengths >= 2^(XLEN-1) will be incorrectly handled.
>
>>
>> On Fri, Feb 14, 2025 at 12:52 AM <daichengrong@iscas.ac.cn> wrote:
>> >
>> > From: daichengrong <daichengrong@iscas.ac.cn>
>> >
>> > Change in v6:
>> >   Optimize the RVV memcpy for small lengths less than VLEN/8 bytes
>> >
>> > Changes in v5:
>> >    fix Indentation in memcpy.c
>> >    check ifunc-impl-list memcpy vector support with by dl_hwcap
>> >
>> > Changes in v4:
>> >   update rvv memcpy support by compiler
>> >   check whether rvv enabled by dl_hwcap
>> >
>> > Changes in v3:
>> >   Remove unnecessary whitespace
>> >
>> > Changes in v2:
>> >   delete size-0 branch
>> >
>> > ---
>> >  sysdeps/riscv/multiarch/memcpy_vector.S       | 43 +++++++++++++++++++
>> >  .../unix/sysv/linux/riscv/multiarch/Makefile  |  8 ++++
>> >  .../linux/riscv/multiarch/ifunc-impl-list.c   | 13 ++++++
>> >  .../unix/sysv/linux/riscv/multiarch/memcpy.c  |  7 +++
>> >  4 files changed, 71 insertions(+)
>> >  create mode 100644 sysdeps/riscv/multiarch/memcpy_vector.S
>> >
>> > diff --git a/sysdeps/riscv/multiarch/memcpy_vector.S b/sysdeps/riscv/multiarch/memcpy_vector.S
>> > new file mode 100644
>> > index 0000000000..4783640776
>> > --- /dev/null
>> > +++ b/sysdeps/riscv/multiarch/memcpy_vector.S
>> > @@ -0,0 +1,43 @@
>> > +/* memcpy for RISC-V Vector.
>> > +   Copyright (C) 2024-2025 Free Software Foundation, Inc.
>> > +   This file is part of the GNU C Library.
>> > +
>> > +   The GNU C Library is free software; you can redistribute it and/or
>> > +   modify it under the terms of the GNU Lesser General Public
>> > +   License as published by the Free Software Foundation; either
>> > +   version 2.1 of the License, or (at your option) any later version.
>> > +
>> > +   The GNU C Library is distributed in the hope that it will be useful,
>> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> > +   Lesser General Public License for more details.
>> > +
>> > +   You should have received a copy of the GNU Lesser General Public
>> > +   License along with the GNU C Library; if not, see
>> > +   <https://www.gnu.org/licenses/>.  */
>> > +
>> > +
>> > +#include <sysdep.h>
>> > +#include <sys/asm.h>
>> > +
>> > +ENTRY (__memcpy_vector)
>> > +    beq        a2, zero, L(ret)
>> > +    mv     a6, a0
>> > +
>> > +    /* if LEN > VLMAX jump to block_copy handling.  */
>> > +    vsetvli a4,zero,e8,m1,ta,ma
>> > +    bgt a2,a4,L(block_copy)
>> > +    vle8.v  v8,(a1)
>> > +    vse8.v  v8,(a6)
>> > +    ret
>> > +L(block_copy):
>> > +    vsetvli a3,a2,e8,m8,ta,ma
>> > +    vle8.v  v8,(a1)
>> > +    vse8.v  v8,(a6)
>> > +    add     a1,a1,a3
>> > +    add     a6,a6,a3
>> > +    sub     a2,a2,a3
>> > +    bnez    a2,L(block_copy)
>> > +L(ret):
>> > +    ret
>> > +END (__memcpy_vector)
>> > diff --git a/sysdeps/unix/sysv/linux/riscv/multiarch/Makefile b/sysdeps/unix/sysv/linux/riscv/multiarch/Makefile
>> > index fcef5659d4..394033e077 100644
>> > --- a/sysdeps/unix/sysv/linux/riscv/multiarch/Makefile
>> > +++ b/sysdeps/unix/sysv/linux/riscv/multiarch/Makefile
>> > @@ -5,5 +5,13 @@ sysdep_routines += \
>> >    memcpy_noalignment \
>> >    # sysdep_routines
>> >
>> > +ifeq ($(have-gcc-riscv-rvv),yes)
>> > +sysdep_routines += \
>> > +  memcpy_vector \
>> > +  # rvv sysdep_routines
>> > +
>> > +ASFLAGS-memcpy_vector.S += -march=rv64gcv
>> > +endif
>> > +
>> >  CFLAGS-memcpy_noalignment.c += -mno-strict-align
>> >  endif
>> > diff --git a/sysdeps/unix/sysv/linux/riscv/multiarch/ifunc-impl-list.c b/sysdeps/unix/sysv/linux/riscv/multiarch/ifunc-impl-list.c
>> > index 1c1deca8f6..2088097558 100644
>> > --- a/sysdeps/unix/sysv/linux/riscv/multiarch/ifunc-impl-list.c
>> > +++ b/sysdeps/unix/sysv/linux/riscv/multiarch/ifunc-impl-list.c
>> > @@ -19,6 +19,7 @@
>> >  #include <ifunc-impl-list.h>
>> >  #include <string.h>
>> >  #include <sys/hwprobe.h>
>> > +#include <ldsodefs.h>
>> >
>> >  size_t
>> >  __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>> > @@ -27,6 +28,9 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>> >    size_t i = max;
>> >
>> >    bool fast_unaligned = false;
>> > +#if defined(HAVE_RISCV_ASM_VECTOR_SUPPORT)
>> > +  bool rvv_ext = false;
>> > +#endif
>> >
>> >    struct riscv_hwprobe pair = { .key = RISCV_HWPROBE_KEY_CPUPERF_0 };
>> >    if (__riscv_hwprobe (&pair, 1, 0, NULL, 0) == 0
>> > @@ -34,7 +38,16 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>> >            == RISCV_HWPROBE_MISALIGNED_FAST)
>> >      fast_unaligned = true;
>> >
>> > +#if defined(HAVE_RISCV_ASM_VECTOR_SUPPORT)
>> > +  if (GLRO(dl_hwcap) & COMPAT_HWCAP_ISA_V)
>> > +    rvv_ext = true;
>> > +#endif
>> > +
>> >    IFUNC_IMPL (i, name, memcpy,
>> > +#if defined(HAVE_RISCV_ASM_VECTOR_SUPPORT)
>> > +             IFUNC_IMPL_ADD (array, i, memcpy, rvv_ext,
>> > +                             __memcpy_vector)
>> > +#endif
>> >               IFUNC_IMPL_ADD (array, i, memcpy, fast_unaligned,
>> >                               __memcpy_noalignment)
>> >               IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_generic))
>> > diff --git a/sysdeps/unix/sysv/linux/riscv/multiarch/memcpy.c b/sysdeps/unix/sysv/linux/riscv/multiarch/memcpy.c
>> > index 8544f5402a..1982e4ab65 100644
>> > --- a/sysdeps/unix/sysv/linux/riscv/multiarch/memcpy.c
>> > +++ b/sysdeps/unix/sysv/linux/riscv/multiarch/memcpy.c
>> > @@ -32,11 +32,18 @@ extern __typeof (__redirect_memcpy) __libc_memcpy;
>> >
>> >  extern __typeof (__redirect_memcpy) __memcpy_generic attribute_hidden;
>> >  extern __typeof (__redirect_memcpy) __memcpy_noalignment attribute_hidden;
>> > +extern __typeof (__redirect_memcpy) __memcpy_vector attribute_hidden;
>> >
>> >  static inline __typeof (__redirect_memcpy) *
>> >  select_memcpy_ifunc (uint64_t dl_hwcap, __riscv_hwprobe_t hwprobe_func)
>> >  {
>> >    unsigned long long int v;
>> > +
>> > +#if defined(HAVE_RISCV_ASM_VECTOR_SUPPORT)
>> > +  if (dl_hwcap & COMPAT_HWCAP_ISA_V)
>> > +    return __memcpy_vector;
>> > +#endif
>> > +
>> >    if (__riscv_hwprobe_one (hwprobe_func, RISCV_HWPROBE_KEY_CPUPERF_0, &v) == 0
>> >        && (v & RISCV_HWPROBE_MISALIGNED_MASK) == RISCV_HWPROBE_MISALIGNED_FAST)
>> >      return __memcpy_noalignment;
>> > --
>> > 2.25.1
>> >


More information about the Libc-alpha mailing list