Re: [PATCH v2 2/2] riscv: Add support for str(n)cmp inline expansion

[Philipp Tomsich <philipp.tomsich@vrull.eu>]

Applied to master. Thanks!
Philipp.

On Tue, 12 Sept 2023 at 05:34, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 9/6/23 10:07, Christoph Muellner wrote:
> > From: Christoph Müllner <christoph.muellner@vrull.eu>
> >
> > This patch implements expansions for the cmpstrsi and cmpstrnsi
> > builtins for RV32/RV64 for xlen-aligned strings if Zbb or XTheadBb
> > instructions are available.  The expansion basically emits a comparison
> > sequence which compares XLEN bits per step if possible.
> >
> > This allows to inline calls to strcmp() and strncmp() if both strings
> > are xlen-aligned.  For strncmp() the length parameter needs to be known.
> > The benefits over calls to libc are:
> > * no call/ret instructions
> > * no stack frame allocation
> > * no register saving/restoring
> > * no alignment tests
> >
> > The inlining mechanism is gated by a new switches ('-minline-strcmp' and
> > '-minline-strncmp') and by the variable 'optimize_size'.
> > The amount of emitted unrolled loop iterations can be controlled by the
> > parameter '--param=riscv-strcmp-inline-limit=N', which defaults to 64.
> >
> > The comparision sequence is inspired by the strcmp example
> > in the appendix of the Bitmanip specification (incl. the fast
> > result calculation in case the first word does not contain
> > a NULL byte).  Additional inspiration comes from rs6000-string.c.
> >
> > The emitted sequence is not triggering any readahead pagefault issues,
> > because only aligned strings are accessed by aligned xlen-loads.
> >
> > This patch has been tested using the glibc string tests on QEMU:
> > * rv64gc_zbb/rv64gc_xtheadbb with riscv-strcmp-inline-limit=64
> > * rv64gc_zbb/rv64gc_xtheadbb with riscv-strcmp-inline-limit=8
> > * rv32gc_zbb/rv32gc_xtheadbb with riscv-strcmp-inline-limit=64
> >
> > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/bitmanip.md (*<optab>_not<mode>): Export INSN name.
> >       (<optab>_not<mode>3): Likewise.
> >       * config/riscv/riscv-protos.h (riscv_expand_strcmp): New
> >       prototype.
> >       * config/riscv/riscv-string.cc (GEN_EMIT_HELPER3): New helper
> >       macros.
> >       (GEN_EMIT_HELPER2): Likewise.
> >       (emit_strcmp_scalar_compare_byte): New function.
> >       (emit_strcmp_scalar_compare_subword): Likewise.
> >       (emit_strcmp_scalar_compare_word): Likewise.
> >       (emit_strcmp_scalar_load_and_compare): Likewise.
> >       (emit_strcmp_scalar_call_to_libc): Likewise.
> >       (emit_strcmp_scalar_result_calculation_nonul): Likewise.
> >       (emit_strcmp_scalar_result_calculation): Likewise.
> >       (riscv_expand_strcmp_scalar): Likewise.
> >       (riscv_expand_strcmp): Likewise.
> >       * config/riscv/riscv.md (*slt<u>_<X:mode><GPR:mode>): Export
> >       INSN name.
> >       (@slt<u>_<X:mode><GPR:mode>3): Likewise.
> >       (cmpstrnsi): Invoke expansion function for str(n)cmp.
> >       (cmpstrsi): Likewise.
> >       * config/riscv/riscv.opt: Add new parameter
> >       '-mstring-compare-inline-limit'.
> >       * doc/invoke.texi: Document new parameter
> >       '-mstring-compare-inline-limit'.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.target/riscv/xtheadbb-strcmp-unaligned.c: New test.
> >       * gcc.target/riscv/xtheadbb-strcmp.c: New test.
> >       * gcc.target/riscv/zbb-strcmp-disabled-2.c: New test.
> >       * gcc.target/riscv/zbb-strcmp-disabled.c: New test.
> >       * gcc.target/riscv/zbb-strcmp-unaligned.c: New test.
> >       * gcc.target/riscv/zbb-strcmp.c: New test.
> OK for the trunk.  THanks for pushing this along.
>
> jeff