Re: [PATCH] riscv: Clarify vlmax and length handling.
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_op
Re: [PATCH] riscv: Clarify vlmax and length handling.
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.
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.
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.
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.
[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); }