Re: [PATCH] riscv: Clarify vlmax and length handling.

2023-05-10 Thread juzhe.zh...@rivai.ai
This part LGTM.



juzhe.zh...@rivai.ai
 
From: Robin Dapp
Date: 2023-05-10 23:24
To: gcc-patches; juzhe.zh...@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_"
(match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
{
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (mode), operands[0],
-   operands[1], operands[2], mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (mode), operands[0],
+  operands[1], operands[2], mode);
   DONE;
})
@@ -43,8 +43,8 @@ (define_expand "len_store_"
(match_operand 3 "const_0_operand")]
   "TARGET_VECTOR"
{
-  riscv_vector::emit_nonvlmax_op (code_for_pred_mov (mode), operands[0],
-   operands[1], operands[2], mode);
+  riscv_vector::emit_len_op (code_for_pred_mov (mode), operands[0],
+  operands[1], operands[2], mode);
   DONE;
})
@@ -79,15 +79,15 @@ (define_expand "3"
   if (inner == E_QImode || inner == E_HImode || inner == E_SImode)
op2mode = inner;
-  riscv_vector::emit_nonvlmax_binop (code_for_pred_scalar
- (, mode),
- operands[0], operands[1], cst,
- NULL_RTX, mode, op2mode);
+  riscv_vector::emit_len_binop (code_for_pred_scalar
+ (, mode),
+ operands[0], operands[1], cst,
+ NULL_RTX, mode, op2mode);
 }
   else
-riscv_vector::emit_nonvlmax_binop (code_for_pred
-(, mode),
-operands[0], operands[1], operands[2],
-NULL_RTX, mode);
+riscv_vector::emit_len_binop (code_for_pred
+   (, mode),
+   operands[0], operands[1], operands[2],
+   NULL_RTX, 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 

Re: [PATCH] riscv: Clarify vlmax and length handling.

2023-05-10 Thread Kito Cheng via Gcc-patches
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  wrote:
>
> On Wed, 10 May 2023 11:50:32 PDT (-0700), rdapp@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.


Re: [PATCH] riscv: Clarify vlmax and length handling.

2023-05-10 Thread Palmer Dabbelt

On Wed, 10 May 2023 11:50:32 PDT (-0700), rdapp@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.


Re: [PATCH] riscv: Clarify vlmax and length handling.

2023-05-10 Thread Robin Dapp via Gcc-patches
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).


Re: [PATCH] riscv: Clarify vlmax and length handling.

2023-05-10 Thread Palmer Dabbelt

On Wed, 10 May 2023 08:24:40 PDT (-0700), rdapp@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.