Re: [PATCH] RISC-V: Refactor RVV machine modes

[Kito Cheng <kito.cheng@sifive.com>]

Thansk, that's really awesome!

One comment about mode iterator is the naming seems like still
prefixed with VNX which inconsistent with new mode naming scheme.

> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index cd5b19457f8..03e19259505 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -121,45 +121,30 @@
>    DONE;
>  })
>
> -(define_expand "len_mask_gather_load<VNX16_QHSD:mode><VNX16_QHSDI:mode>"
> -  [(match_operand:VNX16_QHSD 0 "register_operand")
> +(define_expand "len_mask_gather_load<VNX16_QHS:mode><VNX16_QHSI:mode>"

Why DI is gone? and I saw many other pattern has similar issue?

> +  [(match_operand:VNX16_QHS 0 "register_operand")
>     (match_operand 1 "pmode_reg_or_0_operand")
> -   (match_operand:VNX16_QHSDI 2 "register_operand")
> -   (match_operand 3 "<VNX16_QHSD:gs_extension>")
> -   (match_operand 4 "<VNX16_QHSD:gs_scale>")
> +   (match_operand:VNX16_QHSI 2 "register_operand")
> +   (match_operand 3 "<VNX16_QHS:gs_extension>")
> +   (match_operand 4 "<VNX16_QHS:gs_scale>")
>     (match_operand 5 "autovec_length_operand")
>     (match_operand 6 "const_0_operand")
> -   (match_operand:<VNX16_QHSD:VM> 7 "vector_mask_operand")]
> +   (match_operand:<VNX16_QHS:VM> 7 "vector_mask_operand")]
>    "TARGET_VECTOR"
>  {
>    riscv_vector::expand_gather_scatter (operands, true);
>    DONE;
>  })
>
> -(define_expand "len_mask_gather_load<VNX32_QHS:mode><VNX32_QHSI:mode>"
> -  [(match_operand:VNX32_QHS 0 "register_operand")
> +(define_expand "len_mask_gather_load<VNX32_QH:mode><VNX32_QHI:mode>"

Like this, SI is gone?

> @@ -2172,7 +2145,7 @@ preferred_simd_mode (scalar_mode mode)
>       riscv_autovec_lmul < RVV_M2. Since GCC loop vectorizer report ICE when we
>       enable -march=rv64gc_zve32* and -march=rv32gc_zve64*. in the
>       'can_duplicate_and_interleave_p' of tree-vect-slp.cc. Since we have
> -     VNx1SImode in -march=*zve32* and VNx1DImode in -march=*zve64*, they are
> +     RVVM1SImode in -march=*zve32* and RVVM1DImode in -march=*zve64*, they are

This comment might need to review.

>       enabled in targetm. vector_mode_supported_p and SLP vectorizer will try to
>       use them. Currently, we can support auto-vectorization in
>       -march=rv32_zve32x_zvl128b. Wheras, -march=rv32_zve32x_zvl32b or
> @@ -2371,7 +2344,7 @@ autovectorize_vector_modes (vector_modes *modes, bool)
>        poly_uint64 full_size
>         = BYTES_PER_RISCV_VECTOR * ((int) riscv_autovec_lmul);
>
> -      /* Start with a VNxYYQImode where YY is the number of units that
> +      /* Start with a RVVM1QImode where YY is the number of units that
>          fit a whole vector.
>          Then try YY = nunits / 2, nunits / 4 and nunits / 8 which
>          is guided by the extensions we have available (vf2, vf4 and vf8).

We don't have YY now :P

> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index f1f5a73389e..c47bcd2b412 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -972,8 +972,8 @@ riscv_valid_lo_sum_p (enum riscv_symbol_type sym_type, machine_mode mode,
>  }
>
>  /* Return true if mode is the RVV enabled mode.
> -   For example: 'VNx1DI' mode is disabled if MIN_VLEN == 32.
> -   'VNx1SI' mode is enabled if MIN_VLEN == 32.  */
> +   For example: 'RVVM1DI' mode is disabled if MIN_VLEN == 32.
> +   'RVVM1SI' mode is enabled if MIN_VLEN == 32.  */

This comment need to updated :)

>
>  bool
>  riscv_v_ext_vector_mode_p (machine_mode mode)
> @@ -1023,11 +1023,36 @@ riscv_v_ext_mode_p (machine_mode mode)
>  poly_int64
>  riscv_v_adjust_nunits (machine_mode mode, int scale)
>  {
> +  gcc_assert (GET_MODE_CLASS (mode) == MODE_VECTOR_BOOL);
>    if (riscv_v_ext_mode_p (mode))
> -    return riscv_vector_chunks * scale;
> +    {
> +      if (TARGET_MIN_VLEN == 32)
> +       scale = scale / 2;
> +      return riscv_vector_chunks * scale;
> +    }
>    return scale;
>  }

> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index 19683152259..643e7ea7330 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -1040,6 +1040,7 @@ extern unsigned riscv_stack_boundary;
>  extern unsigned riscv_bytes_per_vector_chunk;
>  extern poly_uint16 riscv_vector_chunks;
>  extern poly_int64 riscv_v_adjust_nunits (enum machine_mode, int);
> +extern poly_int64 riscv_v_adjust_nunits (machine_mode, bool, int, int);
>  extern poly_int64 riscv_v_adjust_precision (enum machine_mode, int);
>  extern poly_int64 riscv_v_adjust_bytesize (enum machine_mode, int);
>  /* The number of bits and bytes in a RVV vector.  */

> +  (cond [(eq_attr "mode" "RVVM8QI,RVVM1BI") (symbol_ref "riscv_vector::get_vlmul(E_RVVM8QImode)")

This could be just using constant value rather than ask
riscv_vector::get_vlmul now :)

> +        (eq_attr "mode" "RVVM8QI,RVVM1BI") (symbol_ref "riscv_vector::get_ratio(E_RVVM8QImode)")

They are constant now too.