Re: [PATCH v3 0/5] riscv: Vectorized mem*/str* function

[Sergei Lewis <slewis@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 8562 bytes --]

Yes, I've been in email conversation with Yun and have an updated version
of that patchset locally. Since a key design goal of this approach is to
explicitly align memory accesses instead of using fault only first loads in
order to take advantage of uarch specific fast paths, my plan here is to
submit an ifunc based version of the patch, gated behind suitable tests to
make sure it is only enabled when it is not only safe but also outperforms
more generic alternatives, once suitable hwcaps support is available to
enable appropriate checks to be written. There seems little point sending
an update to the list until it can be gated in this manner, but I am
certainly happy to do so if there is interest.

On Wed, May 10, 2023 at 10:02 AM Hau Hsu via Libc-alpha <
libc-alpha@sourceware.org> wrote:

>
>
> > On May 8, 2023, at 10:06 PM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Thu, 04 May 2023 00:48:46 PDT (-0700), hau.hsu@sifive.com <mailto:
> hau.hsu@sifive.com> wrote:
> >> This is v3 patchset of adding vectorized mem*/str* functions for
> >> RISC-V.
> >>
> >> This patch proposes implementations of memchr, memcmp, memcpy, memmove,
> >> memset, strcat, strchr, strcmp, strcpy, strlen, strncat, strncmp,
> >> strncpy and strnlen that leverage the RISC-V V extension (RVV), version
> >> 1.0 (https://github.com/riscv/riscv-v-spec/releases/tag/v1.0). These
> >> routines are from https://github.com/sifive/sifive-libc, which we agree
> >> to be contributed to the Free Software Foundation. With regards to
> >> IFUNC, some details concerning `hwcap` are still under discussion in the
> >> community. For the purposes of reviewing this patch, we have temporarily
> >> opted for RVV delegation at compile time. Once the `hwcap` mechanism is
> >> ready, we’ll rebase on it.
> >
> > IMO it's fine to allow users to build a glibc that assumes the V
> extension, so we don't need to block this on having the dynamic probing
> working.
> >
> > That said, we do need to get the Linux uABI sorted out as right now we
> can't even turn on V for userspace.
>
> Does this mean that our current implementation that checks whether a user
> is building
> glibc with RVV compile flags is acceptable, at least for now?
>
> >> These routines assume VLEN is at least 32 bits, as is required by all
> >> currently defined vector extensions, and they support arbitrarily large
> >> VLEN. All implementations work for both RV32 and RV64 platforms, and
> >> make no assumptions about page size.
> >>
> >> The `mem*` (known-length) routines use LMUL=8 to minimize dynamic code
> >> size, while the `str*` (unknown-length) routines use LMUL=1 instead.
> >> Longer LMUL will still minimize dynamic code size for the latter
> >> routines, but it will also increase the cost of the remainder/tail loop:
> >> more data loaded and comparisons performed past the `\0`. This overhead
> >> will be particularly pronounced for smaller strings.
> >>
> >> Measured performance improvements of the vectorized ("rvv")
> >> implementations vs. the existing Glibc ("scalar") implementations are as
> >
> > There's been a few of these posted so I forget exactly where the reviews
> ended up, but at least one of the asks was to compare these against
> vectorized versions of the standard glibc routines.
>
> I guess you mean this thread?
> https://sourceware.org/pipermail/libc-alpha/2023-April/147056.html <
> https://sourceware.org/pipermail/libc-alpha/2023-April/147056.html>
>
> >
> >> follows:
> >> memchr: 85% time savings (i.e., if scalar is 100ms, then rvv is 15ms)
> >> memcmp: 55%
> >> memcpy: 88%
> >> memmove: 80%
> >> memset: 88%
> >> strcmp: 85%
> >> strlen: 70%
> >> strcat: 53%
> >> strchr: 85%
> >> strcpy: 70%
> >> strncmp 90%
> >> strncat: 50%
> >> strncpy: 60%
> >> strnlen: 80%
> >> Above data are collected on SiFive X280 (FPGA simulation), across a wide
> >> range of problem sizes.
> >
> > That's certainly more realistic of a system than the QEMU results, but
> the general consensus has been that FPGA-based development systems don't
> count as hardware -- not so much because of the FPGA, but because we're
> looking for production systems.  If there's real production systems running
> on FPGAs that's a different story, but it looks like these are just
> pre-silicon development systems.
>
> Yes, the FPGA environment is not a production system, but currently we
> don't have
> any RVV products in hand nor similar simulation platforms, this is the
> best benchmarking environment we have.
>
> Yun Hsiang also ran benchmarks base on Sergei Lewis's commits in the same
> environment:
> https://sourceware.org/pipermail/libc-alpha/2023-May/147821.html <
> https://sourceware.org/pipermail/libc-alpha/2023-May/147821.html>
> Out implementations in this have less instruction/cycle count in most
> cases.
>
> When benchmarking Sergei Lewis's commits, Yun Hsiang encountered some
> errors.
> He helped to debug the source code and pointed out some issues:
> https://sourceware.org/pipermail/libc-alpha/2023-May/147820.html <
> https://sourceware.org/pipermail/libc-alpha/2023-May/147820.html>
>
> We know that different uarch variants might prefer different code, but our
> implementation is more generic.
> It follows the RVV spec 1.0 and has no other hardware assumptions.
> The benchmarking also shows good results, compare with the default and
> other proposed implementations.
>
>
> >
> >> v1: https://sourceware.org/pipermail/libc-alpha/2023-March/145976.html
> >>  * add RISC-V vectoriezed mem*/str* functions
> >>
> >> v2: https://sourceware.org/pipermail/libc-alpha/2023-April/147519.html
> >>  * include the __memcmpeq function
> >>  * set lmul=1 for memcmp for generality
> >>
> >> v3:
> >>  * remove "Contributed by" comments
> >>  * fix licesnce headers
> >>  * avoid using camelcase variables
> >>  * avoid using C99 one line comment
> >>
> >> Jerry Shih (2):
> >>  riscv: vectorized mem* functions
> >>  riscv: vectorized str* functions
> >>
> >> Nick Knight (1):
> >>  riscv: vectorized strchr and strnlen functions
> >>
> >> Vincent Chen (1):
> >>  riscv: Enabling vectorized mem*/str* functions in build time
> >>
> >> Yun Hsiang (1):
> >>  riscv: add vectorized __memcmpeq
> >>
> >> scripts/build-many-glibcs.py   | 10 ++++
> >> sysdeps/riscv/preconfigure     | 19 ++++++++
> >> sysdeps/riscv/preconfigure.ac  | 18 +++++++
> >> sysdeps/riscv/rv32/rvv/Implies |  2 +
> >> sysdeps/riscv/rv64/rvv/Implies |  2 +
> >> sysdeps/riscv/rvv/memchr.S     | 62 ++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/memcmp.S     | 70 +++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/memcmpeq.S   | 67 ++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/memcpy.S     | 50 +++++++++++++++++++
> >> sysdeps/riscv/rvv/memmove.S    | 71 +++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/memset.S     | 49 +++++++++++++++++++
> >> sysdeps/riscv/rvv/strcat.S     | 71 +++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/strchr.S     | 62 ++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/strcmp.S     | 88 ++++++++++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/strcpy.S     | 55 +++++++++++++++++++++
> >> sysdeps/riscv/rvv/strlen.S     | 53 ++++++++++++++++++++
> >> sysdeps/riscv/rvv/strncat.S    | 82 +++++++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/strncmp.S    | 84 ++++++++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/strncpy.S    | 85 ++++++++++++++++++++++++++++++++
> >> sysdeps/riscv/rvv/strnlen.S    | 55 +++++++++++++++++++++
> >> 20 files changed, 1055 insertions(+)
> >> create mode 100644 sysdeps/riscv/rv32/rvv/Implies
> >> create mode 100644 sysdeps/riscv/rv64/rvv/Implies
> >> create mode 100644 sysdeps/riscv/rvv/memchr.S
> >> create mode 100644 sysdeps/riscv/rvv/memcmp.S
> >> create mode 100644 sysdeps/riscv/rvv/memcmpeq.S
> >> create mode 100644 sysdeps/riscv/rvv/memcpy.S
> >> create mode 100644 sysdeps/riscv/rvv/memmove.S
> >> create mode 100644 sysdeps/riscv/rvv/memset.S
> >> create mode 100644 sysdeps/riscv/rvv/strcat.S
> >> create mode 100644 sysdeps/riscv/rvv/strchr.S
> >> create mode 100644 sysdeps/riscv/rvv/strcmp.S
> >> create mode 100644 sysdeps/riscv/rvv/strcpy.S
> >> create mode 100644 sysdeps/riscv/rvv/strlen.S
> >> create mode 100644 sysdeps/riscv/rvv/strncat.S
> >> create mode 100644 sysdeps/riscv/rvv/strncmp.S
> >> create mode 100644 sysdeps/riscv/rvv/strncpy.S
> >> create mode 100644 sysdeps/riscv/rvv/strnlen.S
>
>