Re: [PATCH 2/2] riscv: vectorised mem* and str* functions

[Sergei Lewis <slewis@rivosinc.com>]

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

Thank you for the feedback!

The idea of this changeset is to provide vectorised implementations for the
specific case where someone opts in by explicitly describing the build
target as supporting the "V" extension via the build environment when
building glibc; where this is not done, existing behaviour is unchanged.

I agree that multiarch/ifuncs absolutely has to be the way going forward.
Most people in the wild will be using prebuilt binaries, and generally
these will be built for the lowest common denominator; we do want to use
optimal code where we can detect the target supports it even if this was
not known at build time. Moreover, we can reasonably support much more
specific optimisation (e.g. specialise on VLENB, determine whether high
LMUL or low LMUL / more unrolls is preferable etc) via ifuncs than at
compile time.

Even once the compiler issues are sorted out, coming up with a robust
mechanism for probing the relevant properties of the environment for ifuncs
is a significant engineering challenge on RISCV, and one that I expect to
be addressing going forward. Once that is in place, it will be possible via
ifuncs to make use of the vectorised implementations even in binaries where
the V extension was not explicitly target at compile time, if it is
available at runtime.

Getting the optimised implementations out early for anyone opting in to
them will help anyone else working in this space and anyone looking at
benchmarks on specific targets; it would also get the code in front of more
eyeballs than just mine while I crack on with ifuncs.

Certainly, though, if the ifuncs infrastructure has already been created by
someone else and can be shared, that would be super helpful, and if the
community would prefer I keep these private until that point we can do that.

On Wed, Feb 1, 2023 at 5:07 PM Jeff Law <jeffreyalaw@gmail.com> wrote:

>
>
> On 2/1/23 09:42, Florian Weimer wrote:
> > * Jeff Law via Libc-alpha:
> >
> >> On 2/1/23 02:52, Sergei Lewis wrote:
> >>> Initial implementations of memchr, memcmp, memcpy, memmove, memset,
> strchr,
> >>> strcmp, strcpy, strlen, strncmp, strncpy, strnlen, strrchr, strspn
> >>> targeting the riscv "V" extension, version 1.0
> >>> The vectorised implementations assume VLENB of at least 128 and at
> >>> least 32
> >>> registers (as mandated by the "V" extension spec). They also assume
> that
> >>> VLENB is a power of two which is no larger than the page size, and (as
> >>> vectorised code in glibc for other platforms does) that it is safe to
> read
> >>> past null terminators / buffer ends provided one does not cross a page
> >>> boundary.
> >>> Signed-off-by: Sergei Lewis <slewis@rivosinc.com>
> >>> ---
> >>>    sysdeps/riscv/rv64/rvv/Implies     |   2 +
> >>>    sysdeps/riscv/rv64/rvv/memchr.S    | 127 +++++++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/memcmp.S    |  93 ++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/memcpy.S    | 154 +++++++++++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/memmove.c   |  22 ++++
> >>>    sysdeps/riscv/rv64/rvv/memset.S    |  89 ++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strchr.S    |  92 ++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strchrnul.c |  22 ++++
> >>>    sysdeps/riscv/rv64/rvv/strcmp.S    | 108 +++++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strcpy.S    |  72 +++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strcspn.c   |  22 ++++
> >>>    sysdeps/riscv/rv64/rvv/strlen.S    |  67 ++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strncmp.S   | 104 ++++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strncpy.S   |  96 +++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strnlen.S   |  81 +++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strrchr.S   |  88 ++++++++++++++
> >>>    sysdeps/riscv/rv64/rvv/strspn.S    | 189
> +++++++++++++++++++++++++++++
> >> Does this need to be revamped given the recent push to do more with
> >> generic code and target specific hooks for mem* and str*?
> >>
> >> Shouldn't the implementations be in a multiarch directory?  I would
> >> fully expect we're going to need both a vector and scalar
> >> implementation selected by an ifunc.
> >
> > I think most RISC-V GCC compilers won't have enabled IFUNC support?
> > Looking at gcc/config.gcc in GCC 12, I see this:
> >
> > *-*-linux* | *-*-gnu*)
> >          case ${target} in
> >          aarch64*-* | arm*-* | i[34567]86-* | powerpc*-* | s390*-* |
> sparc*-* | x86_64-* | loongarch*-*)
> >                  default_gnu_indirect_function=yes
> >                  ;;
> >          esac
> >
> > But maybe that's not the right place to look at?
> Clearly something we need to fix.
>
> I'd hesitate to turn on the gcc bits without having the kernel/user
> interface settled.  There was a proposal that added a syscall to get the
> processor capabilities -- I'd asked the authors to reach out to you and
> Carlos on whether or not that was acceptable for glibc.  I'm not sure if
> that happened or not.
>
> >
> > We have an assembler hack to be able to still build IFUNC resolvers
> > written in C, but I don't know if this works on RISC-V.
> It probably doesn't yet.
>
> >
> > Ideally the GCC defaults would change, too, and well before IFUNCs are
> > in common use.
> They're not common, but I suspect that'll change in the next ~6 months.
>
> Jeff
>