[v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV

  From: Pan Li <pan2.li@intel.com>

This patch would like to introduce one new gcc option for RVV. To
appoint the bits size of one RVV vector register. Valid arguments to
'-mrvv-vector-bits=' are:

* 64
* 128
* 256
* 512
* 1024
* 2048
* 4096
* 8192
* 16384
* 32768
* 65536
* scalable
* zvl

1. The scalable will be the default values which take min_vlen for
   the riscv_vector_chunks.
2. The zvl will pick up the zvl*b from the march option. For example,
   the mrvv-vector-bits will be 1024 when march=rv64gcv_zvl1024b.
3. Otherwise, it will take the value provide and complain error if none
   of above valid value is given.

This option may influence the code gen when auto-vector. For example,

void test_rvv_vector_bits (int *a, int *b, int *out)
{
  for (int i = 0; i < 8; i++)
    out[i] = a[i] + b[i];
}

It will generate code similar to below when build with
  -march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=zvl

test_rvv_vector_bits:
  ...
  vsetivli	zero,4,e32,m1,ta,ma
  vle32.v	v1,0(a0)
  vle32.v	v2,0(a1)
  vadd.vv	v1,v1,v2
  vse32.v	v1,0(a2)
  ...
  vle32.v	v1,0(a0)
  vle32.v	v2,0(a1)
  vadd.vv	v1,v1,v2
  vse32.v	v1,0(a2)

And it will become more simply similar to below when build with
  -march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=256

test_rvv_vector_bits:
  ...
  vsetivli	zero,8,e32,m2,ta,ma
  vle32.v	v2,0(a0)
  vle32.v	v4,0(a1)
  vadd.vv	v2,v2,v4
  vse32.v	v2,0(a2)

Passed the regression test of rvv.

gcc/ChangeLog:

	* config/riscv/riscv-opts.h (enum rvv_vector_bits_enum): New enum for
	different RVV vector bits.
	* config/riscv/riscv.cc (riscv_convert_vector_bits): New func to
	get the RVV vector bits, with given min_vlen.
	(riscv_convert_vector_chunks): Combine the mrvv-vector-bits
	option with min_vlen to RVV vector chunks.
	(riscv_override_options_internal): Update comments and rename the
	vector chunks.
	* config/riscv/riscv.opt: Add option mrvv-vector-bits.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/base/rvv-vector-bits-1.c: New test.
	* gcc.target/riscv/rvv/base/rvv-vector-bits-2.c: New test.
	* gcc.target/riscv/rvv/base/rvv-vector-bits-3.c: New test.
	* gcc.target/riscv/rvv/base/rvv-vector-bits-4.c: New test.

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/config/riscv/riscv-opts.h                 | 16 ++++++
 gcc/config/riscv/riscv.cc                     | 49 ++++++++++++++++---
 gcc/config/riscv/riscv.opt                    | 47 ++++++++++++++++++
 .../riscv/rvv/base/rvv-vector-bits-1.c        |  6 +++
 .../riscv/rvv/base/rvv-vector-bits-2.c        | 20 ++++++++
 .../riscv/rvv/base/rvv-vector-bits-3.c        | 25 ++++++++++
 .../riscv/rvv/base/rvv-vector-bits-4.c        |  6 +++
 7 files changed, 163 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-4.c
  

I would prefer to only keep zvl and scalable or zvl only, since I
don't see too much value in specifying a value which different from
zvl*b, that's a legacy option used before zvl*b option was introduced,
and the reason to add that is that could used for compatible with
clang/LLVM for riscv_rvv_vector_bits attribute I think?

On Fri, Feb 23, 2024 at 4:06 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> This patch would like to introduce one new gcc option for RVV. To
> appoint the bits size of one RVV vector register. Valid arguments to
> '-mrvv-vector-bits=' are:
>
> * 64
> * 128
> * 256
> * 512
> * 1024
> * 2048
> * 4096
> * 8192
> * 16384
> * 32768
> * 65536
> * scalable
> * zvl
>
> 1. The scalable will be the default values which take min_vlen for
>    the riscv_vector_chunks.
> 2. The zvl will pick up the zvl*b from the march option. For example,
>    the mrvv-vector-bits will be 1024 when march=rv64gcv_zvl1024b.
> 3. Otherwise, it will take the value provide and complain error if none
>    of above valid value is given.
>
> This option may influence the code gen when auto-vector. For example,
>
> void test_rvv_vector_bits (int *a, int *b, int *out)
> {
>   for (int i = 0; i < 8; i++)
>     out[i] = a[i] + b[i];
> }
>
> It will generate code similar to below when build with
>   -march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=zvl
>
> test_rvv_vector_bits:
>   ...
>   vsetivli      zero,4,e32,m1,ta,ma
>   vle32.v       v1,0(a0)
>   vle32.v       v2,0(a1)
>   vadd.vv       v1,v1,v2
>   vse32.v       v1,0(a2)
>   ...
>   vle32.v       v1,0(a0)
>   vle32.v       v2,0(a1)
>   vadd.vv       v1,v1,v2
>   vse32.v       v1,0(a2)
>
> And it will become more simply similar to below when build with
>   -march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=256
>
> test_rvv_vector_bits:
>   ...
>   vsetivli      zero,8,e32,m2,ta,ma
>   vle32.v       v2,0(a0)
>   vle32.v       v4,0(a1)
>   vadd.vv       v2,v2,v4
>   vse32.v       v2,0(a2)
>
> Passed the regression test of rvv.
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv-opts.h (enum rvv_vector_bits_enum): New enum for
>         different RVV vector bits.
>         * config/riscv/riscv.cc (riscv_convert_vector_bits): New func to
>         get the RVV vector bits, with given min_vlen.
>         (riscv_convert_vector_chunks): Combine the mrvv-vector-bits
>         option with min_vlen to RVV vector chunks.
>         (riscv_override_options_internal): Update comments and rename the
>         vector chunks.
>         * config/riscv/riscv.opt: Add option mrvv-vector-bits.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/base/rvv-vector-bits-1.c: New test.
>         * gcc.target/riscv/rvv/base/rvv-vector-bits-2.c: New test.
>         * gcc.target/riscv/rvv/base/rvv-vector-bits-3.c: New test.
>         * gcc.target/riscv/rvv/base/rvv-vector-bits-4.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/config/riscv/riscv-opts.h                 | 16 ++++++
>  gcc/config/riscv/riscv.cc                     | 49 ++++++++++++++++---
>  gcc/config/riscv/riscv.opt                    | 47 ++++++++++++++++++
>  .../riscv/rvv/base/rvv-vector-bits-1.c        |  6 +++
>  .../riscv/rvv/base/rvv-vector-bits-2.c        | 20 ++++++++
>  .../riscv/rvv/base/rvv-vector-bits-3.c        | 25 ++++++++++
>  .../riscv/rvv/base/rvv-vector-bits-4.c        |  6 +++
>  7 files changed, 163 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-1.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-2.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-3.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-4.c
>
> diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> index 4edddbadc37..b2141190731 100644
> --- a/gcc/config/riscv/riscv-opts.h
> +++ b/gcc/config/riscv/riscv-opts.h
> @@ -129,6 +129,22 @@ enum vsetvl_strategy_enum {
>    VSETVL_OPT_NO_FUSION,
>  };
>
> +enum rvv_vector_bits_enum {
> +  RVV_VECTOR_BITS_SCALABLE,
> +  RVV_VECTOR_BITS_ZVL,
> +  RVV_VECTOR_BITS_64 = 64,
> +  RVV_VECTOR_BITS_128 = 128,
> +  RVV_VECTOR_BITS_256 = 256,
> +  RVV_VECTOR_BITS_512 = 512,
> +  RVV_VECTOR_BITS_1024 = 1024,
> +  RVV_VECTOR_BITS_2048 = 2048,
> +  RVV_VECTOR_BITS_4096 = 4096,
> +  RVV_VECTOR_BITS_8192 = 8192,
> +  RVV_VECTOR_BITS_16384 = 16384,
> +  RVV_VECTOR_BITS_32768 = 32768,
> +  RVV_VECTOR_BITS_65536 = 65536,
> +};
> +
>  #define TARGET_ZICOND_LIKE (TARGET_ZICOND || (TARGET_XVENTANACONDOPS && TARGET_64BIT))
>
>  /* Bit of riscv_zvl_flags will set contintuly, N-1 bit will set if N-bit is
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 5e984ee2a55..366d7ece383 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -8801,13 +8801,50 @@ riscv_init_machine_status (void)
>    return ggc_cleared_alloc<machine_function> ();
>  }
>
> -/* Return the VLEN value associated with -march.
> +static int
> +riscv_convert_vector_bits (int min_vlen)
> +{
> +  int rvv_bits = 0;
> +
> +  switch (rvv_vector_bits)
> +    {
> +      case RVV_VECTOR_BITS_SCALABLE:
> +      case RVV_VECTOR_BITS_ZVL:
> +       rvv_bits = min_vlen;
> +       break;
> +      case RVV_VECTOR_BITS_64:
> +      case RVV_VECTOR_BITS_128:
> +      case RVV_VECTOR_BITS_256:
> +      case RVV_VECTOR_BITS_512:
> +      case RVV_VECTOR_BITS_1024:
> +      case RVV_VECTOR_BITS_2048:
> +      case RVV_VECTOR_BITS_4096:
> +      case RVV_VECTOR_BITS_8192:
> +      case RVV_VECTOR_BITS_16384:
> +      case RVV_VECTOR_BITS_32768:
> +      case RVV_VECTOR_BITS_65536:
> +       rvv_bits = rvv_vector_bits;
> +       break;
> +      default:
> +       gcc_unreachable ();
> +    }
> +
> +  if (rvv_bits < min_vlen)
> +    error ("RVV vector bits %d cannot be less than minimal vector length %d",
> +      rvv_bits, min_vlen);
> +
> +  return rvv_bits;
> +}
> +
> +/* Return the VLEN value associated with -march and -mwrvv-vector-bits.
>     TODO: So far we only support length-agnostic value. */
>  static poly_uint16
> -riscv_convert_vector_bits (struct gcc_options *opts)
> +riscv_convert_vector_chunks (struct gcc_options *opts)
>  {
>    int chunk_num;
>    int min_vlen = TARGET_MIN_VLEN_OPTS (opts);
> +  int rvv_bits = riscv_convert_vector_bits (min_vlen);
> +
>    if (min_vlen > 32)
>      {
>        /* When targetting minimum VLEN > 32, we should use 64-bit chunk size.
> @@ -8826,7 +8863,7 @@ riscv_convert_vector_bits (struct gcc_options *opts)
>            - TARGET_MIN_VLEN = 2048bit: [256,256]
>            - TARGET_MIN_VLEN = 4096bit: [512,512]
>            FIXME: We currently DON'T support TARGET_MIN_VLEN > 4096bit.  */
> -      chunk_num = min_vlen / 64;
> +      chunk_num = rvv_bits / 64;
>      }
>    else
>      {
> @@ -8848,7 +8885,7 @@ riscv_convert_vector_bits (struct gcc_options *opts)
>    if (TARGET_VECTOR_OPTS_P (opts))
>      {
>        if (opts->x_riscv_autovec_preference == RVV_FIXED_VLMAX)
> -       return (int) min_vlen / (riscv_bytes_per_vector_chunk * 8);
> +       return (int) rvv_bits / (riscv_bytes_per_vector_chunk * 8);
>        else
>         return poly_uint16 (chunk_num, chunk_num);
>      }
> @@ -8920,8 +8957,8 @@ riscv_override_options_internal (struct gcc_options *opts)
>    if (TARGET_VECTOR && TARGET_BIG_ENDIAN)
>      sorry ("Current RISC-V GCC does not support RVV in big-endian mode");
>
> -  /* Convert -march to a chunks count.  */
> -  riscv_vector_chunks = riscv_convert_vector_bits (opts);
> +  /* Convert -march and -mrvv-vector-bits to a chunks count.  */
> +  riscv_vector_chunks = riscv_convert_vector_chunks (opts);
>  }
>
>  /* Implement TARGET_OPTION_OVERRIDE.  */
> diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
> index 20685c42aed..73ae6abe871 100644
> --- a/gcc/config/riscv/riscv.opt
> +++ b/gcc/config/riscv/riscv.opt
> @@ -607,3 +607,50 @@ Enum(stringop_strategy) String(vector) Value(STRATEGY_VECTOR)
>  mstringop-strategy=
>  Target RejectNegative Joined Enum(stringop_strategy) Var(stringop_strategy) Init(STRATEGY_AUTO)
>  Specify stringop expansion strategy.
> +
> +Enum
> +Name(rvv_vector_bits) Type(enum rvv_vector_bits_enum)
> +The possible RVV vector register lengths:
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(scalable) Value(RVV_VECTOR_BITS_SCALABLE)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(64) Value(RVV_VECTOR_BITS_64)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(128) Value(RVV_VECTOR_BITS_128)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(256) Value(RVV_VECTOR_BITS_256)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(512) Value(RVV_VECTOR_BITS_512)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(1024) Value(RVV_VECTOR_BITS_1024)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(2048) Value(RVV_VECTOR_BITS_2048)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(4096) Value(RVV_VECTOR_BITS_4096)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(8192) Value(RVV_VECTOR_BITS_8192)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(16384) Value(RVV_VECTOR_BITS_16384)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(32768) Value(RVV_VECTOR_BITS_32768)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(65536) Value(RVV_VECTOR_BITS_65536)
> +
> +EnumValue
> +Enum(rvv_vector_bits) String(zvl) Value(RVV_VECTOR_BITS_ZVL)
> +
> +mrvv-vector-bits=
> +Target RejectNegative Joined Enum(rvv_vector_bits) Var(rvv_vector_bits) Init(RVV_VECTOR_BITS_SCALABLE)
> +-mrvv-vector-bits=<number>     Set the number of bits in an RVV vector register.
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-1.c
> new file mode 100644
> index 00000000000..b06d791f383
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-1.c
> @@ -0,0 +1,6 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64 -mrvv-vector-bits=128 -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +/* { dg-error "RVV vector bits 128 cannot be less than minimal vector length 256" "" { target { "riscv*-*-*" } } 0 } */
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-2.c
> new file mode 100644
> index 00000000000..37744339080
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-2.c
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=256 -O3 -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_rvv_vector_bits_256:
> +** ...
> +** vsetivli\s+zero,\s*8,\s*e32,\s*m2,\s*ta,\s*ma
> +** vle32\.v\s+v[0-9]+,\s*0\(a0\)
> +** vle32\.v\s+v[0-9]+,\s*0\(a1\)
> +** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
> +** vse32\.v\s+v[0-9]+,\s*0\(a2\)
> +** ret
> +** ...
> +*/
> +void test_rvv_vector_bits_256 (int *a, int *b, int *out)
> +{
> +  for (int i = 0; i < 8; i++)
> +    out[i] = a[i] + b[i];
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-3.c b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-3.c
> new file mode 100644
> index 00000000000..962cc8ffa6d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-3.c
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=zvl -O3 -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_rvv_vector_bits_zvl:
> +** ...
> +** vsetivli\s+zero,\s*4,\s*e32,\s*m1,\s*ta,\s*ma
> +** vle32\.v\s+v[0-9]+,\s*0\(a0\)
> +** vle32\.v\s+v[0-9]+,\s*0\(a1\)
> +** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
> +** vse32\.v\s+v[0-9]+,\s*0\(a2\)
> +** ...
> +** vle32\.v\s+v[0-9]+,\s*0\(a0\)
> +** vle32\.v\s+v[0-9]+,\s*0\(a1\)
> +** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
> +** vse32\.v\s+v[0-9]+,\s*0\(a2\)
> +** ret
> +** ...
> +*/
> +void test_rvv_vector_bits_zvl (int *a, int *b, int *out)
> +{
> +  for (int i = 0; i < 8; i++)
> +    out[i] = a[i] + b[i];
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-4.c b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-4.c
> new file mode 100644
> index 00000000000..863f96187e1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-4.c
> @@ -0,0 +1,6 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64 -mrvv-vector-bits=invalid-bits -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +/* { dg-error "unrecognized argument in option '-mrvv-vector-bits=invalid-bits" "" { target { "riscv*-*-*" } } 0 } */
> --
> 2.34.1
>
  

On 2/23/24 01:22, Kito Cheng wrote:
> I would prefer to only keep zvl and scalable or zvl only, since I
> don't see too much value in specifying a value which different from
> zvl*b, that's a legacy option used before zvl*b option was introduced,
> and the reason to add that is that could used for compatible with
> clang/LLVM for riscv_rvv_vector_bits attribute I think?
And if we want this (I'm not sure), it really feels like it ought to 
defer to gcc-15.

jeff
  

I personally think it's better to has VLS compile option and attribute in GCC-14.
Since there are many people porting different libraury (eigen/highway/xnnpack/openBLAS,...) with VLS feature,
they test them with Clang.

If we don't support it, we will end up with Clang can compile those lib but GCC-14 can't which will make RISC-V
folks think GCC is still pretty far behind than Clang.

Besides, VLS compile option and attribute are pretty safe codes, I would surprise that it will cause issues on current RVV support.

So, +1 from my side to support VLS compile option and attribute on GCC-14.

But I'd like to CC more RISC-V GCC folks to see the votes. 
If most of the people don't want this in GCC-14 and defer it to GCC-15, I won't insist on it.

Thanks.



juzhe.zhong@rivai.ai
 
From: Jeff Law
Date: 2024-02-23 16:29
To: Kito Cheng; pan2.li
CC: gcc-patches; juzhe.zhong; yanzhang.wang
Subject: Re: [PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
 
 
On 2/23/24 01:22, Kito Cheng wrote:
> I would prefer to only keep zvl and scalable or zvl only, since I
> don't see too much value in specifying a value which different from
> zvl*b, that's a legacy option used before zvl*b option was introduced,
> and the reason to add that is that could used for compatible with
> clang/LLVM for riscv_rvv_vector_bits attribute I think?
And if we want this (I'm not sure), it really feels like it ought to 
defer to gcc-15.
 
jeff
  

> I would prefer to only keep zvl and scalable or zvl only, since I

> don't see too much value in specifying a value which different from

> zvl*b, that's a legacy option used before zvl*b option was introduced,

> and the reason to add that is that could used for compatible with

> clang/LLVM for riscv_rvv_vector_bits attribute I think?



Yes, exactly to be compatible with clang/llvm. Just take zvl is good enough IMO, and update in v2 once we have alignment.



> And if we want this (I'm not sure), it really feels like it ought to defer to gcc-15.

> But I'd like to CC more RISC-V GCC folks to see the votes.

> If most of the people don't want this in GCC-14 and defer it to GCC-15, I won't insist on it.



Sure, let’s wait for a while.



Pan

From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
Sent: Friday, February 23, 2024 4:38 PM
To: jeffreyalaw <jeffreyalaw@gmail.com>; kito.cheng <kito.cheng@gmail.com>; Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>; Robin Dapp <rdapp.gcc@gmail.com>; palmer <palmer@rivosinc.com>; vineetg <vineetg@rivosinc.com>; Patrick O'Neill <patrick@rivosinc.com>; Edwin Lu <ewlu@rivosinc.com>
Subject: Re: Re: [PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV

I personally think it's better to has VLS compile option and attribute in GCC-14.
Since there are many people porting different libraury (eigen/highway/xnnpack/openBLAS,...) with VLS feature,
they test them with Clang.

If we don't support it, we will end up with Clang can compile those lib but GCC-14 can't which will make RISC-V
folks think GCC is still pretty far behind than Clang.

Besides, VLS compile option and attribute are pretty safe codes, I would surprise that it will cause issues on current RVV support.

So, +1 from my side to support VLS compile option and attribute on GCC-14.

But I'd like to CC more RISC-V GCC folks to see the votes.
If most of the people don't want this in GCC-14 and defer it to GCC-15, I won't insist on it.

Thanks.

________________________________
juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>

From: Jeff Law<mailto:jeffreyalaw@gmail.com>
Date: 2024-02-23 16:29
To: Kito Cheng<mailto:kito.cheng@gmail.com>; pan2.li<mailto:pan2.li@intel.com>
CC: gcc-patches<mailto:gcc-patches@gcc.gnu.org>; juzhe.zhong<mailto:juzhe.zhong@rivai.ai>; yanzhang.wang<mailto:yanzhang.wang@intel.com>
Subject: Re: [PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV


On 2/23/24 01:22, Kito Cheng wrote:
> I would prefer to only keep zvl and scalable or zvl only, since I
> don't see too much value in specifying a value which different from
> zvl*b, that's a legacy option used before zvl*b option was introduced,
> and the reason to add that is that could used for compatible with
> clang/LLVM for riscv_rvv_vector_bits attribute I think?
And if we want this (I'm not sure), it really feels like it ought to
defer to gcc-15.

jeff
  

+CC Greg who might also have some bits in flight here.

On 2/23/24 01:23, Li, Pan2 wrote:
>
> > I would prefer to only keep zvl and scalable or zvl only, since I
>
> > don't see too much value in specifying a value which different from
>
> > zvl*b, that's a legacy option used before zvl*b option was introduced,
>

+1

> > and the reason to add that is that could used for compatible with
>
> > clang/LLVM for riscv_rvv_vector_bits attribute I think?
>
>  
>
> Yes, exactly to be compatible with clang/llvm. Just take zvl is good
> enough IMO, and update in v2 once we have alignment.
>

+1

It seems you would also want to implement feature macro
__riscv_v_fixed_vlen which llvm does and downstream projects such as
xsimd rely on.

>  
>
> > And if we want this (I'm not sure), it really feels like it ought to
> defer to gcc-15.
>
> > But I'd like to CC more RISC-V GCC folks to see the votes.
>
> > If most of the people don't want this in GCC-14 and defer it to
> GCC-15, I won't insist on it.
>
>  
>
> Sure, let’s wait for a while.
>

Sure it is late in cycle, but I DO agree to gcc-14 inclusion. And thats
because it is related to end user experience: gcc is merely catching up
to what llvm already has.  Rivos folks working on some downstream
projects have brought up this disparity internally. If we don't now, the
projects will have to carry that for posterity. For that reason I'd
consider this as *fix* category such as a VSETVL fix.

P.S. Some of this is captured in PR/112817 and it would be nice to
update stuff there too.

But to me what is more important under same umbrella, for gcc-14 still,
is *attribute riscv_rvv_vector_bits* for VLS codegen (also discussed in
same PR/112817).
Again this is from same devs for downstream projects complain that gcc
is not up to par with llvm there - and this is no longer just syntactic
sugar tucked away in a makefile. They actively need #ifdef ugliness in
their code to handle llvm vs. gcc. Granted this part of work might (or
not) be trivial, specially this late, but I'm just putting it out there
for consideration.

Thx,
-Vineet



>  
>
> Pan
>
>  
>
> *From:*juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
> *Sent:* Friday, February 23, 2024 4:38 PM
> *To:* jeffreyalaw <jeffreyalaw@gmail.com>; kito.cheng
> <kito.cheng@gmail.com>; Li, Pan2 <pan2.li@intel.com>
> *Cc:* gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang
> <yanzhang.wang@intel.com>; Robin Dapp <rdapp.gcc@gmail.com>; palmer
> <palmer@rivosinc.com>; vineetg <vineetg@rivosinc.com>; Patrick O'Neill
> <patrick@rivosinc.com>; Edwin Lu <ewlu@rivosinc.com>
> *Subject:* Re: Re: [PATCH v1] RISC-V: Introduce gcc option
> mrvv-vector-bits for RVV
>
>  
>
> I personally think it's better to has VLS compile option and attribute
> in GCC-14.
>
> Since there are many people porting different libraury
> (eigen/highway/xnnpack/openBLAS,...) with VLS feature,
>
> they test them with Clang.
>
>  
>
> If we don't support it, we will end up with Clang can compile those
> lib but GCC-14 can't which will make RISC-V
>
> folks think GCC is still pretty far behind than Clang.
>
>  
>
> Besides, VLS compile option and attribute are pretty safe codes, I
> would surprise that it will cause issues on current RVV support.
>
>  
>
> So, +1 from my side to support VLS compile option and attribute on GCC-14.
>
>  
>
> But I'd like to CC more RISC-V GCC folks to see the votes. 
>
> If most of the people don't want this in GCC-14 and defer it to
> GCC-15, I won't insist on it.
>
>  
>
> Thanks.
>
>  
>
> ------------------------------------------------------------------------
>
> juzhe.zhong@rivai.ai
>
>      
>
>     *From:* Jeff Law <mailto:jeffreyalaw@gmail.com>
>
>     *Date:* 2024-02-23 16:29
>
>     *To:* Kito Cheng <mailto:kito.cheng@gmail.com>; pan2.li
>     <mailto:pan2.li@intel.com>
>
>     *CC:* gcc-patches <mailto:gcc-patches@gcc.gnu.org>; juzhe.zhong
>     <mailto:juzhe.zhong@rivai.ai>; yanzhang.wang
>     <mailto:yanzhang.wang@intel.com>
>
>     *Subject:* Re: [PATCH v1] RISC-V: Introduce gcc option
>     mrvv-vector-bits for RVV
>
>      
>
>      
>
>     On 2/23/24 01:22, Kito Cheng wrote:
>
>     > I would prefer to only keep zvl and scalable or zvl only, since I
>
>     > don't see too much value in specifying a value which different from
>
>     > zvl*b, that's a legacy option used before zvl*b option was
>     introduced,
>
>     > and the reason to add that is that could used for compatible with
>
>     > clang/LLVM for riscv_rvv_vector_bits attribute I think?
>
>     And if we want this (I'm not sure), it really feels like it ought to
>
>     defer to gcc-15.
>
>      
>
>     jeff
>
>      
>
>      
>