diff mbox series

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

Message ID 20240228103532.2079576-1-pan2.li@intel.com
State New
Headers show
Series [v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV | expand

Commit Message

Li, Pan2 Feb. 28, 2024, 10:35 a.m. UTC 
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:

* scalable
* zvl

The scalable will pick up the zvl*b in the march as the minimal vlen.
For example, the minimal vlen will be 512 when
march=rv64gcv_zvl512b and mrvv-vector-bits=scalable.

The zvl will pick up the zvl*b in the march as exactly vlen.
For example, the vlen will be 1024 exactly when
march=rv64gcv_zvl1024b and mrvv-vector-bits=zvl.

Given below sample:

void test_rvv_vector_bits ()
{
  vint32m1_t x;
  asm volatile ("def %0": "=vr"(x));
  asm volatile (""::: "v0",   "v1",  "v2",  "v3",  "v4",  "v5",  "v6",  "v7",
                      "v8",   "v9", "v10", "v11", "v12", "v13", "v14", "v15",
                      "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
                      "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");
  asm volatile ("use %0": : "vr"(x));
}

With -march=rv64gcv_zvl128b -mrvv-vector-bits=scalable we have (for min_vlen >= 128)
  csrr    t0,vlenb
  sub     sp,sp,t0
  def v1
  vs1r.v  v1,0(sp)
  vl1re32.v       v1,0(sp)
  use v1
  csrr    t0,vlenb
  add     sp,sp,t0
  jr      ra

With -march=rv64gcv_zvl128b -mrvv-vector-bits=zvl we have (for vlen = 128)
  addi    sp,sp,-16
  def v1
  vs1r.v  v1,0(sp)
  vl1re32.v       v1,0(sp)
  use v1
  addi    sp,sp,16
  jr      ra

The below test are passed for this patch.

* The riscv fully regression test.

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.
	* gcc.target/riscv/rvv/base/rvv-vector-bits-5.c: New test.
	* gcc.target/riscv/rvv/base/rvv-vector-bits-6.c: New test.

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/config/riscv/riscv-opts.h                 |  8 +++++
 gcc/config/riscv/riscv.cc                     | 35 +++++++++++++++----
 gcc/config/riscv/riscv.opt                    | 14 ++++++++
 .../riscv/rvv/base/rvv-vector-bits-1.c        |  7 ++++
 .../riscv/rvv/base/rvv-vector-bits-2.c        |  7 ++++
 .../riscv/rvv/base/rvv-vector-bits-3.c        |  9 +++++
 .../riscv/rvv/base/rvv-vector-bits-4.c        |  9 +++++
 .../riscv/rvv/base/rvv-vector-bits-5.c        | 17 +++++++++
 .../riscv/rvv/base/rvv-vector-bits-6.c        | 17 +++++++++
 9 files changed, 116 insertions(+), 7 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
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-5.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-6.c

Comments

Kito Cheng Feb. 28, 2024, 12:56 p.m. UTC | #1 
Take one more look, I think this option should work and integrate with
--param=riscv-autovec-preference= since they have similar jobs but
slightly different.

We have 3 value for  --param=riscv-autovec-preference=: none, scalable
and fixed-vlmax

-mrvv-vector-bits=scalable is work like
--param=riscv-autovec-preference=scalable and
-mrvv-vector-bits=zvl is work like
--param=riscv-autovec-preference=fixed-vlmax.

So I think...we need to do some conflict check, like:

-mrvv-vector-bits=zvl can't work with --param=riscv-autovec-preference=scalable
-mrvv-vector-bits=scalable can't work with
--param=riscv-autovec-preference=fixed-vlmax

but it may not just alias since there is some useful combinations like:

-mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
NO auto vectorization but intrinsic code still could benefit from the
-mrvv-vector-bits=zvl option.

-mrvv-vector-bits=scalable with --param=riscv-autovec-preference=none
Should still work for VLS code gen, but just disable auto
vectorization per the option semantic.

However here is something we need some fix, since
--param=riscv-autovec-preference=none still disable VLS code gen for
now, you can see some example here:
https://godbolt.org/z/fMTr3eW7K

But I think it's really the right behavior here, this part might need
to be fixed in vls_mode_valid_p and some other places.


Anyway I think we need to check all use sites with RVV_FIXED_VLMAX and
RVV_SCALABLE, and need to make sure all use site of RVV_FIXED_VLMAX
also checked with RVV_VECTOR_BITS_ZVL.



> -/* Return the VLEN value associated with -march.
> +static int
> +riscv_convert_vector_bits (int min_vlen)

Not sure if we really need this function, it seems it always returns min_vlen?

> +{
> +  int rvv_bits = 0;
> +
> +  switch (rvv_vector_bits)
> +    {
> +      case RVV_VECTOR_BITS_ZVL:
> +      case RVV_VECTOR_BITS_SCALABLE:
> +       rvv_bits = min_vlen;
> +       break;
> +      default:
> +       gcc_unreachable ();
> +    }
> +
> +  return rvv_bits;
> +}
> +
> +/* Return the VLEN value associated with -march and -mwrvv-vector-bits.
Li, Pan2 Feb. 28, 2024, 1:52 p.m. UTC | #2 
Oops, this is more complicated than original expectation.

Consider somehow the mrvv-vector-bits (zvl or scalable) decides how we perform the auto-vec.
I may have one proposal to combine them together?

For example, mrvv-vector-bits=zvl indicates we will auto-vect in fixed-vlmax way, and
mrvv-vector-bits=scalable indicates we will perform scalable auto-vec. That may make
things clean and get ride of the conflict code in many places (maybe).

Please help to correct me if any misunderstandings. Meanwhile, this change is sort of now or never up to point IMO.
We can only do it before GCC-14 release or never I guess (to avoid breaking changes).

> However here is something we need some fix, since
> --param=riscv-autovec-preference=none still disable VLS code gen for
> now, you can see some example here:
> https://godbolt.org/z/fMTr3eW7K

I am not quite sure about the right behavior here, when mrvv-vector-bits is given while riscv-autovec-preference is none...

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Wednesday, February 28, 2024 8:57 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; rdapp.gcc@gmail.com; jeffreyalaw@gmail.com
Subject: Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV

Take one more look, I think this option should work and integrate with
--param=riscv-autovec-preference= since they have similar jobs but
slightly different.

We have 3 value for  --param=riscv-autovec-preference=: none, scalable
and fixed-vlmax

-mrvv-vector-bits=scalable is work like
--param=riscv-autovec-preference=scalable and
-mrvv-vector-bits=zvl is work like
--param=riscv-autovec-preference=fixed-vlmax.

So I think...we need to do some conflict check, like:

-mrvv-vector-bits=zvl can't work with --param=riscv-autovec-preference=scalable
-mrvv-vector-bits=scalable can't work with
--param=riscv-autovec-preference=fixed-vlmax

but it may not just alias since there is some useful combinations like:

-mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
NO auto vectorization but intrinsic code still could benefit from the
-mrvv-vector-bits=zvl option.

-mrvv-vector-bits=scalable with --param=riscv-autovec-preference=none
Should still work for VLS code gen, but just disable auto
vectorization per the option semantic.

However here is something we need some fix, since
--param=riscv-autovec-preference=none still disable VLS code gen for
now, you can see some example here:
https://godbolt.org/z/fMTr3eW7K

But I think it's really the right behavior here, this part might need
to be fixed in vls_mode_valid_p and some other places.


Anyway I think we need to check all use sites with RVV_FIXED_VLMAX and
RVV_SCALABLE, and need to make sure all use site of RVV_FIXED_VLMAX
also checked with RVV_VECTOR_BITS_ZVL.



> -/* Return the VLEN value associated with -march.
> +static int
> +riscv_convert_vector_bits (int min_vlen)

Not sure if we really need this function, it seems it always returns min_vlen?

> +{
> +  int rvv_bits = 0;
> +
> +  switch (rvv_vector_bits)
> +    {
> +      case RVV_VECTOR_BITS_ZVL:
> +      case RVV_VECTOR_BITS_SCALABLE:
> +       rvv_bits = min_vlen;
> +       break;
> +      default:
> +       gcc_unreachable ();
> +    }
> +
> +  return rvv_bits;
> +}
> +
> +/* Return the VLEN value associated with -march and -mwrvv-vector-bits.
钟居哲 Feb. 28, 2024, 1:58 p.m. UTC | #3 
I think it makes more sense to remove --param=riscv-autovec-preference and add -mrvv-vector-bits....



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2024-02-28 20:56
To: pan2.li
CC: gcc-patches; juzhe.zhong; yanzhang.wang; rdapp.gcc; jeffreyalaw
Subject: Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
Take one more look, I think this option should work and integrate with
--param=riscv-autovec-preference= since they have similar jobs but
slightly different.
 
We have 3 value for  --param=riscv-autovec-preference=: none, scalable
and fixed-vlmax
 
-mrvv-vector-bits=scalable is work like
--param=riscv-autovec-preference=scalable and
-mrvv-vector-bits=zvl is work like
--param=riscv-autovec-preference=fixed-vlmax.
 
So I think...we need to do some conflict check, like:
 
-mrvv-vector-bits=zvl can't work with --param=riscv-autovec-preference=scalable
-mrvv-vector-bits=scalable can't work with
--param=riscv-autovec-preference=fixed-vlmax
 
but it may not just alias since there is some useful combinations like:
 
-mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
NO auto vectorization but intrinsic code still could benefit from the
-mrvv-vector-bits=zvl option.
 
-mrvv-vector-bits=scalable with --param=riscv-autovec-preference=none
Should still work for VLS code gen, but just disable auto
vectorization per the option semantic.
 
However here is something we need some fix, since
--param=riscv-autovec-preference=none still disable VLS code gen for
now, you can see some example here:
https://godbolt.org/z/fMTr3eW7K
 
But I think it's really the right behavior here, this part might need
to be fixed in vls_mode_valid_p and some other places.
 
 
Anyway I think we need to check all use sites with RVV_FIXED_VLMAX and
RVV_SCALABLE, and need to make sure all use site of RVV_FIXED_VLMAX
also checked with RVV_VECTOR_BITS_ZVL.
 
 
 
> -/* Return the VLEN value associated with -march.
> +static int
> +riscv_convert_vector_bits (int min_vlen)
 
Not sure if we really need this function, it seems it always returns min_vlen?
 
> +{
> +  int rvv_bits = 0;
> +
> +  switch (rvv_vector_bits)
> +    {
> +      case RVV_VECTOR_BITS_ZVL:
> +      case RVV_VECTOR_BITS_SCALABLE:
> +       rvv_bits = min_vlen;
> +       break;
> +      default:
> +       gcc_unreachable ();
> +    }
> +
> +  return rvv_bits;
> +}
> +
> +/* Return the VLEN value associated with -march and -mwrvv-vector-bits.
Kito Cheng Feb. 28, 2024, 2:55 p.m. UTC | #4 
Hmm, maybe only keep --param=riscv-autovec-preference=none and remove other
two if we think that might still useful? But anyway I have no strong
opinion to keep that, I mean I am ok to remove whole
--param=riscv-autovec-preference.

钟居哲 <juzhe.zhong@rivai.ai> 於 2024年2月28日 週三 21:59 寫道：

> I think it makes more sense to remove --param=riscv-autovec-preference and
> add -mrvv-vector-bits....
>
> ------------------------------
> juzhe.zhong@rivai.ai
>
>
> *From:* Kito Cheng <kito.cheng@gmail.com>
> *Date:* 2024-02-28 20:56
> *To:* pan2.li <pan2.li@intel.com>
> *CC:* gcc-patches <gcc-patches@gcc.gnu.org>; juzhe.zhong
> <juzhe.zhong@rivai.ai>; yanzhang.wang <yanzhang.wang@intel.com>; rdapp.gcc
> <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>
> *Subject:* Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits
> for RVV
> Take one more look, I think this option should work and integrate with
> --param=riscv-autovec-preference= since they have similar jobs but
> slightly different.
>
> We have 3 value for  --param=riscv-autovec-preference=: none, scalable
> and fixed-vlmax
>
> -mrvv-vector-bits=scalable is work like
> --param=riscv-autovec-preference=scalable and
> -mrvv-vector-bits=zvl is work like
> --param=riscv-autovec-preference=fixed-vlmax.
>
> So I think...we need to do some conflict check, like:
>
> -mrvv-vector-bits=zvl can't work with
> --param=riscv-autovec-preference=scalable
> -mrvv-vector-bits=scalable can't work with
> --param=riscv-autovec-preference=fixed-vlmax
>
> but it may not just alias since there is some useful combinations like:
>
> -mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
> NO auto vectorization but intrinsic code still could benefit from the
> -mrvv-vector-bits=zvl option.
>
> -mrvv-vector-bits=scalable with --param=riscv-autovec-preference=none
> Should still work for VLS code gen, but just disable auto
> vectorization per the option semantic.
>
> However here is something we need some fix, since
> --param=riscv-autovec-preference=none still disable VLS code gen for
> now, you can see some example here:
> https://godbolt.org/z/fMTr3eW7K
>
> But I think it's really the right behavior here, this part might need
> to be fixed in vls_mode_valid_p and some other places.
>
>
> Anyway I think we need to check all use sites with RVV_FIXED_VLMAX and
> RVV_SCALABLE, and need to make sure all use site of RVV_FIXED_VLMAX
> also checked with RVV_VECTOR_BITS_ZVL.
>
>
>
> > -/* Return the VLEN value associated with -march.
> > +static int
> > +riscv_convert_vector_bits (int min_vlen)
>
> Not sure if we really need this function, it seems it always returns
> min_vlen?
>
> > +{
> > +  int rvv_bits = 0;
> > +
> > +  switch (rvv_vector_bits)
> > +    {
> > +      case RVV_VECTOR_BITS_ZVL:
> > +      case RVV_VECTOR_BITS_SCALABLE:
> > +       rvv_bits = min_vlen;
> > +       break;
> > +      default:
> > +       gcc_unreachable ();
> > +    }
> > +
> > +  return rvv_bits;
> > +}
> > +
> > +/* Return the VLEN value associated with -march and -mwrvv-vector-bits.
>
>
>
钟居哲 Feb. 29, 2024, 1:23 a.m. UTC | #5 
I think it makes more sense to remove the whole --param=riscv-autovec-preference since we should use
-fno-tree-vectorize instead of --param=riscv-autovec-preference=none which is more reasonable compile option
for users.

--param is just a internal testing option that we added before, ideally we should remove them.



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2024-02-28 22:55
To: 钟居哲
CC: pan2.li; gcc-patches; yanzhang.wang; rdapp.gcc; Jeff Law
Subject: Re: Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
Hmm, maybe only keep --param=riscv-autovec-preference=none and remove other two if we think that might still useful? But anyway I have no strong opinion to keep that, I mean I am ok to remove whole --param=riscv-autovec-preference.

钟居哲 <juzhe.zhong@rivai.ai> 於 2024年2月28日 週三 21:59 寫道：
I think it makes more sense to remove --param=riscv-autovec-preference and add -mrvv-vector-bits....



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2024-02-28 20:56
To: pan2.li
CC: gcc-patches; juzhe.zhong; yanzhang.wang; rdapp.gcc; jeffreyalaw
Subject: Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV
Take one more look, I think this option should work and integrate with
--param=riscv-autovec-preference= since they have similar jobs but
slightly different.
 
We have 3 value for  --param=riscv-autovec-preference=: none, scalable
and fixed-vlmax
 
-mrvv-vector-bits=scalable is work like
--param=riscv-autovec-preference=scalable and
-mrvv-vector-bits=zvl is work like
--param=riscv-autovec-preference=fixed-vlmax.
 
So I think...we need to do some conflict check, like:
 
-mrvv-vector-bits=zvl can't work with --param=riscv-autovec-preference=scalable
-mrvv-vector-bits=scalable can't work with
--param=riscv-autovec-preference=fixed-vlmax
 
but it may not just alias since there is some useful combinations like:
 
-mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
NO auto vectorization but intrinsic code still could benefit from the
-mrvv-vector-bits=zvl option.
 
-mrvv-vector-bits=scalable with --param=riscv-autovec-preference=none
Should still work for VLS code gen, but just disable auto
vectorization per the option semantic.
 
However here is something we need some fix, since
--param=riscv-autovec-preference=none still disable VLS code gen for
now, you can see some example here:
https://godbolt.org/z/fMTr3eW7K
 
But I think it's really the right behavior here, this part might need
to be fixed in vls_mode_valid_p and some other places.
 
 
Anyway I think we need to check all use sites with RVV_FIXED_VLMAX and
RVV_SCALABLE, and need to make sure all use site of RVV_FIXED_VLMAX
also checked with RVV_VECTOR_BITS_ZVL.
 
 
 
> -/* Return the VLEN value associated with -march.
> +static int
> +riscv_convert_vector_bits (int min_vlen)
 
Not sure if we really need this function, it seems it always returns min_vlen?
 
> +{
> +  int rvv_bits = 0;
> +
> +  switch (rvv_vector_bits)
> +    {
> +      case RVV_VECTOR_BITS_ZVL:
> +      case RVV_VECTOR_BITS_SCALABLE:
> +       rvv_bits = min_vlen;
> +       break;
> +      default:
> +       gcc_unreachable ();
> +    }
> +
> +  return rvv_bits;
> +}
> +
> +/* Return the VLEN value associated with -march and -mwrvv-vector-bits.
Li, Pan2 Feb. 29, 2024, 1:23 a.m. UTC | #6 
Personally I prefer to remove --param=riscv-autovec-preference=none and only allow
mrvv-vector-bits, to avoid tricky(maybe) sematic of none preference. However, let’s
wait for a while in case there are some comments from others.

Pan

From: Kito Cheng <kito.cheng@gmail.com>
Sent: Wednesday, February 28, 2024 10:55 PM
To: 钟居哲 <juzhe.zhong@rivai.ai>
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>; rdapp.gcc <rdapp.gcc@gmail.com>; Jeff Law <jeffreyalaw@gmail.com>
Subject: Re: Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV

Hmm, maybe only keep --param=riscv-autovec-preference=none and remove other two if we think that might still useful? But anyway I have no strong opinion to keep that, I mean I am ok to remove whole --param=riscv-autovec-preference.

钟居哲 <juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>> 於 2024年2月28日 週三 21:59 寫道：
I think it makes more sense to remove --param=riscv-autovec-preference and add -mrvv-vector-bits....
Robin Dapp Feb. 29, 2024, 1:21 p.m. UTC | #7 
> I think it makes more sense to remove the whole
> --param=riscv-autovec-preference since we should use 
> -fno-tree-vectorize instead of --param=riscv-autovec-preference=none
> which is more reasonable compile option for users.
> 
> --param is just a internal testing option that we added before,
> ideally we should remove them.
Yes, I agree with that.  At least the "none" part doesn't seem
necessary.

Regards
 Robin
Vineet Gupta March 1, 2024, 6:59 p.m. UTC | #8 
Hi Pan,

On 2/28/24 17:23, Li, Pan2 wrote:
>
> Personally I prefer to remove --param=riscv-autovec-preference=none
> and only allow
>
> mrvv-vector-bits, to avoid tricky(maybe) sematic of none preference.
> However, let’s
>
> wait for a while in case there are some comments from others.
>

We are very interested in this topic. Could you please CC me and Palmer
for future versions of the patchset.

Thx,
-Vineet

>  
>
> Pan
>
>  
>
> *From:*Kito Cheng <kito.cheng@gmail.com>
> *Sent:* Wednesday, February 28, 2024 10:55 PM
> *To:* 钟居哲<juzhe.zhong@rivai.ai>
> *Cc:* Li, Pan2 <pan2.li@intel.com>; gcc-patches
> <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>;
> rdapp.gcc <rdapp.gcc@gmail.com>; Jeff Law <jeffreyalaw@gmail.com>
> *Subject:* Re: Re: [PATCH v3] RISC-V: Introduce gcc option
> mrvv-vector-bits for RVV
>
>  
>
> Hmm, maybe only keep --param=riscv-autovec-preference=none and remove
> other two if we think that might still useful? But anyway I have no
> strong opinion to keep that, I mean I am ok to remove whole
> --param=riscv-autovec-preference.
>
>  
>
> 钟居哲 <juzhe.zhong@rivai.ai> 於 2024年2月28日週三 21:59 寫道：
>
>     I think it makes more sense to remove
>     --param=riscv-autovec-preference and add -mrvv-vector-bits....
>
>      
>
>     ------------------------------------------------------------------------
>
>     juzhe.zhong@rivai.ai
>
>          
>
>         *From:* Kito Cheng <mailto:kito.cheng@gmail.com>
>
>         *Date:* 2024-02-28 20:56
>
>         *To:* 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>; rdapp.gcc
>         <mailto:rdapp.gcc@gmail.com>; jeffreyalaw
>         <mailto:jeffreyalaw@gmail.com>
>
>         *Subject:* Re: [PATCH v3] RISC-V: Introduce gcc option
>         mrvv-vector-bits for RVV
>
>         Take one more look, I think this option should work and
>         integrate with
>
>         --param=riscv-autovec-preference= since they have similar jobs but
>
>         slightly different.
>
>          
>
>         We have 3 value for  --param=riscv-autovec-preference=: none,
>         scalable
>
>         and fixed-vlmax
>
>          
>
>         -mrvv-vector-bits=scalable is work like
>
>         --param=riscv-autovec-preference=scalable and
>
>         -mrvv-vector-bits=zvl is work like
>
>         --param=riscv-autovec-preference=fixed-vlmax.
>
>          
>
>         So I think...we need to do some conflict check, like:
>
>          
>
>         -mrvv-vector-bits=zvl can't work with
>         --param=riscv-autovec-preference=scalable
>
>         -mrvv-vector-bits=scalable can't work with
>
>         --param=riscv-autovec-preference=fixed-vlmax
>
>          
>
>         but it may not just alias since there is some useful
>         combinations like:
>
>          
>
>         -mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
>
>         NO auto vectorization but intrinsic code still could benefit
>         from the
>
>         -mrvv-vector-bits=zvl option.
>
>          
>
>         -mrvv-vector-bits=scalable with
>         --param=riscv-autovec-preference=none
>
>         Should still work for VLS code gen, but just disable auto
>
>         vectorization per the option semantic.
>
>          
>
>         However here is something we need some fix, since
>
>         --param=riscv-autovec-preference=none still disable VLS code
>         gen for
>
>         now, you can see some example here:
>
>         https://godbolt.org/z/fMTr3eW7K
>
>          
>
>         But I think it's really the right behavior here, this part
>         might need
>
>         to be fixed in vls_mode_valid_p and some other places.
>
>          
>
>          
>
>         Anyway I think we need to check all use sites with
>         RVV_FIXED_VLMAX and
>
>         RVV_SCALABLE, and need to make sure all use site of
>         RVV_FIXED_VLMAX
>
>         also checked with RVV_VECTOR_BITS_ZVL.
>
>          
>
>          
>
>          
>
>         > -/* Return the VLEN value associated with -march.
>
>         > +static int
>
>         > +riscv_convert_vector_bits (int min_vlen)
>
>          
>
>         Not sure if we really need this function, it seems it always
>         returns min_vlen?
>
>          
>
>         > +{
>
>         > +  int rvv_bits = 0;
>
>         > +
>
>         > +  switch (rvv_vector_bits)
>
>         > +    {
>
>         > +      case RVV_VECTOR_BITS_ZVL:
>
>         > +      case RVV_VECTOR_BITS_SCALABLE:
>
>         > +       rvv_bits = min_vlen;
>
>         > +       break;
>
>         > +      default:
>
>         > +       gcc_unreachable ();
>
>         > +    }
>
>         > +
>
>         > +  return rvv_bits;
>
>         > +}
>
>         > +
>
>         > +/* Return the VLEN value associated with -march and
>         -mwrvv-vector-bits.
>
>          
>
Li, Pan2 March 2, 2024, 12:42 a.m. UTC | #9 
Sure thing.

Pan

-----Original Message-----
From: Vineet Gupta <vineetg@rivosinc.com> 
Sent: Saturday, March 2, 2024 3:00 AM
To: Li, Pan2 <pan2.li@intel.com>; Kito Cheng <kito.cheng@gmail.com>; 钟居哲 <juzhe.zhong@rivai.ai>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>; rdapp.gcc <rdapp.gcc@gmail.com>; Jeff Law <jeffreyalaw@gmail.com>
Subject: Re: [PATCH v3] RISC-V: Introduce gcc option mrvv-vector-bits for RVV

Hi Pan,

On 2/28/24 17:23, Li, Pan2 wrote:
>
> Personally I prefer to remove --param=riscv-autovec-preference=none
> and only allow
>
> mrvv-vector-bits, to avoid tricky(maybe) sematic of none preference.
> However, let’s
>
> wait for a while in case there are some comments from others.
>

We are very interested in this topic. Could you please CC me and Palmer
for future versions of the patchset.

Thx,
-Vineet

>  
>
> Pan
>
>  
>
> *From:*Kito Cheng <kito.cheng@gmail.com>
> *Sent:* Wednesday, February 28, 2024 10:55 PM
> *To:* 钟居哲<juzhe.zhong@rivai.ai>
> *Cc:* Li, Pan2 <pan2.li@intel.com>; gcc-patches
> <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>;
> rdapp.gcc <rdapp.gcc@gmail.com>; Jeff Law <jeffreyalaw@gmail.com>
> *Subject:* Re: Re: [PATCH v3] RISC-V: Introduce gcc option
> mrvv-vector-bits for RVV
>
>  
>
> Hmm, maybe only keep --param=riscv-autovec-preference=none and remove
> other two if we think that might still useful? But anyway I have no
> strong opinion to keep that, I mean I am ok to remove whole
> --param=riscv-autovec-preference.
>
>  
>
> 钟居哲 <juzhe.zhong@rivai.ai> 於 2024年2月28日週三 21:59 寫道：
>
>     I think it makes more sense to remove
>     --param=riscv-autovec-preference and add -mrvv-vector-bits....
>
>      
>
>     ------------------------------------------------------------------------
>
>     juzhe.zhong@rivai.ai
>
>          
>
>         *From:* Kito Cheng <mailto:kito.cheng@gmail.com>
>
>         *Date:* 2024-02-28 20:56
>
>         *To:* 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>; rdapp.gcc
>         <mailto:rdapp.gcc@gmail.com>; jeffreyalaw
>         <mailto:jeffreyalaw@gmail.com>
>
>         *Subject:* Re: [PATCH v3] RISC-V: Introduce gcc option
>         mrvv-vector-bits for RVV
>
>         Take one more look, I think this option should work and
>         integrate with
>
>         --param=riscv-autovec-preference= since they have similar jobs but
>
>         slightly different.
>
>          
>
>         We have 3 value for  --param=riscv-autovec-preference=: none,
>         scalable
>
>         and fixed-vlmax
>
>          
>
>         -mrvv-vector-bits=scalable is work like
>
>         --param=riscv-autovec-preference=scalable and
>
>         -mrvv-vector-bits=zvl is work like
>
>         --param=riscv-autovec-preference=fixed-vlmax.
>
>          
>
>         So I think...we need to do some conflict check, like:
>
>          
>
>         -mrvv-vector-bits=zvl can't work with
>         --param=riscv-autovec-preference=scalable
>
>         -mrvv-vector-bits=scalable can't work with
>
>         --param=riscv-autovec-preference=fixed-vlmax
>
>          
>
>         but it may not just alias since there is some useful
>         combinations like:
>
>          
>
>         -mrvv-vector-bits=zvl with --param=riscv-autovec-preference=none:
>
>         NO auto vectorization but intrinsic code still could benefit
>         from the
>
>         -mrvv-vector-bits=zvl option.
>
>          
>
>         -mrvv-vector-bits=scalable with
>         --param=riscv-autovec-preference=none
>
>         Should still work for VLS code gen, but just disable auto
>
>         vectorization per the option semantic.
>
>          
>
>         However here is something we need some fix, since
>
>         --param=riscv-autovec-preference=none still disable VLS code
>         gen for
>
>         now, you can see some example here:
>
>         https://godbolt.org/z/fMTr3eW7K
>
>          
>
>         But I think it's really the right behavior here, this part
>         might need
>
>         to be fixed in vls_mode_valid_p and some other places.
>
>          
>
>          
>
>         Anyway I think we need to check all use sites with
>         RVV_FIXED_VLMAX and
>
>         RVV_SCALABLE, and need to make sure all use site of
>         RVV_FIXED_VLMAX
>
>         also checked with RVV_VECTOR_BITS_ZVL.
>
>          
>
>          
>
>          
>
>         > -/* Return the VLEN value associated with -march.
>
>         > +static int
>
>         > +riscv_convert_vector_bits (int min_vlen)
>
>          
>
>         Not sure if we really need this function, it seems it always
>         returns min_vlen?
>
>          
>
>         > +{
>
>         > +  int rvv_bits = 0;
>
>         > +
>
>         > +  switch (rvv_vector_bits)
>
>         > +    {
>
>         > +      case RVV_VECTOR_BITS_ZVL:
>
>         > +      case RVV_VECTOR_BITS_SCALABLE:
>
>         > +       rvv_bits = min_vlen;
>
>         > +       break;
>
>         > +      default:
>
>         > +       gcc_unreachable ();
>
>         > +    }
>
>         > +
>
>         > +  return rvv_bits;
>
>         > +}
>
>         > +
>
>         > +/* Return the VLEN value associated with -march and
>         -mwrvv-vector-bits.
>
>          
>
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index 4edddbadc37..2a311c9d2a3 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -129,6 +129,14 @@  enum vsetvl_strategy_enum {
   VSETVL_OPT_NO_FUSION,
 };
 
+/* RVV vector bits for option -mrvv-vector-bits, default is scalable.  */
+enum rvv_vector_bits_enum {
+  /* scalable indicates taking the value of zvl*b as the minimal vlen.  */
+  RVV_VECTOR_BITS_SCALABLE,
+  /* zvl indicates taking the value of zvl*b as the exactly vlen.  */
+  RVV_VECTOR_BITS_ZVL,
+};
+
 #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..b6b133210ff 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8801,13 +8801,33 @@  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_ZVL:
+      case RVV_VECTOR_BITS_SCALABLE:
+	rvv_bits = min_vlen;
+	break;
+      default:
+	gcc_unreachable ();
+    }
+
+  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 +8846,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
     {
@@ -8847,8 +8867,9 @@  riscv_convert_vector_bits (struct gcc_options *opts)
      compile-time constant if TARGET_VECTOR is disabled.  */
   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);
+      if (opts->x_riscv_autovec_preference == RVV_FIXED_VLMAX
+	|| opts->x_rvv_vector_bits == RVV_VECTOR_BITS_ZVL)
+	return (int) rvv_bits / (riscv_bytes_per_vector_chunk * 8);
       else
 	return poly_uint16 (chunk_num, chunk_num);
     }
@@ -8920,8 +8941,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..bf60dcc8b53 100644
--- a/gcc/config/riscv/riscv.opt
+++ b/gcc/config/riscv/riscv.opt
@@ -607,3 +607,17 @@  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(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=zvl	Set the number of bits in zvl for 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..20708460201
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-1.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64 -mrvv-vector-bits=128 -O3" } */
+
+#include "riscv_vector.h"
+
+/* { dg-error "unrecognized argument in option '-mrvv-vector-bits=128'" "" { target { "riscv*-*-*" } } 0 } */
+/* { dg-message "note: valid arguments to '-mrvv-vector-bits=' are: scalable zvl" "" { 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..54c86ffcc56
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-2.c
@@ -0,0 +1,7 @@ 
+/* { 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 } */
+/* { dg-message "note: valid arguments to '-mrvv-vector-bits=' are: scalable zvl" "" { target { "riscv*-*-*" } } 0 } */
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..9c9acebd5e3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-3.c
@@ -0,0 +1,9 @@ 
+/* Test that we do not have error when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=zvl -O3" } */
+
+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..9589bf81296
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-4.c
@@ -0,0 +1,9 @@ 
+/* Test that we do not have error when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=scalable -O3" } */
+
+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-5.c b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-5.c
new file mode 100644
index 00000000000..1f03bbce04f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-5.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=zvl -O3" } */
+
+#include "riscv_vector.h"
+
+void test_rvv_vector_bits_zvl ()
+{
+  vint32m1_t x;
+  asm volatile ("def %0": "=vr"(x));
+  asm volatile (""::: "v0",   "v1",  "v2",  "v3",  "v4",  "v5",  "v6",  "v7",
+		      "v8",   "v9", "v10", "v11", "v12", "v13", "v14", "v15",
+		      "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
+		      "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");
+  asm volatile ("use %0": : "vr"(x));
+}
+
+/* { dg-final { scan-assembler-not {csrr\s+[atx][0-9]+,\s*vlenb} } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-6.c b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-6.c
new file mode 100644
index 00000000000..ea762090457
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/rvv-vector-bits-6.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl128b -mabi=lp64 -mrvv-vector-bits=scalable -O3" } */
+
+#include "riscv_vector.h"
+
+void test_rvv_vector_bits_scalable ()
+{
+  vint32m1_t x;
+  asm volatile ("def %0": "=vr"(x));
+  asm volatile (""::: "v0",   "v1",  "v2",  "v3",  "v4",  "v5",  "v6",  "v7",
+		      "v8",   "v9", "v10", "v11", "v12", "v13", "v14", "v15",
+		      "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
+		      "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");
+  asm volatile ("use %0": : "vr"(x));
+}
+
+/* { dg-final { scan-assembler-times {csrr\s+[atx][0-9]+,\s*vlenb} 2 } } */