Re: [PATCH] RISC-V: Vectorized str(n)cmp and strlen.

[Jeff Law <jeffreyalaw@gmail.com>]

On 11/30/23 15:22, Robin Dapp wrote:
> Hi,
> 
> this adds vectorized implementations of strcmp and strncmp as well as
> strlen.  strlen falls back to the previously implemented rawmemchr.
> Also, it fixes a rawmemchr bug causing a SPEC2017 execution failure:
> We would only ever increment the source address by 1 regardless of
> the input type.
> 
> The patch also changes the stringop-strategy handling slightly:
> auto is now an aggregate (including vector and scalar,
> possibly more in the future) and expansion functions try all
> matching strategies in their preferred order.
> 
> As before, str* expansion is guarded by -minline-str* and not active
> by default.  This might change in the future as I would rather have
> those on by default.  As of now, though, there is still a latent bug:
> 
> With -minline-strlen and -minline-strcmp we have several execution
> failures in gcc.c-torture/execute/builtins/.  From my initial analysis
> it looks like we don't insert a vsetvl at the right spot (which would
> be right after a setjmp in those cases).  This leaves the initial
> vle8ff without a proper vtype or vl causing a SIGILL.
> Still, I figured I'd rather post the patch as-is so the bug can be
> reproduced upstream.
> 
> Regards
>   Robin
> 
> gcc/ChangeLog:
> 
> 	PR target/112109
> 
> 	* config/riscv/riscv-opts.h (enum riscv_stringop_strategy_enum):
> 	Rename.
> 	(enum stringop_strategy_enum): To this.
> 	* config/riscv/riscv-protos.h (expand_rawmemchr): Add strlen
> 	param.
> 	(expand_strcmp): Define.
> 	* config/riscv/riscv-string.cc (riscv_expand_strcmp):  Add
> 	vector version.
> 	(riscv_expand_strlen): Ditto.
> 	(riscv_expand_block_move_scalar): Handle existing scalar expansion.
> 	(riscv_expand_block_move): Expand to either vector or scalar
> 	version.
> 	(expand_block_move): Add stringop strategy.
> 	(expand_rawmemchr): Handle strlen and fix increment bug.
> 	(expand_strcmp): New expander.
> 	* config/riscv/riscv.md: Add vector.
> 	* config/riscv/riscv.opt: Ditto.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/rvv/autovec/builtin/strcmp-run.c: New test.
> 	* gcc.target/riscv/rvv/autovec/builtin/strcmp.c: New test.
> 	* gcc.target/riscv/rvv/autovec/builtin/strlen-run.c: New test.
> 	* gcc.target/riscv/rvv/autovec/builtin/strlen.c: New test.
Do you want to extract the rawmemchr fix and push it forward 
independently?  Or do we think the vsetvl issue will be resolved quickly 
enough that extraction of that fix would just be "make work"?

Jeff