[PATCH v6 3/3] RISC-V: add riscv vector support for memcpy
daichengrong
daichengrong@iscas.ac.cn
Wed Feb 19 08:21:54 GMT 2025
在 2025/2/15 8:53:07, Palmer Dabbelt 写道:
> 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.
>
Running bench-memcpy-random on K230, it has a clear improvement.
{
"timing_type": "hp_timing",
"functions": {
"memcpy": {
"bench-variant": "random",
"ifuncs": ["__memcpy_vector", "__memcpy_generic"],
"results": [
{
"length": 32768,
"timings": [932459, 1.21299e+06]
},
{
"length": 65536,
"timings": [1.17527e+06, 1.41768e+06]
},
{
"length": 131072,
"timings": [1.61894e+06, 1.87552e+06]
},
{
"length": 262144,
"timings": [2.6958e+06, 3.00319e+06]
},
{
"length": 524288,
"timings": [3.66587e+06, 3.91688e+06]
},
{
"length": 1048576,
"timings": [4.37236e+06, 4.59822e+06]
}]
}
}
}
> 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