Re: [PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
+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.zh...@rivai.ai > *Sent:* Friday, February 23, 2024 4:38 PM > *To:* jeffreyalaw ; kito.cheng > ; Li, Pan2 > *Cc:* gcc-patches ; Wang, Yanzhang > ; Robin Dapp ; palmer > ; vineetg ; Patrick O'Neill > ; Edwin Lu > *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.zh...@rivai.ai > > > > *From:* Jeff Law <mailto:jeffreya...@gmail.com> > > *Date:* 2024-02-23 16:29 > > *To:* Kito Cheng <mailto:kito.ch...@gmail.com>; pan2.li > <mailto:pan2...@intel.com> > > *CC:* gcc-patches <mailto:gcc-patches@gcc.gnu.org>; juzhe.zhong > <mailto:juzhe.zh...@rivai.ai>; yanzhang.wang > <mailto:yanzhang.w...@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 > > > > >
RE: Re: [PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
> 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.zh...@rivai.ai Sent: Friday, February 23, 2024 4:38 PM To: jeffreyalaw ; kito.cheng ; Li, Pan2 Cc: gcc-patches ; Wang, Yanzhang ; Robin Dapp ; palmer ; vineetg ; Patrick O'Neill ; Edwin Lu 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.zh...@rivai.ai<mailto:juzhe.zh...@rivai.ai> From: Jeff Law<mailto:jeffreya...@gmail.com> Date: 2024-02-23 16:29 To: Kito Cheng<mailto:kito.ch...@gmail.com>; pan2.li<mailto:pan2...@intel.com> CC: gcc-patches<mailto:gcc-patches@gcc.gnu.org>; juzhe.zhong<mailto:juzhe.zh...@rivai.ai>; yanzhang.wang<mailto:yanzhang.w...@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
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.zh...@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
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
Re: [PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
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 wrote:
>
> From: Pan Li
>
> 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
> ---
> 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 ();
> }
>
> -/* Return the VLEN value associated with -march.
> +static int
>
[PATCH v1] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
From: Pan Li
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
---
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 ();
}
-/* 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;
+
