public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] riscv: Clarify vlmax and length handling.
@ 2023-05-10 15:24 Robin Dapp
  2023-05-10 18:32 ` Palmer Dabbelt
  2023-05-11  2:30 ` [PATCH] riscv: " juzhe.zhong
  0 siblings, 2 replies; 9+ messages in thread
From: Robin Dapp @ 2023-05-10 15:24 UTC (permalink / raw)
  To: gcc-patches, juzhe.zhong, Kito Cheng, Michael Collison, palmer,
	jeffreyalaw
  Cc: rdapp.gcc

Hi,

this patch tries to improve the wrappers that emit either vlmax or
non-vlmax operations.  Now, emit_len_op can be used to
emit a regular operation.  Depending on whether a length != NULL
is passed either no VLMAX flags are set or we emit a vsetvli and
set VLMAX flags.  The patch also adds some comments that describes
some of the rationale of the current handling of vlmax/nonvlmax
operations.

Bootstrapped and regtested.

Regards
 Robin

--

gcc/ChangeLog:

	* config/riscv/autovec.md: Use renamed functions.
	* config/riscv/riscv-protos.h (emit_vlmax_op): Rename.
	(emit_vlmax_reg_op): To this.
	(emit_nonvlmax_op): Rename.
	(emit_len_op): To this.
	(emit_nonvlmax_binop): Rename.
	(emit_len_binop): To this.
	* config/riscv/riscv-v.cc (emit_pred_op): Add default parameter.
	(emit_pred_binop): Remove vlmax_p.
	(emit_vlmax_op): Rename.
	(emit_vlmax_reg_op): To this.
	(emit_nonvlmax_op): Rename.
	(emit_len_op): To this.
	(emit_nonvlmax_binop): Rename.
	(emit_len_binop): To this.
	(sew64_scalar_helper): Use renamed functions.
	(expand_tuple_move): Use renamed functions.
	* config/riscv/riscv.cc (vector_zero_call_used_regs): Use
	renamed functions.
	* config/riscv/vector.md: Use renamed functions.
---
 gcc/config/riscv/autovec.md     | 24 +++++-----
 gcc/config/riscv/riscv-protos.h |  8 ++--
 gcc/config/riscv/riscv-v.cc     | 82 ++++++++++++++++++++-------------
 gcc/config/riscv/riscv.cc       |  4 +-
 gcc/config/riscv/vector.md      | 12 +++--
 5 files changed, 75 insertions(+), 55 deletions(-)

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index 15f8d007e07..8347e42bb9c 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -31,8 +31,8 @@ (define_expand "len_load_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
 {
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-				  operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+			     operands[1], operands[2], <VM>mode);
   DONE;
 })
 
@@ -43,8 +43,8 @@ (define_expand "len_store_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
 {
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-				  operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+			     operands[1], operands[2], <VM>mode);
   DONE;
 })
 
@@ -79,15 +79,15 @@ (define_expand "<optab><mode>3"
       if (inner == E_QImode || inner == E_HImode || inner == E_SImode)
 	op2mode = inner;
 
-      riscv_vector::emit_nonvlmax_binop (code_for_pred_scalar
-					 (<BINOP_TO_UPPERCASE>, <MODE>mode),
-					 operands[0], operands[1], cst,
-					 NULL_RTX, <VM>mode, op2mode);
+      riscv_vector::emit_len_binop (code_for_pred_scalar
+				    (<BINOP_TO_UPPERCASE>, <MODE>mode),
+				    operands[0], operands[1], cst,
+				    NULL_RTX, <VM>mode, op2mode);
     }
   else
-    riscv_vector::emit_nonvlmax_binop (code_for_pred
-				       (<BINOP_TO_UPPERCASE>, <MODE>mode),
-				       operands[0], operands[1], operands[2],
-				       NULL_RTX, <VM>mode);
+    riscv_vector::emit_len_binop (code_for_pred
+				  (<BINOP_TO_UPPERCASE>, <MODE>mode),
+				  operands[0], operands[1], operands[2],
+				  NULL_RTX, <VM>mode);
   DONE;
 })
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 75cdb90b9c9..bfdf09b17ee 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -167,10 +167,10 @@ bool legitimize_move (rtx, rtx, machine_mode);
 void emit_vlmax_vsetvl (machine_mode, rtx);
 void emit_hard_vlmax_vsetvl (machine_mode, rtx);
 void emit_vlmax_op (unsigned, rtx, rtx, machine_mode);
-void emit_vlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
-			  machine_mode = VOIDmode);
+void emit_vlmax_reg_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode, machine_mode =
+		     VOIDmode);
 enum vlmul_type get_vlmul (machine_mode);
 unsigned int get_ratio (machine_mode);
 unsigned int get_nf (machine_mode);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 3c43dfc5eea..07b7783282f 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -99,27 +99,24 @@ public:
     add_vundef_operand (dest_mode);
   }
 
-  void set_len_and_policy (rtx len, bool vlmax_p)
+  void set_len_and_policy (rtx len, bool force_vlmax = false)
     {
+      bool vlmax_p = force_vlmax;
       gcc_assert (has_dest);
-      gcc_assert (len || vlmax_p);
 
-      if (len)
-	add_input_operand (len, Pmode);
-      else
+      if (!len)
 	{
-	  rtx vlmax = gen_reg_rtx (Pmode);
-	  emit_vlmax_vsetvl (dest_mode, vlmax);
-	  add_input_operand (vlmax, Pmode);
+	  vlmax_p = true;
+	  len = gen_reg_rtx (Pmode);
+	  emit_vlmax_vsetvl (dest_mode, len);
 	}
 
+      add_input_operand (len, Pmode);
+
       if (GET_MODE_CLASS (dest_mode) != MODE_VECTOR_BOOL)
 	add_policy_operand (get_prefer_tail_policy (), get_prefer_mask_policy ());
 
-      if (vlmax_p)
-	add_avl_type_operand (avl_type::VLMAX);
-      else
-	add_avl_type_operand (avl_type::NONVLMAX);
+      add_avl_type_operand (vlmax_p ? avl_type::VLMAX : avl_type::NONVLMAX);
     }
 
   void expand (enum insn_code icode, bool temporary_volatile_p = false)
@@ -251,14 +248,14 @@ set_expander_dest_and_mask (insn_expander<MAX_OPERANDS> &e, rtx mask, rtx dest,
 /* Emit an RVV unmask && vl mov from SRC to DEST.  */
 static void
 emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
-	      machine_mode mask_mode, bool vlmax_p)
+	      machine_mode mask_mode, bool force_vlmax = false)
 {
   insn_expander<8> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
 
   e.add_input_operand (src, GET_MODE (src));
 
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len, force_vlmax);
 
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src));
 }
@@ -266,8 +263,7 @@ emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
 /* Emit an RVV unmask && vl mov from SRC to DEST.  */
 static void
 emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
-		 rtx len, machine_mode mask_mode, machine_mode op2mode,
-		 bool vlmax_p)
+		 rtx len, machine_mode mask_mode, machine_mode op2mode)
 {
   insn_expander<9> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
@@ -275,37 +271,59 @@ emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
   e.add_input_operand (src1, GET_MODE (src1));
   e.add_input_operand (src2, op2mode == VOIDmode ? GET_MODE (src2) : op2mode);
 
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len);
 
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src1) || MEM_P (src2));
 }
 
+/* The RISC-V vsetvli pass uses vlmax operation for optimization.  Whether or
+   not an instruction actually is a vlmax operation is not recognizable from
+   the length operand alone but the avl_type operand is used instead.
+   In general, there are two cases:
+
+    - Emit a vlmax operation by passing a NULL length.  Here we emit
+      a vsetvli with vlmax configuration and set the avl_type to VLMAX.
+    - Emit an operation that uses the existing (last-set) length and
+      set the avl_type to NONVLMAX.
+
+    Sometimes we also need to set the VLMAX avl_type to an operation that
+    already uses a given length register.  This can happen during or after
+    register allocation when we are not allowed to create a new register.
+    For that case we also allow to set the avl_type to VLMAX.
+*/
+
+/* This function emits a VLMAX vsetvli followed by the actual operation.  */
 void
 emit_vlmax_op (unsigned icode, rtx dest, rtx src, machine_mode mask_mode)
 {
-  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode);
 }
 
+/* This function emits an operation with a given LEN that is determined
+   by a previously emitted VLMAX vsetvli.  */
 void
-emit_vlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-	       machine_mode mask_mode)
+emit_len_op (unsigned icode, rtx dest, rtx src, rtx len,
+	     machine_mode mask_mode)
 {
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode);
 }
 
+/* This function emits an operation with a given LEN that is known to be
+   a preceding VLMAX.  It also sets the VLMAX flag which allows further
+   optimization in the vsetvli pass.  */
 void
-emit_nonvlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-		  machine_mode mask_mode)
+emit_vlmax_reg_op (unsigned icode, rtx dest, rtx src, rtx len,
+		   machine_mode mask_mode)
 {
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, false);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode,
+		/* Force VLMAX */ true);
 }
 
 void
-emit_nonvlmax_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
-		     machine_mode mask_mode, machine_mode op2mode)
+emit_len_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
+		machine_mode mask_mode, machine_mode op2mode)
 {
-  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode, op2mode,
-		   false);
+  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode, op2mode);
 }
 
 static void
@@ -667,8 +685,8 @@ sew64_scalar_helper (rtx *operands, rtx *scalar_op, rtx vl,
     *scalar_op = force_reg (scalar_mode, *scalar_op);
 
   rtx tmp = gen_reg_rtx (vector_mode);
-  riscv_vector::emit_nonvlmax_op (code_for_pred_broadcast (vector_mode), tmp,
-				  *scalar_op, vl, mask_mode);
+  riscv_vector::emit_len_op (code_for_pred_broadcast (vector_mode), tmp,
+			     *scalar_op, vl, mask_mode);
   emit_vector_func (operands, tmp);
 
   return true;
@@ -974,7 +992,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
 	      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
 
 	      if (fractional_p)
-		emit_vlmax_op (code_for_pred_mov (subpart_mode), subreg, mem,
+		emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), subreg, mem,
 			       ops[4], mask_mode);
 	      else
 		emit_move_insn (subreg, mem);
@@ -996,7 +1014,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
 	      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
 
 	      if (fractional_p)
-		emit_vlmax_op (code_for_pred_mov (subpart_mode), mem, subreg,
+		emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), mem, subreg,
 			       ops[4], mask_mode);
 	      else
 		emit_move_insn (mem, subreg);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 9e08a1bfe35..9a493295a38 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7373,8 +7373,8 @@ vector_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
 	      emitted_vlmax_vsetvl = true;
 	    }
 
-	  riscv_vector::emit_vlmax_op (code_for_pred_mov (mode), target,
-				       CONST0_RTX (mode), vl, mask_mode);
+	  riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (mode), target,
+					   CONST0_RTX (mode), vl, mask_mode);
 
 	  SET_HARD_REG_BIT (zeroed_hardregs, regno);
 	}
diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index 9ccc0d6a513..75479f27bcd 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -721,8 +721,9 @@ (define_insn_and_split "*mov<V_FRACT:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<V_FRACT:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<V_FRACT:MODE>mode),
-      		operands[0], operands[1], operands[2], <VM>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<V_FRACT:MODE>mode),
+				       operands[0], operands[1], operands[2],
+				       <VM>mode);
     }
   DONE;
 })
@@ -741,8 +742,9 @@ (define_insn_and_split "*mov<VB:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<VB:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<VB:MODE>mode),
-      		operands[0], operands[1], operands[2], <VB:MODE>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<VB:MODE>mode),
+				       operands[0], operands[1], operands[2],
+				       <VB:MODE>mode);
     }
   DONE;
 })
@@ -806,7 +808,7 @@ (define_expand "vec_duplicate<mode>"
   "TARGET_VECTOR"
   {
     riscv_vector::emit_vlmax_op (code_for_pred_broadcast (<MODE>mode),
-		operands[0], operands[1], <VM>mode);
+				 operands[0], operands[1], <VM>mode);
     DONE;
   }
 )
-- 
2.40.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] riscv: Clarify vlmax and length handling.
  2023-05-10 15:24 [PATCH] riscv: Clarify vlmax and length handling Robin Dapp
@ 2023-05-10 18:32 ` Palmer Dabbelt
  2023-05-10 18:50   ` Robin Dapp
  2023-05-11  2:30 ` [PATCH] riscv: " juzhe.zhong
  1 sibling, 1 reply; 9+ messages in thread
From: Palmer Dabbelt @ 2023-05-10 18:32 UTC (permalink / raw)
  To: rdapp.gcc
  Cc: gcc-patches, juzhe.zhong, Kito Cheng, collison, jeffreyalaw, rdapp.gcc

On Wed, 10 May 2023 08:24:40 PDT (-0700), rdapp.gcc@gmail.com wrote:
> Hi,
>
> this patch tries to improve the wrappers that emit either vlmax or
> non-vlmax operations.  Now, emit_len_op can be used to
> emit a regular operation.  Depending on whether a length != NULL
> is passed either no VLMAX flags are set or we emit a vsetvli and
> set VLMAX flags.  The patch also adds some comments that describes
> some of the rationale of the current handling of vlmax/nonvlmax
> operations.
>
> Bootstrapped and regtested.
>
> Regards
>  Robin

It's somewhat common for mail clients to treat "--" as a signature 
deliminator, it's "---" that git uses as a comment deliminator.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] riscv: Clarify vlmax and length handling.
  2023-05-10 18:32 ` Palmer Dabbelt
@ 2023-05-10 18:50   ` Robin Dapp
  2023-05-10 18:53     ` Palmer Dabbelt
  0 siblings, 1 reply; 9+ messages in thread
From: Robin Dapp @ 2023-05-10 18:50 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: gcc-patches, juzhe.zhong, Kito Cheng, collison, jeffreyalaw

> It's somewhat common for mail clients to treat "--" as a signature 
> deliminator, it's "---" that git uses as a comment deliminator.

It's in my muscle memory somehow.  Always did it that way because I
didn't want the same delimiter as in the git part of the message.  Time 
to change that habit I suppose :) (or automate more of the process).

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] riscv: Clarify vlmax and length handling.
  2023-05-10 18:50   ` Robin Dapp
@ 2023-05-10 18:53     ` Palmer Dabbelt
  2023-05-11  2:26       ` Kito Cheng
  0 siblings, 1 reply; 9+ messages in thread
From: Palmer Dabbelt @ 2023-05-10 18:53 UTC (permalink / raw)
  To: rdapp.gcc; +Cc: gcc-patches, juzhe.zhong, Kito Cheng, collison, jeffreyalaw

On Wed, 10 May 2023 11:50:32 PDT (-0700), rdapp.gcc@gmail.com wrote:
>> It's somewhat common for mail clients to treat "--" as a signature
>> deliminator, it's "---" that git uses as a comment deliminator.
>
> It's in my muscle memory somehow.  Always did it that way because I
> didn't want the same delimiter as in the git part of the message.  Time
> to change that habit I suppose :) (or automate more of the process).

I guess if you're committing your own code it doesn't matter, but mixing 
them will trip up git-am and such.

The patch LGTM, but it's mostly Juzhe's code so it's probably best to at 
least give him a chance to see it when he's awake.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] riscv: Clarify vlmax and length handling.
  2023-05-10 18:53     ` Palmer Dabbelt
@ 2023-05-11  2:26       ` Kito Cheng
  2023-05-11 10:29         ` [PATCH v2] RISC-V: " Robin Dapp
  0 siblings, 1 reply; 9+ messages in thread
From: Kito Cheng @ 2023-05-11  2:26 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: rdapp.gcc, gcc-patches, juzhe.zhong, collison, jeffreyalaw

LGTM, and just one nit, use RISC-V in the title would be better since
Palmer's patchwork filter is set to "RISC-V", so using "riscv:" might
be missed during patchwork review meeting :P


On Thu, May 11, 2023 at 2:54 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 10 May 2023 11:50:32 PDT (-0700), rdapp.gcc@gmail.com wrote:
> >> It's somewhat common for mail clients to treat "--" as a signature
> >> deliminator, it's "---" that git uses as a comment deliminator.
> >
> > It's in my muscle memory somehow.  Always did it that way because I
> > didn't want the same delimiter as in the git part of the message.  Time
> > to change that habit I suppose :) (or automate more of the process).
>
> I guess if you're committing your own code it doesn't matter, but mixing
> them will trip up git-am and such.
>
> The patch LGTM, but it's mostly Juzhe's code so it's probably best to at
> least give him a chance to see it when he's awake.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] riscv: Clarify vlmax and length handling.
  2023-05-10 15:24 [PATCH] riscv: Clarify vlmax and length handling Robin Dapp
  2023-05-10 18:32 ` Palmer Dabbelt
@ 2023-05-11  2:30 ` juzhe.zhong
  1 sibling, 0 replies; 9+ messages in thread
From: juzhe.zhong @ 2023-05-11  2:30 UTC (permalink / raw)
  To: Robin Dapp, gcc-patches, kito.cheng, collison, palmer, jeffreyalaw
  Cc: Robin Dapp

[-- Attachment #1: Type: text/plain, Size: 13040 bytes --]

This part LGTM.



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-05-10 23:24
To: gcc-patches; juzhe.zhong@rivai.ai; Kito Cheng; Michael Collison; palmer; jeffreyalaw
CC: rdapp.gcc
Subject: [PATCH] riscv: Clarify vlmax and length handling.
Hi,
 
this patch tries to improve the wrappers that emit either vlmax or
non-vlmax operations.  Now, emit_len_op can be used to
emit a regular operation.  Depending on whether a length != NULL
is passed either no VLMAX flags are set or we emit a vsetvli and
set VLMAX flags.  The patch also adds some comments that describes
some of the rationale of the current handling of vlmax/nonvlmax
operations.
 
Bootstrapped and regtested.
 
Regards
Robin
 
--
 
gcc/ChangeLog:
 
* config/riscv/autovec.md: Use renamed functions.
* config/riscv/riscv-protos.h (emit_vlmax_op): Rename.
(emit_vlmax_reg_op): To this.
(emit_nonvlmax_op): Rename.
(emit_len_op): To this.
(emit_nonvlmax_binop): Rename.
(emit_len_binop): To this.
* config/riscv/riscv-v.cc (emit_pred_op): Add default parameter.
(emit_pred_binop): Remove vlmax_p.
(emit_vlmax_op): Rename.
(emit_vlmax_reg_op): To this.
(emit_nonvlmax_op): Rename.
(emit_len_op): To this.
(emit_nonvlmax_binop): Rename.
(emit_len_binop): To this.
(sew64_scalar_helper): Use renamed functions.
(expand_tuple_move): Use renamed functions.
* config/riscv/riscv.cc (vector_zero_call_used_regs): Use
renamed functions.
* config/riscv/vector.md: Use renamed functions.
---
gcc/config/riscv/autovec.md     | 24 +++++-----
gcc/config/riscv/riscv-protos.h |  8 ++--
gcc/config/riscv/riscv-v.cc     | 82 ++++++++++++++++++++-------------
gcc/config/riscv/riscv.cc       |  4 +-
gcc/config/riscv/vector.md      | 12 +++--
5 files changed, 75 insertions(+), 55 deletions(-)
 
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index 15f8d007e07..8347e42bb9c 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -31,8 +31,8 @@ (define_expand "len_load_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
{
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-   operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+      operands[1], operands[2], <VM>mode);
   DONE;
})
@@ -43,8 +43,8 @@ (define_expand "len_store_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
{
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-   operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+      operands[1], operands[2], <VM>mode);
   DONE;
})
@@ -79,15 +79,15 @@ (define_expand "<optab><mode>3"
       if (inner == E_QImode || inner == E_HImode || inner == E_SImode)
op2mode = inner;
-      riscv_vector::emit_nonvlmax_binop (code_for_pred_scalar
- (<BINOP_TO_UPPERCASE>, <MODE>mode),
- operands[0], operands[1], cst,
- NULL_RTX, <VM>mode, op2mode);
+      riscv_vector::emit_len_binop (code_for_pred_scalar
+     (<BINOP_TO_UPPERCASE>, <MODE>mode),
+     operands[0], operands[1], cst,
+     NULL_RTX, <VM>mode, op2mode);
     }
   else
-    riscv_vector::emit_nonvlmax_binop (code_for_pred
-        (<BINOP_TO_UPPERCASE>, <MODE>mode),
-        operands[0], operands[1], operands[2],
-        NULL_RTX, <VM>mode);
+    riscv_vector::emit_len_binop (code_for_pred
+   (<BINOP_TO_UPPERCASE>, <MODE>mode),
+   operands[0], operands[1], operands[2],
+   NULL_RTX, <VM>mode);
   DONE;
})
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 75cdb90b9c9..bfdf09b17ee 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -167,10 +167,10 @@ bool legitimize_move (rtx, rtx, machine_mode);
void emit_vlmax_vsetvl (machine_mode, rtx);
void emit_hard_vlmax_vsetvl (machine_mode, rtx);
void emit_vlmax_op (unsigned, rtx, rtx, machine_mode);
-void emit_vlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
-   machine_mode = VOIDmode);
+void emit_vlmax_reg_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode, machine_mode =
+      VOIDmode);
enum vlmul_type get_vlmul (machine_mode);
unsigned int get_ratio (machine_mode);
unsigned int get_nf (machine_mode);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 3c43dfc5eea..07b7783282f 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -99,27 +99,24 @@ public:
     add_vundef_operand (dest_mode);
   }
-  void set_len_and_policy (rtx len, bool vlmax_p)
+  void set_len_and_policy (rtx len, bool force_vlmax = false)
     {
+      bool vlmax_p = force_vlmax;
       gcc_assert (has_dest);
-      gcc_assert (len || vlmax_p);
-      if (len)
- add_input_operand (len, Pmode);
-      else
+      if (!len)
{
-   rtx vlmax = gen_reg_rtx (Pmode);
-   emit_vlmax_vsetvl (dest_mode, vlmax);
-   add_input_operand (vlmax, Pmode);
+   vlmax_p = true;
+   len = gen_reg_rtx (Pmode);
+   emit_vlmax_vsetvl (dest_mode, len);
}
+      add_input_operand (len, Pmode);
+
       if (GET_MODE_CLASS (dest_mode) != MODE_VECTOR_BOOL)
add_policy_operand (get_prefer_tail_policy (), get_prefer_mask_policy ());
-      if (vlmax_p)
- add_avl_type_operand (avl_type::VLMAX);
-      else
- add_avl_type_operand (avl_type::NONVLMAX);
+      add_avl_type_operand (vlmax_p ? avl_type::VLMAX : avl_type::NONVLMAX);
     }
   void expand (enum insn_code icode, bool temporary_volatile_p = false)
@@ -251,14 +248,14 @@ set_expander_dest_and_mask (insn_expander<MAX_OPERANDS> &e, rtx mask, rtx dest,
/* Emit an RVV unmask && vl mov from SRC to DEST.  */
static void
emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
-       machine_mode mask_mode, bool vlmax_p)
+       machine_mode mask_mode, bool force_vlmax = false)
{
   insn_expander<8> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
   e.add_input_operand (src, GET_MODE (src));
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len, force_vlmax);
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src));
}
@@ -266,8 +263,7 @@ emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
/* Emit an RVV unmask && vl mov from SRC to DEST.  */
static void
emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
- rtx len, machine_mode mask_mode, machine_mode op2mode,
- bool vlmax_p)
+ rtx len, machine_mode mask_mode, machine_mode op2mode)
{
   insn_expander<9> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
@@ -275,37 +271,59 @@ emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
   e.add_input_operand (src1, GET_MODE (src1));
   e.add_input_operand (src2, op2mode == VOIDmode ? GET_MODE (src2) : op2mode);
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len);
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src1) || MEM_P (src2));
}
+/* The RISC-V vsetvli pass uses vlmax operation for optimization.  Whether or
+   not an instruction actually is a vlmax operation is not recognizable from
+   the length operand alone but the avl_type operand is used instead.
+   In general, there are two cases:
+
+    - Emit a vlmax operation by passing a NULL length.  Here we emit
+      a vsetvli with vlmax configuration and set the avl_type to VLMAX.
+    - Emit an operation that uses the existing (last-set) length and
+      set the avl_type to NONVLMAX.
+
+    Sometimes we also need to set the VLMAX avl_type to an operation that
+    already uses a given length register.  This can happen during or after
+    register allocation when we are not allowed to create a new register.
+    For that case we also allow to set the avl_type to VLMAX.
+*/
+
+/* This function emits a VLMAX vsetvli followed by the actual operation.  */
void
emit_vlmax_op (unsigned icode, rtx dest, rtx src, machine_mode mask_mode)
{
-  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode);
}
+/* This function emits an operation with a given LEN that is determined
+   by a previously emitted VLMAX vsetvli.  */
void
-emit_vlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-        machine_mode mask_mode)
+emit_len_op (unsigned icode, rtx dest, rtx src, rtx len,
+      machine_mode mask_mode)
{
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode);
}
+/* This function emits an operation with a given LEN that is known to be
+   a preceding VLMAX.  It also sets the VLMAX flag which allows further
+   optimization in the vsetvli pass.  */
void
-emit_nonvlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-   machine_mode mask_mode)
+emit_vlmax_reg_op (unsigned icode, rtx dest, rtx src, rtx len,
+    machine_mode mask_mode)
{
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, false);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode,
+ /* Force VLMAX */ true);
}
void
-emit_nonvlmax_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
-      machine_mode mask_mode, machine_mode op2mode)
+emit_len_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
+ machine_mode mask_mode, machine_mode op2mode)
{
-  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode, op2mode,
-    false);
+  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode, op2mode);
}
static void
@@ -667,8 +685,8 @@ sew64_scalar_helper (rtx *operands, rtx *scalar_op, rtx vl,
     *scalar_op = force_reg (scalar_mode, *scalar_op);
   rtx tmp = gen_reg_rtx (vector_mode);
-  riscv_vector::emit_nonvlmax_op (code_for_pred_broadcast (vector_mode), tmp,
-   *scalar_op, vl, mask_mode);
+  riscv_vector::emit_len_op (code_for_pred_broadcast (vector_mode), tmp,
+      *scalar_op, vl, mask_mode);
   emit_vector_func (operands, tmp);
   return true;
@@ -974,7 +992,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
      if (fractional_p)
- emit_vlmax_op (code_for_pred_mov (subpart_mode), subreg, mem,
+ emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), subreg, mem,
       ops[4], mask_mode);
      else
emit_move_insn (subreg, mem);
@@ -996,7 +1014,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
      if (fractional_p)
- emit_vlmax_op (code_for_pred_mov (subpart_mode), mem, subreg,
+ emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), mem, subreg,
       ops[4], mask_mode);
      else
emit_move_insn (mem, subreg);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 9e08a1bfe35..9a493295a38 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7373,8 +7373,8 @@ vector_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
      emitted_vlmax_vsetvl = true;
    }
-   riscv_vector::emit_vlmax_op (code_for_pred_mov (mode), target,
-        CONST0_RTX (mode), vl, mask_mode);
+   riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (mode), target,
+    CONST0_RTX (mode), vl, mask_mode);
  SET_HARD_REG_BIT (zeroed_hardregs, regno);
}
diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index 9ccc0d6a513..75479f27bcd 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -721,8 +721,9 @@ (define_insn_and_split "*mov<V_FRACT:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<V_FRACT:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<V_FRACT:MODE>mode),
-      operands[0], operands[1], operands[2], <VM>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<V_FRACT:MODE>mode),
+        operands[0], operands[1], operands[2],
+        <VM>mode);
     }
   DONE;
})
@@ -741,8 +742,9 @@ (define_insn_and_split "*mov<VB:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<VB:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<VB:MODE>mode),
-      operands[0], operands[1], operands[2], <VB:MODE>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<VB:MODE>mode),
+        operands[0], operands[1], operands[2],
+        <VB:MODE>mode);
     }
   DONE;
})
@@ -806,7 +808,7 @@ (define_expand "vec_duplicate<mode>"
   "TARGET_VECTOR"
   {
     riscv_vector::emit_vlmax_op (code_for_pred_broadcast (<MODE>mode),
- operands[0], operands[1], <VM>mode);
+ operands[0], operands[1], <VM>mode);
     DONE;
   }
)
-- 
2.40.0
 
 
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] RISC-V: Clarify vlmax and length handling.
  2023-05-11  2:26       ` Kito Cheng
@ 2023-05-11 10:29         ` Robin Dapp
  2023-05-11 10:35           ` juzhe.zhong
  0 siblings, 1 reply; 9+ messages in thread
From: Robin Dapp @ 2023-05-11 10:29 UTC (permalink / raw)
  To: Kito Cheng, Palmer Dabbelt
  Cc: gcc-patches, juzhe.zhong, collison, jeffreyalaw

Changes from v1:
  - Change subject to RISC-V ;)
  - Minor comment updates and rebasing.

This patch tries to improve the wrappers that emit either vlmax or
non-vlmax operations.  Now, emit_len_op can be used to
emit a regular operation.  Depending on whether a length != NULL
is passed either no VLMAX flags are set or we emit a vsetvli and
set VLMAX flags.  The patch also adds some comments that describes
some of the rationale of the current handling of vlmax/nonvlmax
operations.

gcc/ChangeLog:

	* config/riscv/autovec.md: Use renamed functions.
	* config/riscv/riscv-protos.h (emit_vlmax_op): Rename.
	(emit_vlmax_reg_op): To this.
	(emit_nonvlmax_op): Rename.
	(emit_len_op): To this.
	(emit_nonvlmax_binop): Rename.
	(emit_len_binop): To this.
	* config/riscv/riscv-v.cc (emit_pred_op): Add default parameter.
	(emit_pred_binop): Remove vlmax_p.
	(emit_vlmax_op): Rename.
	(emit_vlmax_reg_op): To this.
	(emit_nonvlmax_op): Rename.
	(emit_len_op): To this.
	(emit_nonvlmax_binop): Rename.
	(emit_len_binop): To this.
	(sew64_scalar_helper): Use renamed functions.
	(expand_tuple_move): Use renamed functions.
	* config/riscv/riscv.cc (vector_zero_call_used_regs): Use
	renamed functions.
	* config/riscv/vector.md: Use renamed functions.
---
 gcc/config/riscv/autovec.md     | 26 ++++-----
 gcc/config/riscv/riscv-protos.h |  8 +--
 gcc/config/riscv/riscv-v.cc     | 94 ++++++++++++++++++++-------------
 gcc/config/riscv/riscv.cc       |  4 +-
 gcc/config/riscv/vector.md      | 12 +++--
 5 files changed, 83 insertions(+), 61 deletions(-)

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index e249f4be704..58926ed3e67 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -31,8 +31,8 @@ (define_expand "len_load_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
 {
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-				  operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+			     operands[1], operands[2], <VM>mode);
   DONE;
 })
 
@@ -43,8 +43,8 @@ (define_expand "len_store_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
 {
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-				  operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+			     operands[1], operands[2], <VM>mode);
   DONE;
 })
 
@@ -106,16 +106,16 @@ (define_expand "<optab><mode>3"
     {
       rtx cst;
       gcc_assert (const_vec_duplicate_p(operands[2], &cst));
-      riscv_vector::emit_nonvlmax_binop (code_for_pred_scalar
-					 (<CODE>, <MODE>mode),
-					 operands[0], operands[1], cst,
-					 NULL, <VM>mode,
-					 <VEL>mode);
+      riscv_vector::emit_len_binop (code_for_pred_scalar
+				    (<CODE>, <MODE>mode),
+				    operands[0], operands[1], cst,
+				    NULL, <VM>mode,
+				    <VEL>mode);
     }
   else
-    riscv_vector::emit_nonvlmax_binop (code_for_pred
-				       (<CODE>, <MODE>mode),
-				       operands[0], operands[1], operands[2],
-				       NULL, <VM>mode);
+    riscv_vector::emit_len_binop (code_for_pred
+				  (<CODE>, <MODE>mode),
+				  operands[0], operands[1], operands[2],
+				  NULL, <VM>mode);
   DONE;
 })
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 4d0589e502c..55a43d6270e 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -167,10 +167,10 @@ bool legitimize_move (rtx, rtx, machine_mode);
 void emit_vlmax_vsetvl (machine_mode, rtx);
 void emit_hard_vlmax_vsetvl (machine_mode, rtx);
 void emit_vlmax_op (unsigned, rtx, rtx, machine_mode);
-void emit_vlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
-			  machine_mode = VOIDmode);
+void emit_vlmax_reg_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
+		     machine_mode = VOIDmode);
 enum vlmul_type get_vlmul (machine_mode);
 unsigned int get_ratio (machine_mode);
 unsigned int get_nf (machine_mode);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 8f46226d571..7cf5ec9f3ba 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -99,27 +99,24 @@ public:
     add_vundef_operand (dest_mode);
   }
 
-  void set_len_and_policy (rtx len, bool vlmax_p)
+  void set_len_and_policy (rtx len, bool force_vlmax = false)
     {
+      bool vlmax_p = force_vlmax;
       gcc_assert (has_dest);
-      gcc_assert (len || vlmax_p);
 
-      if (len)
-	add_input_operand (len, Pmode);
-      else
+      if (!len)
 	{
-	  rtx vlmax = gen_reg_rtx (Pmode);
-	  emit_vlmax_vsetvl (dest_mode, vlmax);
-	  add_input_operand (vlmax, Pmode);
+	  vlmax_p = true;
+	  len = gen_reg_rtx (Pmode);
+	  emit_vlmax_vsetvl (dest_mode, len);
 	}
 
+      add_input_operand (len, Pmode);
+
       if (GET_MODE_CLASS (dest_mode) != MODE_VECTOR_BOOL)
 	add_policy_operand (get_prefer_tail_policy (), get_prefer_mask_policy ());
 
-      if (vlmax_p)
-	add_avl_type_operand (avl_type::VLMAX);
-      else
-	add_avl_type_operand (avl_type::NONVLMAX);
+      add_avl_type_operand (vlmax_p ? avl_type::VLMAX : avl_type::NONVLMAX);
     }
 
   void expand (enum insn_code icode, bool temporary_volatile_p = false)
@@ -232,14 +229,14 @@ autovec_use_vlmax_p (void)
 /* Emit an RVV unmask && vl mov from SRC to DEST.  */
 static void
 emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
-	      machine_mode mask_mode, bool vlmax_p)
+	      machine_mode mask_mode, bool force_vlmax = false)
 {
   insn_expander<8> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
 
   e.add_input_operand (src, GET_MODE (src));
 
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len, force_vlmax);
 
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src));
 }
@@ -248,8 +245,8 @@ emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
    specified using SCALAR_MODE.  */
 static void
 emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
-		 rtx len, machine_mode mask_mode, machine_mode scalar_mode,
-		 bool vlmax_p)
+		 rtx len, machine_mode mask_mode,
+		 machine_mode scalar_mode = VOIDmode)
 {
   insn_expander<9> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
@@ -267,37 +264,60 @@ emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
   else
     e.add_input_operand (src2, scalar_mode);
 
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len);
 
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src1) || MEM_P (src2));
 }
 
+/* The RISC-V vsetvli pass uses "known vlmax" operations for optimization.
+   Whether or not an instruction actually is a vlmax operation is not
+   recognizable from the length operand alone but the avl_type operand
+   is used instead.  In general, there are two cases:
+
+    - Emit a vlmax operation by passing a NULL length.  Here we emit
+      a vsetvli with vlmax configuration and set the avl_type to VLMAX.
+    - Emit an operation that uses the existing (last-set) length and
+      set the avl_type to NONVLMAX.
+
+    Sometimes we also need to set the VLMAX avl_type to an operation that
+    already uses a given length register.  This can happen during or after
+    register allocation when we are not allowed to create a new register.
+    For that case we also allow to set the avl_type to VLMAX.
+*/
+
+/* This function emits a VLMAX vsetvli followed by the actual operation.  */
 void
 emit_vlmax_op (unsigned icode, rtx dest, rtx src, machine_mode mask_mode)
 {
-  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode);
 }
 
+/* This function emits an operation with a given LEN that is determined
+   by a previously emitted VLMAX vsetvli.  */
 void
-emit_vlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-	       machine_mode mask_mode)
+emit_len_op (unsigned icode, rtx dest, rtx src, rtx len,
+	     machine_mode mask_mode)
 {
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode);
 }
 
+/* This function emits an operation with a given LEN that is known to be
+   a preceding VLMAX.  It also sets the VLMAX flag which allows further
+   optimization in the vsetvli pass.  */
 void
-emit_nonvlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-		  machine_mode mask_mode)
+emit_vlmax_reg_op (unsigned icode, rtx dest, rtx src, rtx len,
+		   machine_mode mask_mode)
 {
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, false);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode,
+		/* Force VLMAX */ true);
 }
 
 void
-emit_nonvlmax_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
-		     machine_mode mask_mode, machine_mode scalar_mode)
+emit_len_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
+		machine_mode mask_mode, machine_mode scalar_mode)
 {
-  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode,
-		   scalar_mode, len ? false : true);
+  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len,
+		   mask_mode, scalar_mode);
 }
 
 /* Emit vid.v instruction.  */
@@ -345,14 +365,14 @@ expand_vec_series (rtx dest, rtx base, rtx step)
 	  int shift = exact_log2 (INTVAL (step));
 	  rtx shift_amount = gen_int_mode (shift, Pmode);
 	  insn_code icode = code_for_pred_scalar (ASHIFT, mode);
-	  emit_nonvlmax_binop (icode, step_adj, vid, shift_amount,
-			       NULL, mask_mode, Pmode);
+	  emit_len_binop (icode, step_adj, vid, shift_amount,
+			  NULL, mask_mode, Pmode);
 	}
       else
 	{
 	  insn_code icode = code_for_pred_scalar (MULT, mode);
-	  emit_nonvlmax_binop (icode, step_adj, vid, step,
-			       NULL, mask_mode, inner_mode);
+	  emit_len_binop (icode, step_adj, vid, step,
+			  NULL, mask_mode, inner_mode);
 	}
     }
 
@@ -367,7 +387,7 @@ expand_vec_series (rtx dest, rtx base, rtx step)
     {
       rtx result = gen_reg_rtx (mode);
       insn_code icode = code_for_pred_scalar (PLUS, mode);
-      emit_nonvlmax_binop (icode, result, step_adj, base,
+      emit_len_binop (icode, result, step_adj, base,
 			   NULL, mask_mode, inner_mode);
       emit_move_insn (dest, result);
     }
@@ -739,8 +759,8 @@ sew64_scalar_helper (rtx *operands, rtx *scalar_op, rtx vl,
     *scalar_op = force_reg (scalar_mode, *scalar_op);
 
   rtx tmp = gen_reg_rtx (vector_mode);
-  riscv_vector::emit_nonvlmax_op (code_for_pred_broadcast (vector_mode), tmp,
-				  *scalar_op, vl, mask_mode);
+  riscv_vector::emit_len_op (code_for_pred_broadcast (vector_mode), tmp,
+			     *scalar_op, vl, mask_mode);
   emit_vector_func (operands, tmp);
 
   return true;
@@ -1046,7 +1066,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
 	      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
 
 	      if (fractional_p)
-		emit_vlmax_op (code_for_pred_mov (subpart_mode), subreg, mem,
+		emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), subreg, mem,
 			       ops[4], mask_mode);
 	      else
 		emit_move_insn (subreg, mem);
@@ -1068,7 +1088,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
 	      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
 
 	      if (fractional_p)
-		emit_vlmax_op (code_for_pred_mov (subpart_mode), mem, subreg,
+		emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), mem, subreg,
 			       ops[4], mask_mode);
 	      else
 		emit_move_insn (mem, subreg);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 02d581589ac..cecb4d49092 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7391,8 +7391,8 @@ vector_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
 	      emitted_vlmax_vsetvl = true;
 	    }
 
-	  riscv_vector::emit_vlmax_op (code_for_pred_mov (mode), target,
-				       CONST0_RTX (mode), vl, mask_mode);
+	  riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (mode), target,
+					   CONST0_RTX (mode), vl, mask_mode);
 
 	  SET_HARD_REG_BIT (zeroed_hardregs, regno);
 	}
diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index 9ccc0d6a513..75479f27bcd 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -721,8 +721,9 @@ (define_insn_and_split "*mov<V_FRACT:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<V_FRACT:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<V_FRACT:MODE>mode),
-      		operands[0], operands[1], operands[2], <VM>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<V_FRACT:MODE>mode),
+				       operands[0], operands[1], operands[2],
+				       <VM>mode);
     }
   DONE;
 })
@@ -741,8 +742,9 @@ (define_insn_and_split "*mov<VB:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<VB:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<VB:MODE>mode),
-      		operands[0], operands[1], operands[2], <VB:MODE>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<VB:MODE>mode),
+				       operands[0], operands[1], operands[2],
+				       <VB:MODE>mode);
     }
   DONE;
 })
@@ -806,7 +808,7 @@ (define_expand "vec_duplicate<mode>"
   "TARGET_VECTOR"
   {
     riscv_vector::emit_vlmax_op (code_for_pred_broadcast (<MODE>mode),
-		operands[0], operands[1], <VM>mode);
+				 operands[0], operands[1], <VM>mode);
     DONE;
   }
 )
-- 
2.40.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RISC-V: Clarify vlmax and length handling.
  2023-05-11 10:29         ` [PATCH v2] RISC-V: " Robin Dapp
@ 2023-05-11 10:35           ` juzhe.zhong
  2023-05-11 11:09             ` Kito Cheng
  0 siblings, 1 reply; 9+ messages in thread
From: juzhe.zhong @ 2023-05-11 10:35 UTC (permalink / raw)
  To: Robin Dapp, kito.cheng, palmer; +Cc: gcc-patches, collison, jeffreyalaw

[-- Attachment #1: Type: text/plain, Size: 14027 bytes --]

LGTM. Plz commit it now. Then I can rebase vec_init patch.



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-05-11 18:29
To: Kito Cheng; Palmer Dabbelt
CC: gcc-patches; juzhe.zhong; collison; jeffreyalaw
Subject: [PATCH v2] RISC-V: Clarify vlmax and length handling.
Changes from v1:
  - Change subject to RISC-V ;)
  - Minor comment updates and rebasing.
 
This patch tries to improve the wrappers that emit either vlmax or
non-vlmax operations.  Now, emit_len_op can be used to
emit a regular operation.  Depending on whether a length != NULL
is passed either no VLMAX flags are set or we emit a vsetvli and
set VLMAX flags.  The patch also adds some comments that describes
some of the rationale of the current handling of vlmax/nonvlmax
operations.
 
gcc/ChangeLog:
 
* config/riscv/autovec.md: Use renamed functions.
* config/riscv/riscv-protos.h (emit_vlmax_op): Rename.
(emit_vlmax_reg_op): To this.
(emit_nonvlmax_op): Rename.
(emit_len_op): To this.
(emit_nonvlmax_binop): Rename.
(emit_len_binop): To this.
* config/riscv/riscv-v.cc (emit_pred_op): Add default parameter.
(emit_pred_binop): Remove vlmax_p.
(emit_vlmax_op): Rename.
(emit_vlmax_reg_op): To this.
(emit_nonvlmax_op): Rename.
(emit_len_op): To this.
(emit_nonvlmax_binop): Rename.
(emit_len_binop): To this.
(sew64_scalar_helper): Use renamed functions.
(expand_tuple_move): Use renamed functions.
* config/riscv/riscv.cc (vector_zero_call_used_regs): Use
renamed functions.
* config/riscv/vector.md: Use renamed functions.
---
gcc/config/riscv/autovec.md     | 26 ++++-----
gcc/config/riscv/riscv-protos.h |  8 +--
gcc/config/riscv/riscv-v.cc     | 94 ++++++++++++++++++++-------------
gcc/config/riscv/riscv.cc       |  4 +-
gcc/config/riscv/vector.md      | 12 +++--
5 files changed, 83 insertions(+), 61 deletions(-)
 
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index e249f4be704..58926ed3e67 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -31,8 +31,8 @@ (define_expand "len_load_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
{
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-   operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+      operands[1], operands[2], <VM>mode);
   DONE;
})
@@ -43,8 +43,8 @@ (define_expand "len_store_<mode>"
    (match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
{
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode), operands[0],
-   operands[1], operands[2], <VM>mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
+      operands[1], operands[2], <VM>mode);
   DONE;
})
@@ -106,16 +106,16 @@ (define_expand "<optab><mode>3"
     {
       rtx cst;
       gcc_assert (const_vec_duplicate_p(operands[2], &cst));
-      riscv_vector::emit_nonvlmax_binop (code_for_pred_scalar
- (<CODE>, <MODE>mode),
- operands[0], operands[1], cst,
- NULL, <VM>mode,
- <VEL>mode);
+      riscv_vector::emit_len_binop (code_for_pred_scalar
+     (<CODE>, <MODE>mode),
+     operands[0], operands[1], cst,
+     NULL, <VM>mode,
+     <VEL>mode);
     }
   else
-    riscv_vector::emit_nonvlmax_binop (code_for_pred
-        (<CODE>, <MODE>mode),
-        operands[0], operands[1], operands[2],
-        NULL, <VM>mode);
+    riscv_vector::emit_len_binop (code_for_pred
+   (<CODE>, <MODE>mode),
+   operands[0], operands[1], operands[2],
+   NULL, <VM>mode);
   DONE;
})
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 4d0589e502c..55a43d6270e 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -167,10 +167,10 @@ bool legitimize_move (rtx, rtx, machine_mode);
void emit_vlmax_vsetvl (machine_mode, rtx);
void emit_hard_vlmax_vsetvl (machine_mode, rtx);
void emit_vlmax_op (unsigned, rtx, rtx, machine_mode);
-void emit_vlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
-void emit_nonvlmax_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
-   machine_mode = VOIDmode);
+void emit_vlmax_reg_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_op (unsigned, rtx, rtx, rtx, machine_mode);
+void emit_len_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
+      machine_mode = VOIDmode);
enum vlmul_type get_vlmul (machine_mode);
unsigned int get_ratio (machine_mode);
unsigned int get_nf (machine_mode);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 8f46226d571..7cf5ec9f3ba 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -99,27 +99,24 @@ public:
     add_vundef_operand (dest_mode);
   }
-  void set_len_and_policy (rtx len, bool vlmax_p)
+  void set_len_and_policy (rtx len, bool force_vlmax = false)
     {
+      bool vlmax_p = force_vlmax;
       gcc_assert (has_dest);
-      gcc_assert (len || vlmax_p);
-      if (len)
- add_input_operand (len, Pmode);
-      else
+      if (!len)
{
-   rtx vlmax = gen_reg_rtx (Pmode);
-   emit_vlmax_vsetvl (dest_mode, vlmax);
-   add_input_operand (vlmax, Pmode);
+   vlmax_p = true;
+   len = gen_reg_rtx (Pmode);
+   emit_vlmax_vsetvl (dest_mode, len);
}
+      add_input_operand (len, Pmode);
+
       if (GET_MODE_CLASS (dest_mode) != MODE_VECTOR_BOOL)
add_policy_operand (get_prefer_tail_policy (), get_prefer_mask_policy ());
-      if (vlmax_p)
- add_avl_type_operand (avl_type::VLMAX);
-      else
- add_avl_type_operand (avl_type::NONVLMAX);
+      add_avl_type_operand (vlmax_p ? avl_type::VLMAX : avl_type::NONVLMAX);
     }
   void expand (enum insn_code icode, bool temporary_volatile_p = false)
@@ -232,14 +229,14 @@ autovec_use_vlmax_p (void)
/* Emit an RVV unmask && vl mov from SRC to DEST.  */
static void
emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
-       machine_mode mask_mode, bool vlmax_p)
+       machine_mode mask_mode, bool force_vlmax = false)
{
   insn_expander<8> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
   e.add_input_operand (src, GET_MODE (src));
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len, force_vlmax);
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src));
}
@@ -248,8 +245,8 @@ emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
    specified using SCALAR_MODE.  */
static void
emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
- rtx len, machine_mode mask_mode, machine_mode scalar_mode,
- bool vlmax_p)
+ rtx len, machine_mode mask_mode,
+ machine_mode scalar_mode = VOIDmode)
{
   insn_expander<9> e;
   e.set_dest_and_mask (mask, dest, mask_mode);
@@ -267,37 +264,60 @@ emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
   else
     e.add_input_operand (src2, scalar_mode);
-  e.set_len_and_policy (len, vlmax_p);
+  e.set_len_and_policy (len);
   e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src1) || MEM_P (src2));
}
+/* The RISC-V vsetvli pass uses "known vlmax" operations for optimization.
+   Whether or not an instruction actually is a vlmax operation is not
+   recognizable from the length operand alone but the avl_type operand
+   is used instead.  In general, there are two cases:
+
+    - Emit a vlmax operation by passing a NULL length.  Here we emit
+      a vsetvli with vlmax configuration and set the avl_type to VLMAX.
+    - Emit an operation that uses the existing (last-set) length and
+      set the avl_type to NONVLMAX.
+
+    Sometimes we also need to set the VLMAX avl_type to an operation that
+    already uses a given length register.  This can happen during or after
+    register allocation when we are not allowed to create a new register.
+    For that case we also allow to set the avl_type to VLMAX.
+*/
+
+/* This function emits a VLMAX vsetvli followed by the actual operation.  */
void
emit_vlmax_op (unsigned icode, rtx dest, rtx src, machine_mode mask_mode)
{
-  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode);
}
+/* This function emits an operation with a given LEN that is determined
+   by a previously emitted VLMAX vsetvli.  */
void
-emit_vlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-        machine_mode mask_mode)
+emit_len_op (unsigned icode, rtx dest, rtx src, rtx len,
+      machine_mode mask_mode)
{
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, true);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode);
}
+/* This function emits an operation with a given LEN that is known to be
+   a preceding VLMAX.  It also sets the VLMAX flag which allows further
+   optimization in the vsetvli pass.  */
void
-emit_nonvlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
-   machine_mode mask_mode)
+emit_vlmax_reg_op (unsigned icode, rtx dest, rtx src, rtx len,
+    machine_mode mask_mode)
{
-  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, false);
+  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode,
+ /* Force VLMAX */ true);
}
void
-emit_nonvlmax_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
-      machine_mode mask_mode, machine_mode scalar_mode)
+emit_len_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
+ machine_mode mask_mode, machine_mode scalar_mode)
{
-  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode,
-    scalar_mode, len ? false : true);
+  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len,
+    mask_mode, scalar_mode);
}
/* Emit vid.v instruction.  */
@@ -345,14 +365,14 @@ expand_vec_series (rtx dest, rtx base, rtx step)
  int shift = exact_log2 (INTVAL (step));
  rtx shift_amount = gen_int_mode (shift, Pmode);
  insn_code icode = code_for_pred_scalar (ASHIFT, mode);
-   emit_nonvlmax_binop (icode, step_adj, vid, shift_amount,
-        NULL, mask_mode, Pmode);
+   emit_len_binop (icode, step_adj, vid, shift_amount,
+   NULL, mask_mode, Pmode);
}
       else
{
  insn_code icode = code_for_pred_scalar (MULT, mode);
-   emit_nonvlmax_binop (icode, step_adj, vid, step,
-        NULL, mask_mode, inner_mode);
+   emit_len_binop (icode, step_adj, vid, step,
+   NULL, mask_mode, inner_mode);
}
     }
@@ -367,7 +387,7 @@ expand_vec_series (rtx dest, rtx base, rtx step)
     {
       rtx result = gen_reg_rtx (mode);
       insn_code icode = code_for_pred_scalar (PLUS, mode);
-      emit_nonvlmax_binop (icode, result, step_adj, base,
+      emit_len_binop (icode, result, step_adj, base,
   NULL, mask_mode, inner_mode);
       emit_move_insn (dest, result);
     }
@@ -739,8 +759,8 @@ sew64_scalar_helper (rtx *operands, rtx *scalar_op, rtx vl,
     *scalar_op = force_reg (scalar_mode, *scalar_op);
   rtx tmp = gen_reg_rtx (vector_mode);
-  riscv_vector::emit_nonvlmax_op (code_for_pred_broadcast (vector_mode), tmp,
-   *scalar_op, vl, mask_mode);
+  riscv_vector::emit_len_op (code_for_pred_broadcast (vector_mode), tmp,
+      *scalar_op, vl, mask_mode);
   emit_vector_func (operands, tmp);
   return true;
@@ -1046,7 +1066,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
      if (fractional_p)
- emit_vlmax_op (code_for_pred_mov (subpart_mode), subreg, mem,
+ emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), subreg, mem,
       ops[4], mask_mode);
      else
emit_move_insn (subreg, mem);
@@ -1068,7 +1088,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
      rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
      if (fractional_p)
- emit_vlmax_op (code_for_pred_mov (subpart_mode), mem, subreg,
+ emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), mem, subreg,
       ops[4], mask_mode);
      else
emit_move_insn (mem, subreg);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 02d581589ac..cecb4d49092 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7391,8 +7391,8 @@ vector_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
      emitted_vlmax_vsetvl = true;
    }
-   riscv_vector::emit_vlmax_op (code_for_pred_mov (mode), target,
-        CONST0_RTX (mode), vl, mask_mode);
+   riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (mode), target,
+    CONST0_RTX (mode), vl, mask_mode);
  SET_HARD_REG_BIT (zeroed_hardregs, regno);
}
diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index 9ccc0d6a513..75479f27bcd 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -721,8 +721,9 @@ (define_insn_and_split "*mov<V_FRACT:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<V_FRACT:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<V_FRACT:MODE>mode),
-      operands[0], operands[1], operands[2], <VM>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<V_FRACT:MODE>mode),
+        operands[0], operands[1], operands[2],
+        <VM>mode);
     }
   DONE;
})
@@ -741,8 +742,9 @@ (define_insn_and_split "*mov<VB:mode><P:mode>_lra"
   else
     {
       riscv_vector::emit_vlmax_vsetvl (<VB:MODE>mode, operands[2]);
-      riscv_vector::emit_vlmax_op (code_for_pred_mov (<VB:MODE>mode),
-      operands[0], operands[1], operands[2], <VB:MODE>mode);
+      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<VB:MODE>mode),
+        operands[0], operands[1], operands[2],
+        <VB:MODE>mode);
     }
   DONE;
})
@@ -806,7 +808,7 @@ (define_expand "vec_duplicate<mode>"
   "TARGET_VECTOR"
   {
     riscv_vector::emit_vlmax_op (code_for_pred_broadcast (<MODE>mode),
- operands[0], operands[1], <VM>mode);
+ operands[0], operands[1], <VM>mode);
     DONE;
   }
)
-- 
2.40.0
 
 
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RISC-V: Clarify vlmax and length handling.
  2023-05-11 10:35           ` juzhe.zhong
@ 2023-05-11 11:09             ` Kito Cheng
  0 siblings, 0 replies; 9+ messages in thread
From: Kito Cheng @ 2023-05-11 11:09 UTC (permalink / raw)
  To: 钟居哲
  Cc: Robin Dapp, palmer, gcc-patches, collison, jeffreyalaw

[-- Attachment #1: Type: text/plain, Size: 14859 bytes --]

LGTM

juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai> 於 2023年5月11日 週四 18:36 寫道:

> LGTM. Plz commit it now. Then I can rebase vec_init patch.
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Robin Dapp
> Date: 2023-05-11 18:29
> To: Kito Cheng; Palmer Dabbelt
> CC: gcc-patches; juzhe.zhong; collison; jeffreyalaw
> Subject: [PATCH v2] RISC-V: Clarify vlmax and length handling.
> Changes from v1:
>   - Change subject to RISC-V ;)
>   - Minor comment updates and rebasing.
>
> This patch tries to improve the wrappers that emit either vlmax or
> non-vlmax operations.  Now, emit_len_op can be used to
> emit a regular operation.  Depending on whether a length != NULL
> is passed either no VLMAX flags are set or we emit a vsetvli and
> set VLMAX flags.  The patch also adds some comments that describes
> some of the rationale of the current handling of vlmax/nonvlmax
> operations.
>
> gcc/ChangeLog:
>
> * config/riscv/autovec.md: Use renamed functions.
> * config/riscv/riscv-protos.h (emit_vlmax_op): Rename.
> (emit_vlmax_reg_op): To this.
> (emit_nonvlmax_op): Rename.
> (emit_len_op): To this.
> (emit_nonvlmax_binop): Rename.
> (emit_len_binop): To this.
> * config/riscv/riscv-v.cc (emit_pred_op): Add default parameter.
> (emit_pred_binop): Remove vlmax_p.
> (emit_vlmax_op): Rename.
> (emit_vlmax_reg_op): To this.
> (emit_nonvlmax_op): Rename.
> (emit_len_op): To this.
> (emit_nonvlmax_binop): Rename.
> (emit_len_binop): To this.
> (sew64_scalar_helper): Use renamed functions.
> (expand_tuple_move): Use renamed functions.
> * config/riscv/riscv.cc (vector_zero_call_used_regs): Use
> renamed functions.
> * config/riscv/vector.md: Use renamed functions.
> ---
> gcc/config/riscv/autovec.md     | 26 ++++-----
> gcc/config/riscv/riscv-protos.h |  8 +--
> gcc/config/riscv/riscv-v.cc     | 94 ++++++++++++++++++++-------------
> gcc/config/riscv/riscv.cc       |  4 +-
> gcc/config/riscv/vector.md      | 12 +++--
> 5 files changed, 83 insertions(+), 61 deletions(-)
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index e249f4be704..58926ed3e67 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -31,8 +31,8 @@ (define_expand "len_load_<mode>"
>     (match_operand 3 "const_0_operand")]
>    "TARGET_VECTOR"
> {
> -  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode),
> operands[0],
> -   operands[1], operands[2], <VM>mode);
> +  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
> +      operands[1], operands[2], <VM>mode);
>    DONE;
> })
> @@ -43,8 +43,8 @@ (define_expand "len_store_<mode>"
>     (match_operand 3 "const_0_operand")]
>    "TARGET_VECTOR"
> {
> -  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (<MODE>mode),
> operands[0],
> -   operands[1], operands[2], <VM>mode);
> +  riscv_vector::emit_len_op (code_for_pred_mov (<MODE>mode), operands[0],
> +      operands[1], operands[2], <VM>mode);
>    DONE;
> })
> @@ -106,16 +106,16 @@ (define_expand "<optab><mode>3"
>      {
>        rtx cst;
>        gcc_assert (const_vec_duplicate_p(operands[2], &cst));
> -      riscv_vector::emit_nonvlmax_binop (code_for_pred_scalar
> - (<CODE>, <MODE>mode),
> - operands[0], operands[1], cst,
> - NULL, <VM>mode,
> - <VEL>mode);
> +      riscv_vector::emit_len_binop (code_for_pred_scalar
> +     (<CODE>, <MODE>mode),
> +     operands[0], operands[1], cst,
> +     NULL, <VM>mode,
> +     <VEL>mode);
>      }
>    else
> -    riscv_vector::emit_nonvlmax_binop (code_for_pred
> -        (<CODE>, <MODE>mode),
> -        operands[0], operands[1], operands[2],
> -        NULL, <VM>mode);
> +    riscv_vector::emit_len_binop (code_for_pred
> +   (<CODE>, <MODE>mode),
> +   operands[0], operands[1], operands[2],
> +   NULL, <VM>mode);
>    DONE;
> })
> diff --git a/gcc/config/riscv/riscv-protos.h
> b/gcc/config/riscv/riscv-protos.h
> index 4d0589e502c..55a43d6270e 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -167,10 +167,10 @@ bool legitimize_move (rtx, rtx, machine_mode);
> void emit_vlmax_vsetvl (machine_mode, rtx);
> void emit_hard_vlmax_vsetvl (machine_mode, rtx);
> void emit_vlmax_op (unsigned, rtx, rtx, machine_mode);
> -void emit_vlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
> -void emit_nonvlmax_op (unsigned, rtx, rtx, rtx, machine_mode);
> -void emit_nonvlmax_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
> -   machine_mode = VOIDmode);
> +void emit_vlmax_reg_op (unsigned, rtx, rtx, rtx, machine_mode);
> +void emit_len_op (unsigned, rtx, rtx, rtx, machine_mode);
> +void emit_len_binop (unsigned, rtx, rtx, rtx, rtx, machine_mode,
> +      machine_mode = VOIDmode);
> enum vlmul_type get_vlmul (machine_mode);
> unsigned int get_ratio (machine_mode);
> unsigned int get_nf (machine_mode);
> diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> index 8f46226d571..7cf5ec9f3ba 100644
> --- a/gcc/config/riscv/riscv-v.cc
> +++ b/gcc/config/riscv/riscv-v.cc
> @@ -99,27 +99,24 @@ public:
>      add_vundef_operand (dest_mode);
>    }
> -  void set_len_and_policy (rtx len, bool vlmax_p)
> +  void set_len_and_policy (rtx len, bool force_vlmax = false)
>      {
> +      bool vlmax_p = force_vlmax;
>        gcc_assert (has_dest);
> -      gcc_assert (len || vlmax_p);
> -      if (len)
> - add_input_operand (len, Pmode);
> -      else
> +      if (!len)
> {
> -   rtx vlmax = gen_reg_rtx (Pmode);
> -   emit_vlmax_vsetvl (dest_mode, vlmax);
> -   add_input_operand (vlmax, Pmode);
> +   vlmax_p = true;
> +   len = gen_reg_rtx (Pmode);
> +   emit_vlmax_vsetvl (dest_mode, len);
> }
> +      add_input_operand (len, Pmode);
> +
>        if (GET_MODE_CLASS (dest_mode) != MODE_VECTOR_BOOL)
> add_policy_operand (get_prefer_tail_policy (), get_prefer_mask_policy ());
> -      if (vlmax_p)
> - add_avl_type_operand (avl_type::VLMAX);
> -      else
> - add_avl_type_operand (avl_type::NONVLMAX);
> +      add_avl_type_operand (vlmax_p ? avl_type::VLMAX :
> avl_type::NONVLMAX);
>      }
>    void expand (enum insn_code icode, bool temporary_volatile_p = false)
> @@ -232,14 +229,14 @@ autovec_use_vlmax_p (void)
> /* Emit an RVV unmask && vl mov from SRC to DEST.  */
> static void
> emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx src, rtx len,
> -       machine_mode mask_mode, bool vlmax_p)
> +       machine_mode mask_mode, bool force_vlmax = false)
> {
>    insn_expander<8> e;
>    e.set_dest_and_mask (mask, dest, mask_mode);
>    e.add_input_operand (src, GET_MODE (src));
> -  e.set_len_and_policy (len, vlmax_p);
> +  e.set_len_and_policy (len, force_vlmax);
>    e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src));
> }
> @@ -248,8 +245,8 @@ emit_pred_op (unsigned icode, rtx mask, rtx dest, rtx
> src, rtx len,
>     specified using SCALAR_MODE.  */
> static void
> emit_pred_binop (unsigned icode, rtx mask, rtx dest, rtx src1, rtx src2,
> - rtx len, machine_mode mask_mode, machine_mode scalar_mode,
> - bool vlmax_p)
> + rtx len, machine_mode mask_mode,
> + machine_mode scalar_mode = VOIDmode)
> {
>    insn_expander<9> e;
>    e.set_dest_and_mask (mask, dest, mask_mode);
> @@ -267,37 +264,60 @@ emit_pred_binop (unsigned icode, rtx mask, rtx dest,
> rtx src1, rtx src2,
>    else
>      e.add_input_operand (src2, scalar_mode);
> -  e.set_len_and_policy (len, vlmax_p);
> +  e.set_len_and_policy (len);
>    e.expand ((enum insn_code) icode, MEM_P (dest) || MEM_P (src1) || MEM_P
> (src2));
> }
> +/* The RISC-V vsetvli pass uses "known vlmax" operations for optimization.
> +   Whether or not an instruction actually is a vlmax operation is not
> +   recognizable from the length operand alone but the avl_type operand
> +   is used instead.  In general, there are two cases:
> +
> +    - Emit a vlmax operation by passing a NULL length.  Here we emit
> +      a vsetvli with vlmax configuration and set the avl_type to VLMAX.
> +    - Emit an operation that uses the existing (last-set) length and
> +      set the avl_type to NONVLMAX.
> +
> +    Sometimes we also need to set the VLMAX avl_type to an operation that
> +    already uses a given length register.  This can happen during or after
> +    register allocation when we are not allowed to create a new register.
> +    For that case we also allow to set the avl_type to VLMAX.
> +*/
> +
> +/* This function emits a VLMAX vsetvli followed by the actual operation.
> */
> void
> emit_vlmax_op (unsigned icode, rtx dest, rtx src, machine_mode mask_mode)
> {
> -  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode, true);
> +  emit_pred_op (icode, NULL_RTX, dest, src, NULL_RTX, mask_mode);
> }
> +/* This function emits an operation with a given LEN that is determined
> +   by a previously emitted VLMAX vsetvli.  */
> void
> -emit_vlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
> -        machine_mode mask_mode)
> +emit_len_op (unsigned icode, rtx dest, rtx src, rtx len,
> +      machine_mode mask_mode)
> {
> -  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, true);
> +  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode);
> }
> +/* This function emits an operation with a given LEN that is known to be
> +   a preceding VLMAX.  It also sets the VLMAX flag which allows further
> +   optimization in the vsetvli pass.  */
> void
> -emit_nonvlmax_op (unsigned icode, rtx dest, rtx src, rtx len,
> -   machine_mode mask_mode)
> +emit_vlmax_reg_op (unsigned icode, rtx dest, rtx src, rtx len,
> +    machine_mode mask_mode)
> {
> -  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode, false);
> +  emit_pred_op (icode, NULL_RTX, dest, src, len, mask_mode,
> + /* Force VLMAX */ true);
> }
> void
> -emit_nonvlmax_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx
> len,
> -      machine_mode mask_mode, machine_mode scalar_mode)
> +emit_len_binop (unsigned icode, rtx dest, rtx src1, rtx src2, rtx len,
> + machine_mode mask_mode, machine_mode scalar_mode)
> {
> -  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len, mask_mode,
> -    scalar_mode, len ? false : true);
> +  emit_pred_binop (icode, NULL_RTX, dest, src1, src2, len,
> +    mask_mode, scalar_mode);
> }
> /* Emit vid.v instruction.  */
> @@ -345,14 +365,14 @@ expand_vec_series (rtx dest, rtx base, rtx step)
>   int shift = exact_log2 (INTVAL (step));
>   rtx shift_amount = gen_int_mode (shift, Pmode);
>   insn_code icode = code_for_pred_scalar (ASHIFT, mode);
> -   emit_nonvlmax_binop (icode, step_adj, vid, shift_amount,
> -        NULL, mask_mode, Pmode);
> +   emit_len_binop (icode, step_adj, vid, shift_amount,
> +   NULL, mask_mode, Pmode);
> }
>        else
> {
>   insn_code icode = code_for_pred_scalar (MULT, mode);
> -   emit_nonvlmax_binop (icode, step_adj, vid, step,
> -        NULL, mask_mode, inner_mode);
> +   emit_len_binop (icode, step_adj, vid, step,
> +   NULL, mask_mode, inner_mode);
> }
>      }
> @@ -367,7 +387,7 @@ expand_vec_series (rtx dest, rtx base, rtx step)
>      {
>        rtx result = gen_reg_rtx (mode);
>        insn_code icode = code_for_pred_scalar (PLUS, mode);
> -      emit_nonvlmax_binop (icode, result, step_adj, base,
> +      emit_len_binop (icode, result, step_adj, base,
>    NULL, mask_mode, inner_mode);
>        emit_move_insn (dest, result);
>      }
> @@ -739,8 +759,8 @@ sew64_scalar_helper (rtx *operands, rtx *scalar_op,
> rtx vl,
>      *scalar_op = force_reg (scalar_mode, *scalar_op);
>    rtx tmp = gen_reg_rtx (vector_mode);
> -  riscv_vector::emit_nonvlmax_op (code_for_pred_broadcast (vector_mode),
> tmp,
> -   *scalar_op, vl, mask_mode);
> +  riscv_vector::emit_len_op (code_for_pred_broadcast (vector_mode), tmp,
> +      *scalar_op, vl, mask_mode);
>    emit_vector_func (operands, tmp);
>    return true;
> @@ -1046,7 +1066,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
>       rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
>       if (fractional_p)
> - emit_vlmax_op (code_for_pred_mov (subpart_mode), subreg, mem,
> + emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), subreg, mem,
>        ops[4], mask_mode);
>       else
> emit_move_insn (subreg, mem);
> @@ -1068,7 +1088,7 @@ expand_tuple_move (machine_mode mask_mode, rtx *ops)
>       rtx mem = gen_rtx_MEM (subpart_mode, ops[3]);
>       if (fractional_p)
> - emit_vlmax_op (code_for_pred_mov (subpart_mode), mem, subreg,
> + emit_vlmax_reg_op (code_for_pred_mov (subpart_mode), mem, subreg,
>        ops[4], mask_mode);
>       else
> emit_move_insn (mem, subreg);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 02d581589ac..cecb4d49092 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7391,8 +7391,8 @@ vector_zero_call_used_regs (HARD_REG_SET
> need_zeroed_hardregs)
>       emitted_vlmax_vsetvl = true;
>     }
> -   riscv_vector::emit_vlmax_op (code_for_pred_mov (mode), target,
> -        CONST0_RTX (mode), vl, mask_mode);
> +   riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (mode), target,
> +    CONST0_RTX (mode), vl, mask_mode);
>   SET_HARD_REG_BIT (zeroed_hardregs, regno);
> }
> diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
> index 9ccc0d6a513..75479f27bcd 100644
> --- a/gcc/config/riscv/vector.md
> +++ b/gcc/config/riscv/vector.md
> @@ -721,8 +721,9 @@ (define_insn_and_split "*mov<V_FRACT:mode><P:mode>_lra"
>    else
>      {
>        riscv_vector::emit_vlmax_vsetvl (<V_FRACT:MODE>mode, operands[2]);
> -      riscv_vector::emit_vlmax_op (code_for_pred_mov (<V_FRACT:MODE>mode),
> -      operands[0], operands[1], operands[2], <VM>mode);
> +      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov
> (<V_FRACT:MODE>mode),
> +        operands[0], operands[1], operands[2],
> +        <VM>mode);
>      }
>    DONE;
> })
> @@ -741,8 +742,9 @@ (define_insn_and_split "*mov<VB:mode><P:mode>_lra"
>    else
>      {
>        riscv_vector::emit_vlmax_vsetvl (<VB:MODE>mode, operands[2]);
> -      riscv_vector::emit_vlmax_op (code_for_pred_mov (<VB:MODE>mode),
> -      operands[0], operands[1], operands[2], <VB:MODE>mode);
> +      riscv_vector::emit_vlmax_reg_op (code_for_pred_mov (<VB:MODE>mode),
> +        operands[0], operands[1], operands[2],
> +        <VB:MODE>mode);
>      }
>    DONE;
> })
> @@ -806,7 +808,7 @@ (define_expand "vec_duplicate<mode>"
>    "TARGET_VECTOR"
>    {
>      riscv_vector::emit_vlmax_op (code_for_pred_broadcast (<MODE>mode),
> - operands[0], operands[1], <VM>mode);
> + operands[0], operands[1], <VM>mode);
>      DONE;
>    }
> )
> --
> 2.40.0
>
>
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-05-11 11:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 15:24 [PATCH] riscv: Clarify vlmax and length handling Robin Dapp
2023-05-10 18:32 ` Palmer Dabbelt
2023-05-10 18:50   ` Robin Dapp
2023-05-10 18:53     ` Palmer Dabbelt
2023-05-11  2:26       ` Kito Cheng
2023-05-11 10:29         ` [PATCH v2] RISC-V: " Robin Dapp
2023-05-11 10:35           ` juzhe.zhong
2023-05-11 11:09             ` Kito Cheng
2023-05-11  2:30 ` [PATCH] riscv: " juzhe.zhong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).