Hi Sergei,
thanks for contributing this!
Small general remarks/nits upfront:
The code looks like it hasn't been run through clang-format or
similar. Please make sure that it adheres to the GNU coding
conventions. The same applies to comments. Some of them start
in lowercase.
As you rely on the vector length, please make sure to test various
combinations (also "exotic" ones) like zve32 and zve64.
Also, please specify which configurations it has been tested on.
> * config/riscv/riscv.md (movmem<mode>): Use
> riscv_vector::expand_block_move,
> if and only if we know the entire operation can be performed using one
> vector
> load followed by one vector store
>
> gcc/testsuite/ChangeLog
>
> * gcc.target/riscv/rvv/base/movmem-1.c: New test
Please add a PR target/112109 here. I believe after these
patches have landed we can close that bug.
> ---
> gcc/config/riscv/riscv.md | 22 +++++++
> .../gcc.target/riscv/rvv/base/movmem-1.c | 59 +++++++++++++++++++
> 2 files changed, 81 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/movmem-1.c
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index eed997116b0..88fde290a8a 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -2359,6 +2359,28 @@
> FAIL;
> })
>
> +;; inlining general memmove is a pessimisation: we can't avoid having to
> decide
> +;; which direction to go at runtime, which is costly in instruction count
> +;; however for situations where the entire move fits in one vector operation
> +;; we can do all reads before doing any writes so we don't have to worry
> +;; so generate the inline vector code in such situations
> +;; nb. prefer scalar path for tiny memmoves
> +(define_expand "movmem<mode>"
> + [(parallel [(set (match_operand:BLK 0 "general_operand")
> + (match_operand:BLK 1 "general_operand"))
> + (use (match_operand:P 2 ""))
> + (use (match_operand:SI 3 "const_int_operand"))])]
> + "TARGET_VECTOR"
> +{
> + if ((INTVAL (operands[2]) >= TARGET_MIN_VLEN/8)
If operands[2] is used as an int we need to make sure the constraint
says so. Shouldn't operand[1] be a memory_operand?
> + && (INTVAL (operands[2]) <= TARGET_MIN_VLEN)
> + && riscv_vector::expand_block_move (operands[0], operands[1],
> + operands[2]))
> + DONE;
> + else
> + FAIL;
> +})
> +
> +#define MIN_VECTOR_BYTES (__riscv_v_min_vlen/8)
> +
> +/* tiny memmoves should not be vectorised
> +** f1:
> +** li\s+a2,15
> +** tail\s+memmove
> +*/
> +char * f1 (char *a, char const *b)
> +{
> + return memmove (a, b, 15);
> +}
The < 16 assumption might not hold on embedded targets.
Same with the other tests.
Regards
Robin
