[PATCH v6 3/3] RISC-V: add riscv vector support for memcpy
Palmer Dabbelt
palmer@rivosinc.com
Sat Feb 15 00:53:07 GMT 2025
On Fri, 14 Feb 2025 16:43:39 PST (-0800), aswaterman@gmail.com wrote:
> On Fri, Feb 14, 2025 at 9:22 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>>
>> 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.
>
> Indeed. But there's benefit to making progress now. My
> recommendation is to incorporate the routines in the ISA spec, as
> they're sensible defaults that will provide speedup over the status
> quo across a broad range of implementations, even if they aren't quite
> optimal for some. Routines optimized for specific uarches can be
> contributed over time.
Ya, it's hard to get a whole lot simpler than the ones in the ISA
manual. I think Jeff had a patch set to add them for the mem/str
routines, but I don't think it got posted?
Shouldn't be too hard to just run them on some V-supporting board (we've
got a bunch of the Banana Pi boards floating around). IIRC there's some
benchmarks in the test suite so it sort of needs to get done before
merging them anyway. If they're not faster than the scalar code on any
HW then something's gone off the rails, but hopefully we don't need to
worry about that.
Then if someone's got other fancy stuff that makes some design go fast
we can look at it, but at least we've got a reasonable baseline to
compare against to tell if it's worth taking the extra complexity.
>> > 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