Re: Re: [PATCH] riscv: Add vectorized binops and insn_expander helpers.

2023-05-10 Thread 钟居哲
>> This I added in order to match the scalar variants like
 
 >>  [(set (match_operand:VI_QHS 0 "register_operand"  "=vd,vd, vr, vr")
>> (if_then_else:VI_QHS
>>   (unspec:
>> [(match_operand: 1 "vector_mask_operand" "vm,vm,Wc1,Wc1")
 >> (match_operand 5 "vector_length_operand""rK,rK, rK, rK")
>>  (match_operand 6 "const_int_operand"" i, i,  i,  i")
>>  (match_operand 7 "const_int_operand"" i, i,  i,  i")
 >> (match_operand 8 "const_int_operand"" i, i,  i,  i")
 >> (reg:SI VL_REGNUM)
 >> (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
 >>  (any_commutative_binop:VI_QHS
 >>(vec_duplicate:VI_QHS
 >>  (match_operand: 4 "reg_or_0_operand"  "rJ,rJ, rJ, rJ"))
 
>> Any other way to get there?

No, you don't need to care about that. 
Intrinsic patterns are well designed, you just use "GET_MODE_INNER" which can
well handle that.

 >> Hmm I see, the VOIDmode being abused as default might be confusing here.
 >> Would an additional parameter like "bool set_op2_mode" make it clearer?
 >> Another option is to separate this into another function altogether like
 >> emit_len_binop_scalar or so.

No, you just use op2mode which you pass through.




juzhe.zh...@rivai.ai
 
From: Robin Dapp
Date: 2023-05-11 02:02
To: 钟居哲; gcc-patches; kito.cheng; Michael Collison; palmer; Jeff Law
Subject: Re: [PATCH] riscv: Add vectorized binops and insn_expander helpers.
> +  machine_mode op2mode = Pmode;
> +  if (inner == E_QImode || inner == E_HImode || inner == E_SImode)
> + op2mode = inner;
 
This I added in order to match the scalar variants like
 
  [(set (match_operand:VI_QHS 0 "register_operand"  "=vd,vd, vr, vr")
(if_then_else:VI_QHS
  (unspec:
[(match_operand: 1 "vector_mask_operand" "vm,vm,Wc1,Wc1")
 (match_operand 5 "vector_length_operand""rK,rK, rK, rK")
 (match_operand 6 "const_int_operand"" i, i,  i,  i")
 (match_operand 7 "const_int_operand"" i, i,  i,  i")
 (match_operand 8 "const_int_operand"" i, i,  i,  i")
 (reg:SI VL_REGNUM)
 (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
  (any_commutative_binop:VI_QHS
(vec_duplicate:VI_QHS
  (match_operand: 4 "reg_or_0_operand"  "rJ,rJ, rJ, rJ"))
 
Any other way to get there?
 
> + e.add_input_operand (src2, op2mode == VOIDmode ? GET_MODE (src2) : op2mode);
> Very confusing here.
 
Hmm I see, the VOIDmode being abused as default might be confusing here.
Would an additional parameter like "bool set_op2_mode" make it clearer?
Another option is to separate this into another function altogether like
emit_len_binop_scalar or so.
 
> +  
> change it into 
 
Done and removed the rest.
 
Thanks.
 


Re: [PATCH] riscv: Add vectorized binops and insn_expander helpers.

2023-05-10 Thread Robin Dapp via Gcc-patches
> +  machine_mode op2mode = Pmode;
> +  if (inner == E_QImode || inner == E_HImode || inner == E_SImode)
> + op2mode = inner;

This I added in order to match the scalar variants like

  [(set (match_operand:VI_QHS 0 "register_operand"  "=vd,vd, vr, vr")
(if_then_else:VI_QHS
  (unspec:
[(match_operand: 1 "vector_mask_operand" "vm,vm,Wc1,Wc1")
 (match_operand 5 "vector_length_operand""rK,rK, rK, rK")
 (match_operand 6 "const_int_operand"" i, i,  i,  i")
 (match_operand 7 "const_int_operand"" i, i,  i,  i")
 (match_operand 8 "const_int_operand"" i, i,  i,  i")
 (reg:SI VL_REGNUM)
 (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
  (any_commutative_binop:VI_QHS
(vec_duplicate:VI_QHS
  (match_operand: 4 "reg_or_0_operand"  "rJ,rJ, rJ, rJ"))

Any other way to get there?

> + e.add_input_operand (src2, op2mode == VOIDmode ? GET_MODE (src2) : op2mode);
> Very confusing here.

Hmm I see, the VOIDmode being abused as default might be confusing here.
Would an additional parameter like "bool set_op2_mode" make it clearer?
Another option is to separate this into another function altogether like
emit_len_binop_scalar or so.

> +  
> change it into 

Done and removed the rest.

Thanks.


Re: [PATCH] riscv: Add vectorized binops and insn_expander helpers.

2023-05-10 Thread 钟居哲
Thanks Robin.

A couple comments here:
+  machine_mode op2mode = Pmode;
+  if (inner == E_QImode || inner == E_HImode || inner == E_SImode)
+ op2mode = inner;

Remove it.

+  
change it into 

+ e.add_input_operand (src2, op2mode == VOIDmode ? GET_MODE (src2) : op2mode);
Very confusing here.

+(define_code_attr BINOP_TO_UPPERCASE [
+(plus "PLUS")
+(minus "MINUS")
+(and "AND")
+(ior "IOR")
+(xor "XOR")
+(ashift "ASHIFT")
+(ashiftrt "ASHIFTRT")
+(lshiftrt "LSHIFTRT")
+(smax "SMAX")
+(umax "UMAX")
+(smin "SMIN")
+(umin "UMIN")
+(mult "MULT")
+(div "DIV")
+(udiv "UDIV")
+(mod "MOD")
+(umod "UMOD")
+])

Remove it.

Thanks.


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: Add vectorized binops and insn_expander helpers.
Hi,
 
this patch adds basic binary integer operations support.  It is based
on Michael Collison's work and makes use of the existing helpers in
riscv-c.cc.  It introduces emit_nonvlmax_binop which, in turn, uses
emit_pred_binop.  Setting the destination as well as the mask and the
length is factored out into separate functions.
 
There are several things still missing, most notably the scalar variants
(.vx) as well as multiplication variants and more.
 
Bootstrapped and regtested.
 
Regards
Robin
 
--
 
gcc/ChangeLog:
 
* config/riscv/autovec.md (3): Add integer binops.
* config/riscv/riscv-protos.h (emit_nonvlmax_binop): Declare.
* config/riscv/riscv-v.cc (emit_pred_op): New function.
(set_expander_dest_and_mask): New function.
(emit_pred_binop): New function.
(emit_nonvlmax_binop): New function.
* config/riscv/vector-iterators.md: Add new code attribute.
---
gcc/config/riscv/autovec.md  | 33 ++
gcc/config/riscv/riscv-protos.h  |  2 +
gcc/config/riscv/riscv-v.cc  | 98 ++--
gcc/config/riscv/vector-iterators.md | 22 +++
4 files changed, 136 insertions(+), 19 deletions(-)
 
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index f1c5ff5951b..15f8d007e07 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -58,3 +58,36 @@ (define_expand "movmisalign"
 DONE;
   }
)
+
+;; =
+;; == Binary integer operations
+;; =
+
+(define_expand "3"
+  [(set (match_operand:VI 0 "register_operand")
+(any_int_binop:VI
+ (match_operand:VI 1 "")
+ (match_operand:VI 2 "")))]
+  "TARGET_VECTOR"
+{
+  if (!register_operand (operands[2], mode))
+{
+  rtx cst;
+  gcc_assert (const_vec_duplicate_p(operands[2], &cst));
+  machine_mode inner = mode;
+  machine_mode op2mode = Pmode;
+  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);
+}
+  else
+riscv_vector::emit_nonvlmax_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 c0293a306f9..75cdb90b9c9 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -169,6 +169,8 @@ 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);
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 7ca49ca67c1..3c43dfc5eea 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -53,7 +53,7 @@ namespace riscv_vector {
template  class insn_expander
{
public:
-  insn_expander () : m_opno (0) {}
+  insn_expander () : m_opno (0), has_dest(false) {}
   void add_output_operand (rtx x, machine_mode mode)
   {
 create_output_operand (&m_ops[m_opno++], x, mode);
@@ -84,6 +84,44 @@ public:
 add_input_operand (gen_int_mode (type, Pmode), Pmode);
   }
+  void set_dest_and_mask (rtx mask, rtx dest, machine_mode mask_mode)
+  {
+dest_mode = GET_MODE (dest);
+has_dest = true;
+
+add_output_operand (dest, dest_mode);
+
+if (mask)
+  add_input_operand (mask, GET_MODE (mask));
+else
+  add_all_one_mask_operand (mask_mode);
+
+add_vundef_operand (dest_mode);
+  }
+
+  void set_len_and_policy (rtx len, bool vlmax_p)
+{
+  gcc_assert (has_dest);
+  gcc_assert (len || vlmax_p);
+
+