public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
@ 2023-07-31  6:52 Kito Cheng
  2023-07-31  6:58 ` juzhe.zhong
  0 siblings, 1 reply; 11+ messages in thread
From: Kito Cheng @ 2023-07-31  6:52 UTC (permalink / raw)
  To: gcc-patches, kito.cheng, juzhe.zhong, rdapp.gcc, pan2.li; +Cc: Kito Cheng

We always want get_mask_mode return a valid mode, it's something wrong
if it failed, so I think we could just move the `.require ()` into
get_mask_mode, instead of calling that every call-site.

The only exception is riscv_get_mask_mode, it might put supported mode
into get_mask_mode, so added a check with riscv_v_ext_mode_p to make
sure only valid vector mode will ask get_mask_mode.

gcc/ChangeLog:

	* config/riscv/autovec.md (abs<mode>2): Remove `.require ()`.
	* config/riscv/riscv-protos.h (get_mask_mode): Update return
	type.
	* config/riscv/riscv-v.cc (rvv_builder::rvv_builder): Remove
	`.require ()`.
	(emit_vlmax_insn): Ditto.
	(emit_vlmax_fp_insn): Ditto.
	(emit_vlmax_ternary_insn): Ditto.
	(emit_vlmax_fp_ternary_insn): Ditto.
	(emit_nonvlmax_fp_ternary_tu_insn): Ditto.
	(emit_nonvlmax_insn): Ditto.
	(emit_vlmax_slide_insn): Ditto.
	(emit_nonvlmax_slide_tu_insn): Ditto.
	(emit_vlmax_merge_insn): Ditto.
	(emit_vlmax_masked_insn): Ditto.
	(emit_nonvlmax_masked_insn): Ditto.
	(emit_vlmax_masked_store_insn): Ditto.
	(emit_nonvlmax_masked_store_insn): Ditto.
	(emit_vlmax_masked_mu_insn): Ditto.
	(emit_nonvlmax_tu_insn): Ditto.
	(emit_nonvlmax_fp_tu_insn): Ditto.
	(emit_scalar_move_insn): Ditto.
	(emit_vlmax_compress_insn): Ditto.
	(emit_vlmax_reduction_insn): Ditto.
	(emit_vlmax_fp_reduction_insn): Ditto.
	(emit_nonvlmax_fp_reduction_insn): Ditto.
	(expand_vec_series): Ditto.
	(expand_vector_init_merge_repeating_sequence): Ditto.
	(expand_vec_perm): Ditto.
	(shuffle_merge_patterns): Ditto.
	(shuffle_compress_patterns): Ditto.
	(shuffle_decompress_patterns): Ditto.
	(expand_reduction): Ditto.
	(get_mask_mode): Update return type.
	* config/riscv/riscv.cc (riscv_get_mask_mode): Check vector type
	is valid, and use new get_mask_mode interface.
---
 gcc/config/riscv/autovec.md     |  2 +-
 gcc/config/riscv/riscv-protos.h |  2 +-
 gcc/config/riscv/riscv-v.cc     | 68 ++++++++++++++++-----------------
 gcc/config/riscv/riscv.cc       |  5 +--
 4 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index b7ea3101f5a..fd7ec911e87 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -912,7 +912,7 @@ (define_expand "abs<mode>2"
   "TARGET_VECTOR"
 {
   rtx zero = gen_const_vec_duplicate (<MODE>mode, GEN_INT (0));
-  machine_mode mask_mode = riscv_vector::get_mask_mode (<MODE>mode).require ();
+  machine_mode mask_mode = riscv_vector::get_mask_mode (<MODE>mode);
   rtx mask = gen_reg_rtx (mask_mode);
   riscv_vector::expand_vec_cmp (mask, LT, operands[1], zero);
 
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 7d8bf2b81f8..324991e2619 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -312,7 +312,7 @@ bool slide1_sew64_helper (int, machine_mode, machine_mode,
 rtx gen_avl_for_scalar_move (rtx);
 void expand_tuple_move (rtx *);
 machine_mode preferred_simd_mode (scalar_mode);
-opt_machine_mode get_mask_mode (machine_mode);
+machine_mode get_mask_mode (machine_mode);
 void expand_vec_series (rtx, rtx, rtx);
 void expand_vec_init (rtx, rtx);
 void expand_vec_perm (rtx, rtx, rtx, rtx);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 0a355eb3c7a..5ab749f4343 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -285,7 +285,7 @@ public:
     m_inner_mode = GET_MODE_INNER (mode);
     m_inner_bits_size = GET_MODE_BITSIZE (m_inner_mode);
     m_inner_bytes_size = GET_MODE_SIZE (m_inner_mode);
-    m_mask_mode = get_mask_mode (mode).require ();
+    m_mask_mode = get_mask_mode (mode);
 
     gcc_assert (
       int_mode_for_size (inner_bits_size (), 0).exists (&m_inner_int_mode));
@@ -676,7 +676,7 @@ void
 emit_vlmax_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -698,7 +698,7 @@ void
 emit_vlmax_fp_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -721,7 +721,7 @@ void
 emit_vlmax_ternary_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ true,
@@ -742,7 +742,7 @@ void
 emit_vlmax_fp_ternary_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ true,
@@ -764,7 +764,7 @@ static void
 emit_nonvlmax_fp_ternary_tu_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -786,7 +786,7 @@ void
 emit_nonvlmax_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -808,7 +808,7 @@ void
 emit_vlmax_slide_insn (unsigned icode, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SLIDE_OP,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -830,7 +830,7 @@ void
 emit_nonvlmax_slide_tu_insn (unsigned icode, rtx *ops, rtx avl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SLIDE_OP,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -853,7 +853,7 @@ void
 emit_vlmax_merge_insn (unsigned icode, int op_num, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ false,
@@ -908,7 +908,7 @@ static void
 emit_vlmax_masked_insn (unsigned icode, int op_num, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -926,7 +926,7 @@ static void
 emit_nonvlmax_masked_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -945,7 +945,7 @@ static void
 emit_vlmax_masked_store_insn (unsigned icode, int op_num, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ false,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -961,7 +961,7 @@ static void
 emit_nonvlmax_masked_store_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ false,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -978,7 +978,7 @@ void
 emit_vlmax_masked_mu_insn (unsigned icode, int op_num, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -996,7 +996,7 @@ static void
 emit_nonvlmax_tu_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -1015,7 +1015,7 @@ static void
 emit_nonvlmax_fp_tu_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
 					  /*HAS_DEST_P*/ true,
 					  /*FULLY_UNMASKED_P*/ false,
@@ -1036,7 +1036,7 @@ void
 emit_scalar_move_insn (unsigned icode, rtx *ops, rtx len)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SCALAR_MOV_OP,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ false,
@@ -1156,7 +1156,7 @@ static void
 emit_vlmax_compress_insn (unsigned icode, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_COMPRESS_OP,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ false,
@@ -1174,7 +1174,7 @@ static void
 emit_vlmax_reduction_insn (unsigned icode, int op_num, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
+  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -1192,7 +1192,7 @@ static void
 emit_vlmax_fp_reduction_insn (unsigned icode, int op_num, rtx *ops)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
+  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ true,
@@ -1211,7 +1211,7 @@ static void
 emit_nonvlmax_fp_reduction_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
 {
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
+  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
 					  /* HAS_DEST_P */ true,
 					  /* FULLY_UNMASKED_P */ false,
@@ -1248,8 +1248,7 @@ void
 expand_vec_series (rtx dest, rtx base, rtx step)
 {
   machine_mode mode = GET_MODE (dest);
-  machine_mode mask_mode;
-  gcc_assert (get_mask_mode (mode).exists (&mask_mode));
+  machine_mode mask_mode = get_mask_mode (mode);
   poly_int64 nunits_m1 = GET_MODE_NUNITS (mode) - 1;
   poly_int64 value;
 
@@ -1789,10 +1788,10 @@ get_avl_type_rtx (enum avl_type type)
 
 /* Return the appropriate mask mode for MODE.  */
 
-opt_machine_mode
+machine_mode
 get_mask_mode (machine_mode mode)
 {
-  return get_vector_mode (BImode, GET_MODE_NUNITS (mode));
+  return get_vector_mode (BImode, GET_MODE_NUNITS (mode)).require();
 }
 
 /* Return the appropriate M1 mode for MODE.  */
@@ -2318,8 +2317,7 @@ expand_vector_init_insert_elems (rtx target, const rvv_builder &builder,
 				 int nelts_reqd)
 {
   machine_mode mode = GET_MODE (target);
-  machine_mode mask_mode;
-  gcc_assert (get_mask_mode (mode).exists (&mask_mode));
+  machine_mode mask_mode = get_mask_mode (mode);
   rtx dup = expand_vector_broadcast (mode, builder.elt (0));
   emit_move_insn (target, dup);
   int ndups = builder.count_dups (0, nelts_reqd - 1, 1);
@@ -2345,8 +2343,8 @@ expand_vector_init_merge_repeating_sequence (rtx target,
 					     const rvv_builder &builder)
 {
   machine_mode dup_mode = get_repeating_sequence_dup_machine_mode (builder);
-  machine_mode dup_mask_mode = get_mask_mode (dup_mode).require ();
-  machine_mode mask_mode = get_mask_mode (builder.mode ()).require ();
+  machine_mode dup_mask_mode = get_mask_mode (dup_mode);
+  machine_mode mask_mode = get_mask_mode (builder.mode ());
   uint64_t full_nelts = builder.full_nelts ().to_constant ();
 
   /* Step 1: Broadcast the first pattern.  */
@@ -2796,7 +2794,7 @@ expand_vec_perm (rtx target, rtx op0, rtx op1, rtx sel)
      __builtin_shufflevector (vec1, vec2, index...), the index can be any
      value in range of [0, 2 * nunits - 1].  */
   machine_mode mask_mode;
-  mask_mode = get_mask_mode (data_mode).require ();
+  mask_mode = get_mask_mode (data_mode);
   rtx mask = gen_reg_rtx (mask_mode);
   max_sel = gen_const_vector_dup (sel_mode, nunits);
 
@@ -2868,7 +2866,7 @@ shuffle_merge_patterns (struct expand_vec_perm_d *d)
   if (d->testing_p)
     return true;
 
-  machine_mode mask_mode = get_mask_mode (vmode).require ();
+  machine_mode mask_mode = get_mask_mode (vmode);
   rtx mask = gen_reg_rtx (mask_mode);
 
   rtx sel = vec_perm_indices_to_rtx (sel_mode, d->perm);
@@ -2988,7 +2986,7 @@ shuffle_compress_patterns (struct expand_vec_perm_d *d)
     return false;
 
   /* Build a mask that is true when selector element is true.  */
-  machine_mode mask_mode = get_mask_mode (vmode).require ();
+  machine_mode mask_mode = get_mask_mode (vmode);
   rvv_builder builder (mask_mode, vlen, 1);
   for (int i = 0; i < vlen; i++)
     {
@@ -3040,7 +3038,7 @@ static bool
 shuffle_decompress_patterns (struct expand_vec_perm_d *d)
 {
   poly_uint64 nelt = d->perm.length ();
-  machine_mode mask_mode = get_mask_mode (d->vmode).require ();
+  machine_mode mask_mode = get_mask_mode (d->vmode);
 
   /* For constant size indices, we dont't need to handle it here.
      Just leave it to vec_perm<mode>.  */
@@ -3498,7 +3496,7 @@ expand_reduction (rtx_code code, rtx *ops, rtx init, reduction_type type)
   rtx vector = type == reduction_type::UNORDERED ? ops[1] : ops[2];
   machine_mode vmode = GET_MODE (vector);
   machine_mode m1_mode = get_m1_mode (vmode).require ();
-  machine_mode m1_mmode = get_mask_mode (m1_mode).require ();
+  machine_mode m1_mmode = get_mask_mode (m1_mode);
 
   rtx m1_tmp = gen_reg_rtx (m1_mode);
   rtx m1_mask = gen_scalar_move_mask (m1_mmode);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dc78f4cf977..1eacca06b5c 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7541,9 +7541,8 @@ riscv_support_vector_misalignment (machine_mode mode,
 static opt_machine_mode
 riscv_get_mask_mode (machine_mode mode)
 {
-  machine_mode mask_mode = VOIDmode;
-  if (TARGET_VECTOR && riscv_vector::get_mask_mode (mode).exists (&mask_mode))
-    return mask_mode;
+  if (TARGET_VECTOR && riscv_v_ext_mode_p (mode))
+    return riscv_vector::get_mask_mode (mode);
 
   return default_get_mask_mode (mode);
 }
-- 
2.40.1


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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31  6:52 [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC Kito Cheng
@ 2023-07-31  6:58 ` juzhe.zhong
  2023-07-31  7:02   ` Kito Cheng
  0 siblings, 1 reply; 11+ messages in thread
From: juzhe.zhong @ 2023-07-31  6:58 UTC (permalink / raw)
  To: Kito.cheng, gcc-patches, kito.cheng, Robin Dapp, pan2.li; +Cc: Kito.cheng

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

ok



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-07-31 14:52
To: gcc-patches; kito.cheng; juzhe.zhong; rdapp.gcc; pan2.li
CC: Kito Cheng
Subject: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
We always want get_mask_mode return a valid mode, it's something wrong
if it failed, so I think we could just move the `.require ()` into
get_mask_mode, instead of calling that every call-site.
 
The only exception is riscv_get_mask_mode, it might put supported mode
into get_mask_mode, so added a check with riscv_v_ext_mode_p to make
sure only valid vector mode will ask get_mask_mode.
 
gcc/ChangeLog:
 
* config/riscv/autovec.md (abs<mode>2): Remove `.require ()`.
* config/riscv/riscv-protos.h (get_mask_mode): Update return
type.
* config/riscv/riscv-v.cc (rvv_builder::rvv_builder): Remove
`.require ()`.
(emit_vlmax_insn): Ditto.
(emit_vlmax_fp_insn): Ditto.
(emit_vlmax_ternary_insn): Ditto.
(emit_vlmax_fp_ternary_insn): Ditto.
(emit_nonvlmax_fp_ternary_tu_insn): Ditto.
(emit_nonvlmax_insn): Ditto.
(emit_vlmax_slide_insn): Ditto.
(emit_nonvlmax_slide_tu_insn): Ditto.
(emit_vlmax_merge_insn): Ditto.
(emit_vlmax_masked_insn): Ditto.
(emit_nonvlmax_masked_insn): Ditto.
(emit_vlmax_masked_store_insn): Ditto.
(emit_nonvlmax_masked_store_insn): Ditto.
(emit_vlmax_masked_mu_insn): Ditto.
(emit_nonvlmax_tu_insn): Ditto.
(emit_nonvlmax_fp_tu_insn): Ditto.
(emit_scalar_move_insn): Ditto.
(emit_vlmax_compress_insn): Ditto.
(emit_vlmax_reduction_insn): Ditto.
(emit_vlmax_fp_reduction_insn): Ditto.
(emit_nonvlmax_fp_reduction_insn): Ditto.
(expand_vec_series): Ditto.
(expand_vector_init_merge_repeating_sequence): Ditto.
(expand_vec_perm): Ditto.
(shuffle_merge_patterns): Ditto.
(shuffle_compress_patterns): Ditto.
(shuffle_decompress_patterns): Ditto.
(expand_reduction): Ditto.
(get_mask_mode): Update return type.
* config/riscv/riscv.cc (riscv_get_mask_mode): Check vector type
is valid, and use new get_mask_mode interface.
---
gcc/config/riscv/autovec.md     |  2 +-
gcc/config/riscv/riscv-protos.h |  2 +-
gcc/config/riscv/riscv-v.cc     | 68 ++++++++++++++++-----------------
gcc/config/riscv/riscv.cc       |  5 +--
4 files changed, 37 insertions(+), 40 deletions(-)
 
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index b7ea3101f5a..fd7ec911e87 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -912,7 +912,7 @@ (define_expand "abs<mode>2"
   "TARGET_VECTOR"
{
   rtx zero = gen_const_vec_duplicate (<MODE>mode, GEN_INT (0));
-  machine_mode mask_mode = riscv_vector::get_mask_mode (<MODE>mode).require ();
+  machine_mode mask_mode = riscv_vector::get_mask_mode (<MODE>mode);
   rtx mask = gen_reg_rtx (mask_mode);
   riscv_vector::expand_vec_cmp (mask, LT, operands[1], zero);
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 7d8bf2b81f8..324991e2619 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -312,7 +312,7 @@ bool slide1_sew64_helper (int, machine_mode, machine_mode,
rtx gen_avl_for_scalar_move (rtx);
void expand_tuple_move (rtx *);
machine_mode preferred_simd_mode (scalar_mode);
-opt_machine_mode get_mask_mode (machine_mode);
+machine_mode get_mask_mode (machine_mode);
void expand_vec_series (rtx, rtx, rtx);
void expand_vec_init (rtx, rtx);
void expand_vec_perm (rtx, rtx, rtx, rtx);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 0a355eb3c7a..5ab749f4343 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -285,7 +285,7 @@ public:
     m_inner_mode = GET_MODE_INNER (mode);
     m_inner_bits_size = GET_MODE_BITSIZE (m_inner_mode);
     m_inner_bytes_size = GET_MODE_SIZE (m_inner_mode);
-    m_mask_mode = get_mask_mode (mode).require ();
+    m_mask_mode = get_mask_mode (mode);
     gcc_assert (
       int_mode_for_size (inner_bits_size (), 0).exists (&m_inner_int_mode));
@@ -676,7 +676,7 @@ void
emit_vlmax_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -698,7 +698,7 @@ void
emit_vlmax_fp_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -721,7 +721,7 @@ void
emit_vlmax_ternary_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ true,
@@ -742,7 +742,7 @@ void
emit_vlmax_fp_ternary_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ true,
@@ -764,7 +764,7 @@ static void
emit_nonvlmax_fp_ternary_tu_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ false,
@@ -786,7 +786,7 @@ void
emit_nonvlmax_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -808,7 +808,7 @@ void
emit_vlmax_slide_insn (unsigned icode, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SLIDE_OP,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -830,7 +830,7 @@ void
emit_nonvlmax_slide_tu_insn (unsigned icode, rtx *ops, rtx avl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SLIDE_OP,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -853,7 +853,7 @@ void
emit_vlmax_merge_insn (unsigned icode, int op_num, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ false,
@@ -908,7 +908,7 @@ static void
emit_vlmax_masked_insn (unsigned icode, int op_num, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ false,
@@ -926,7 +926,7 @@ static void
emit_nonvlmax_masked_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ false,
@@ -945,7 +945,7 @@ static void
emit_vlmax_masked_store_insn (unsigned icode, int op_num, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ false,
  /*FULLY_UNMASKED_P*/ false,
@@ -961,7 +961,7 @@ static void
emit_nonvlmax_masked_store_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ false,
  /*FULLY_UNMASKED_P*/ false,
@@ -978,7 +978,7 @@ void
emit_vlmax_masked_mu_insn (unsigned icode, int op_num, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ false,
@@ -996,7 +996,7 @@ static void
emit_nonvlmax_tu_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ false,
@@ -1015,7 +1015,7 @@ static void
emit_nonvlmax_fp_tu_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
  /*HAS_DEST_P*/ true,
  /*FULLY_UNMASKED_P*/ false,
@@ -1036,7 +1036,7 @@ void
emit_scalar_move_insn (unsigned icode, rtx *ops, rtx len)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SCALAR_MOV_OP,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ false,
@@ -1156,7 +1156,7 @@ static void
emit_vlmax_compress_insn (unsigned icode, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
+  machine_mode mask_mode = get_mask_mode (dest_mode);
   insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_COMPRESS_OP,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ false,
@@ -1174,7 +1174,7 @@ static void
emit_vlmax_reduction_insn (unsigned icode, int op_num, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
+  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -1192,7 +1192,7 @@ static void
emit_vlmax_fp_reduction_insn (unsigned icode, int op_num, rtx *ops)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
+  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ true,
@@ -1211,7 +1211,7 @@ static void
emit_nonvlmax_fp_reduction_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
{
   machine_mode dest_mode = GET_MODE (ops[0]);
-  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
+  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
   insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
  /* HAS_DEST_P */ true,
  /* FULLY_UNMASKED_P */ false,
@@ -1248,8 +1248,7 @@ void
expand_vec_series (rtx dest, rtx base, rtx step)
{
   machine_mode mode = GET_MODE (dest);
-  machine_mode mask_mode;
-  gcc_assert (get_mask_mode (mode).exists (&mask_mode));
+  machine_mode mask_mode = get_mask_mode (mode);
   poly_int64 nunits_m1 = GET_MODE_NUNITS (mode) - 1;
   poly_int64 value;
@@ -1789,10 +1788,10 @@ get_avl_type_rtx (enum avl_type type)
/* Return the appropriate mask mode for MODE.  */
-opt_machine_mode
+machine_mode
get_mask_mode (machine_mode mode)
{
-  return get_vector_mode (BImode, GET_MODE_NUNITS (mode));
+  return get_vector_mode (BImode, GET_MODE_NUNITS (mode)).require();
}
/* Return the appropriate M1 mode for MODE.  */
@@ -2318,8 +2317,7 @@ expand_vector_init_insert_elems (rtx target, const rvv_builder &builder,
int nelts_reqd)
{
   machine_mode mode = GET_MODE (target);
-  machine_mode mask_mode;
-  gcc_assert (get_mask_mode (mode).exists (&mask_mode));
+  machine_mode mask_mode = get_mask_mode (mode);
   rtx dup = expand_vector_broadcast (mode, builder.elt (0));
   emit_move_insn (target, dup);
   int ndups = builder.count_dups (0, nelts_reqd - 1, 1);
@@ -2345,8 +2343,8 @@ expand_vector_init_merge_repeating_sequence (rtx target,
     const rvv_builder &builder)
{
   machine_mode dup_mode = get_repeating_sequence_dup_machine_mode (builder);
-  machine_mode dup_mask_mode = get_mask_mode (dup_mode).require ();
-  machine_mode mask_mode = get_mask_mode (builder.mode ()).require ();
+  machine_mode dup_mask_mode = get_mask_mode (dup_mode);
+  machine_mode mask_mode = get_mask_mode (builder.mode ());
   uint64_t full_nelts = builder.full_nelts ().to_constant ();
   /* Step 1: Broadcast the first pattern.  */
@@ -2796,7 +2794,7 @@ expand_vec_perm (rtx target, rtx op0, rtx op1, rtx sel)
      __builtin_shufflevector (vec1, vec2, index...), the index can be any
      value in range of [0, 2 * nunits - 1].  */
   machine_mode mask_mode;
-  mask_mode = get_mask_mode (data_mode).require ();
+  mask_mode = get_mask_mode (data_mode);
   rtx mask = gen_reg_rtx (mask_mode);
   max_sel = gen_const_vector_dup (sel_mode, nunits);
@@ -2868,7 +2866,7 @@ shuffle_merge_patterns (struct expand_vec_perm_d *d)
   if (d->testing_p)
     return true;
-  machine_mode mask_mode = get_mask_mode (vmode).require ();
+  machine_mode mask_mode = get_mask_mode (vmode);
   rtx mask = gen_reg_rtx (mask_mode);
   rtx sel = vec_perm_indices_to_rtx (sel_mode, d->perm);
@@ -2988,7 +2986,7 @@ shuffle_compress_patterns (struct expand_vec_perm_d *d)
     return false;
   /* Build a mask that is true when selector element is true.  */
-  machine_mode mask_mode = get_mask_mode (vmode).require ();
+  machine_mode mask_mode = get_mask_mode (vmode);
   rvv_builder builder (mask_mode, vlen, 1);
   for (int i = 0; i < vlen; i++)
     {
@@ -3040,7 +3038,7 @@ static bool
shuffle_decompress_patterns (struct expand_vec_perm_d *d)
{
   poly_uint64 nelt = d->perm.length ();
-  machine_mode mask_mode = get_mask_mode (d->vmode).require ();
+  machine_mode mask_mode = get_mask_mode (d->vmode);
   /* For constant size indices, we dont't need to handle it here.
      Just leave it to vec_perm<mode>.  */
@@ -3498,7 +3496,7 @@ expand_reduction (rtx_code code, rtx *ops, rtx init, reduction_type type)
   rtx vector = type == reduction_type::UNORDERED ? ops[1] : ops[2];
   machine_mode vmode = GET_MODE (vector);
   machine_mode m1_mode = get_m1_mode (vmode).require ();
-  machine_mode m1_mmode = get_mask_mode (m1_mode).require ();
+  machine_mode m1_mmode = get_mask_mode (m1_mode);
   rtx m1_tmp = gen_reg_rtx (m1_mode);
   rtx m1_mask = gen_scalar_move_mask (m1_mmode);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dc78f4cf977..1eacca06b5c 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7541,9 +7541,8 @@ riscv_support_vector_misalignment (machine_mode mode,
static opt_machine_mode
riscv_get_mask_mode (machine_mode mode)
{
-  machine_mode mask_mode = VOIDmode;
-  if (TARGET_VECTOR && riscv_vector::get_mask_mode (mode).exists (&mask_mode))
-    return mask_mode;
+  if (TARGET_VECTOR && riscv_v_ext_mode_p (mode))
+    return riscv_vector::get_mask_mode (mode);
   return default_get_mask_mode (mode);
}
-- 
2.40.1
 
 

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31  6:58 ` juzhe.zhong
@ 2023-07-31  7:02   ` Kito Cheng
  2023-07-31 11:06     ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Kito Cheng @ 2023-07-31  7:02 UTC (permalink / raw)
  To: juzhe.zhong; +Cc: Kito.cheng, gcc-patches, Robin Dapp, pan2.li

Pushed, thanks :)

On Mon, Jul 31, 2023 at 2:59 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> ok
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-07-31 14:52
> To: gcc-patches; kito.cheng; juzhe.zhong; rdapp.gcc; pan2.li
> CC: Kito Cheng
> Subject: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
> We always want get_mask_mode return a valid mode, it's something wrong
> if it failed, so I think we could just move the `.require ()` into
> get_mask_mode, instead of calling that every call-site.
>
> The only exception is riscv_get_mask_mode, it might put supported mode
> into get_mask_mode, so added a check with riscv_v_ext_mode_p to make
> sure only valid vector mode will ask get_mask_mode.
>
> gcc/ChangeLog:
>
> * config/riscv/autovec.md (abs<mode>2): Remove `.require ()`.
> * config/riscv/riscv-protos.h (get_mask_mode): Update return
> type.
> * config/riscv/riscv-v.cc (rvv_builder::rvv_builder): Remove
> `.require ()`.
> (emit_vlmax_insn): Ditto.
> (emit_vlmax_fp_insn): Ditto.
> (emit_vlmax_ternary_insn): Ditto.
> (emit_vlmax_fp_ternary_insn): Ditto.
> (emit_nonvlmax_fp_ternary_tu_insn): Ditto.
> (emit_nonvlmax_insn): Ditto.
> (emit_vlmax_slide_insn): Ditto.
> (emit_nonvlmax_slide_tu_insn): Ditto.
> (emit_vlmax_merge_insn): Ditto.
> (emit_vlmax_masked_insn): Ditto.
> (emit_nonvlmax_masked_insn): Ditto.
> (emit_vlmax_masked_store_insn): Ditto.
> (emit_nonvlmax_masked_store_insn): Ditto.
> (emit_vlmax_masked_mu_insn): Ditto.
> (emit_nonvlmax_tu_insn): Ditto.
> (emit_nonvlmax_fp_tu_insn): Ditto.
> (emit_scalar_move_insn): Ditto.
> (emit_vlmax_compress_insn): Ditto.
> (emit_vlmax_reduction_insn): Ditto.
> (emit_vlmax_fp_reduction_insn): Ditto.
> (emit_nonvlmax_fp_reduction_insn): Ditto.
> (expand_vec_series): Ditto.
> (expand_vector_init_merge_repeating_sequence): Ditto.
> (expand_vec_perm): Ditto.
> (shuffle_merge_patterns): Ditto.
> (shuffle_compress_patterns): Ditto.
> (shuffle_decompress_patterns): Ditto.
> (expand_reduction): Ditto.
> (get_mask_mode): Update return type.
> * config/riscv/riscv.cc (riscv_get_mask_mode): Check vector type
> is valid, and use new get_mask_mode interface.
> ---
> gcc/config/riscv/autovec.md     |  2 +-
> gcc/config/riscv/riscv-protos.h |  2 +-
> gcc/config/riscv/riscv-v.cc     | 68 ++++++++++++++++-----------------
> gcc/config/riscv/riscv.cc       |  5 +--
> 4 files changed, 37 insertions(+), 40 deletions(-)
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index b7ea3101f5a..fd7ec911e87 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -912,7 +912,7 @@ (define_expand "abs<mode>2"
>    "TARGET_VECTOR"
> {
>    rtx zero = gen_const_vec_duplicate (<MODE>mode, GEN_INT (0));
> -  machine_mode mask_mode = riscv_vector::get_mask_mode (<MODE>mode).require ();
> +  machine_mode mask_mode = riscv_vector::get_mask_mode (<MODE>mode);
>    rtx mask = gen_reg_rtx (mask_mode);
>    riscv_vector::expand_vec_cmp (mask, LT, operands[1], zero);
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 7d8bf2b81f8..324991e2619 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -312,7 +312,7 @@ bool slide1_sew64_helper (int, machine_mode, machine_mode,
> rtx gen_avl_for_scalar_move (rtx);
> void expand_tuple_move (rtx *);
> machine_mode preferred_simd_mode (scalar_mode);
> -opt_machine_mode get_mask_mode (machine_mode);
> +machine_mode get_mask_mode (machine_mode);
> void expand_vec_series (rtx, rtx, rtx);
> void expand_vec_init (rtx, rtx);
> void expand_vec_perm (rtx, rtx, rtx, rtx);
> diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> index 0a355eb3c7a..5ab749f4343 100644
> --- a/gcc/config/riscv/riscv-v.cc
> +++ b/gcc/config/riscv/riscv-v.cc
> @@ -285,7 +285,7 @@ public:
>      m_inner_mode = GET_MODE_INNER (mode);
>      m_inner_bits_size = GET_MODE_BITSIZE (m_inner_mode);
>      m_inner_bytes_size = GET_MODE_SIZE (m_inner_mode);
> -    m_mask_mode = get_mask_mode (mode).require ();
> +    m_mask_mode = get_mask_mode (mode);
>      gcc_assert (
>        int_mode_for_size (inner_bits_size (), 0).exists (&m_inner_int_mode));
> @@ -676,7 +676,7 @@ void
> emit_vlmax_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -698,7 +698,7 @@ void
> emit_vlmax_fp_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -721,7 +721,7 @@ void
> emit_vlmax_ternary_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ true,
> @@ -742,7 +742,7 @@ void
> emit_vlmax_fp_ternary_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ true,
> @@ -764,7 +764,7 @@ static void
> emit_nonvlmax_fp_ternary_tu_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -786,7 +786,7 @@ void
> emit_nonvlmax_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -808,7 +808,7 @@ void
> emit_vlmax_slide_insn (unsigned icode, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SLIDE_OP,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -830,7 +830,7 @@ void
> emit_nonvlmax_slide_tu_insn (unsigned icode, rtx *ops, rtx avl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SLIDE_OP,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -853,7 +853,7 @@ void
> emit_vlmax_merge_insn (unsigned icode, int op_num, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ false,
> @@ -908,7 +908,7 @@ static void
> emit_vlmax_masked_insn (unsigned icode, int op_num, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -926,7 +926,7 @@ static void
> emit_nonvlmax_masked_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -945,7 +945,7 @@ static void
> emit_vlmax_masked_store_insn (unsigned icode, int op_num, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ false,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -961,7 +961,7 @@ static void
> emit_nonvlmax_masked_store_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ false,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -978,7 +978,7 @@ void
> emit_vlmax_masked_mu_insn (unsigned icode, int op_num, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -996,7 +996,7 @@ static void
> emit_nonvlmax_tu_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -1015,7 +1015,7 @@ static void
> emit_nonvlmax_fp_tu_insn (unsigned icode, int op_num, rtx *ops, rtx avl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (/*OP_NUM*/ op_num,
>   /*HAS_DEST_P*/ true,
>   /*FULLY_UNMASKED_P*/ false,
> @@ -1036,7 +1036,7 @@ void
> emit_scalar_move_insn (unsigned icode, rtx *ops, rtx len)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_SCALAR_MOV_OP,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ false,
> @@ -1156,7 +1156,7 @@ static void
> emit_vlmax_compress_insn (unsigned icode, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (dest_mode).require ();
> +  machine_mode mask_mode = get_mask_mode (dest_mode);
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (RVV_COMPRESS_OP,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ false,
> @@ -1174,7 +1174,7 @@ static void
> emit_vlmax_reduction_insn (unsigned icode, int op_num, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
> +  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -1192,7 +1192,7 @@ static void
> emit_vlmax_fp_reduction_insn (unsigned icode, int op_num, rtx *ops)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
> +  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ true,
> @@ -1211,7 +1211,7 @@ static void
> emit_nonvlmax_fp_reduction_insn (unsigned icode, int op_num, rtx *ops, rtx vl)
> {
>    machine_mode dest_mode = GET_MODE (ops[0]);
> -  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1])).require ();
> +  machine_mode mask_mode = get_mask_mode (GET_MODE (ops[1]));
>    insn_expander<RVV_INSN_OPERANDS_MAX> e (op_num,
>   /* HAS_DEST_P */ true,
>   /* FULLY_UNMASKED_P */ false,
> @@ -1248,8 +1248,7 @@ void
> expand_vec_series (rtx dest, rtx base, rtx step)
> {
>    machine_mode mode = GET_MODE (dest);
> -  machine_mode mask_mode;
> -  gcc_assert (get_mask_mode (mode).exists (&mask_mode));
> +  machine_mode mask_mode = get_mask_mode (mode);
>    poly_int64 nunits_m1 = GET_MODE_NUNITS (mode) - 1;
>    poly_int64 value;
> @@ -1789,10 +1788,10 @@ get_avl_type_rtx (enum avl_type type)
> /* Return the appropriate mask mode for MODE.  */
> -opt_machine_mode
> +machine_mode
> get_mask_mode (machine_mode mode)
> {
> -  return get_vector_mode (BImode, GET_MODE_NUNITS (mode));
> +  return get_vector_mode (BImode, GET_MODE_NUNITS (mode)).require();
> }
> /* Return the appropriate M1 mode for MODE.  */
> @@ -2318,8 +2317,7 @@ expand_vector_init_insert_elems (rtx target, const rvv_builder &builder,
> int nelts_reqd)
> {
>    machine_mode mode = GET_MODE (target);
> -  machine_mode mask_mode;
> -  gcc_assert (get_mask_mode (mode).exists (&mask_mode));
> +  machine_mode mask_mode = get_mask_mode (mode);
>    rtx dup = expand_vector_broadcast (mode, builder.elt (0));
>    emit_move_insn (target, dup);
>    int ndups = builder.count_dups (0, nelts_reqd - 1, 1);
> @@ -2345,8 +2343,8 @@ expand_vector_init_merge_repeating_sequence (rtx target,
>      const rvv_builder &builder)
> {
>    machine_mode dup_mode = get_repeating_sequence_dup_machine_mode (builder);
> -  machine_mode dup_mask_mode = get_mask_mode (dup_mode).require ();
> -  machine_mode mask_mode = get_mask_mode (builder.mode ()).require ();
> +  machine_mode dup_mask_mode = get_mask_mode (dup_mode);
> +  machine_mode mask_mode = get_mask_mode (builder.mode ());
>    uint64_t full_nelts = builder.full_nelts ().to_constant ();
>    /* Step 1: Broadcast the first pattern.  */
> @@ -2796,7 +2794,7 @@ expand_vec_perm (rtx target, rtx op0, rtx op1, rtx sel)
>       __builtin_shufflevector (vec1, vec2, index...), the index can be any
>       value in range of [0, 2 * nunits - 1].  */
>    machine_mode mask_mode;
> -  mask_mode = get_mask_mode (data_mode).require ();
> +  mask_mode = get_mask_mode (data_mode);
>    rtx mask = gen_reg_rtx (mask_mode);
>    max_sel = gen_const_vector_dup (sel_mode, nunits);
> @@ -2868,7 +2866,7 @@ shuffle_merge_patterns (struct expand_vec_perm_d *d)
>    if (d->testing_p)
>      return true;
> -  machine_mode mask_mode = get_mask_mode (vmode).require ();
> +  machine_mode mask_mode = get_mask_mode (vmode);
>    rtx mask = gen_reg_rtx (mask_mode);
>    rtx sel = vec_perm_indices_to_rtx (sel_mode, d->perm);
> @@ -2988,7 +2986,7 @@ shuffle_compress_patterns (struct expand_vec_perm_d *d)
>      return false;
>    /* Build a mask that is true when selector element is true.  */
> -  machine_mode mask_mode = get_mask_mode (vmode).require ();
> +  machine_mode mask_mode = get_mask_mode (vmode);
>    rvv_builder builder (mask_mode, vlen, 1);
>    for (int i = 0; i < vlen; i++)
>      {
> @@ -3040,7 +3038,7 @@ static bool
> shuffle_decompress_patterns (struct expand_vec_perm_d *d)
> {
>    poly_uint64 nelt = d->perm.length ();
> -  machine_mode mask_mode = get_mask_mode (d->vmode).require ();
> +  machine_mode mask_mode = get_mask_mode (d->vmode);
>    /* For constant size indices, we dont't need to handle it here.
>       Just leave it to vec_perm<mode>.  */
> @@ -3498,7 +3496,7 @@ expand_reduction (rtx_code code, rtx *ops, rtx init, reduction_type type)
>    rtx vector = type == reduction_type::UNORDERED ? ops[1] : ops[2];
>    machine_mode vmode = GET_MODE (vector);
>    machine_mode m1_mode = get_m1_mode (vmode).require ();
> -  machine_mode m1_mmode = get_mask_mode (m1_mode).require ();
> +  machine_mode m1_mmode = get_mask_mode (m1_mode);
>    rtx m1_tmp = gen_reg_rtx (m1_mode);
>    rtx m1_mask = gen_scalar_move_mask (m1_mmode);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index dc78f4cf977..1eacca06b5c 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7541,9 +7541,8 @@ riscv_support_vector_misalignment (machine_mode mode,
> static opt_machine_mode
> riscv_get_mask_mode (machine_mode mode)
> {
> -  machine_mode mask_mode = VOIDmode;
> -  if (TARGET_VECTOR && riscv_vector::get_mask_mode (mode).exists (&mask_mode))
> -    return mask_mode;
> +  if (TARGET_VECTOR && riscv_v_ext_mode_p (mode))
> +    return riscv_vector::get_mask_mode (mode);
>    return default_get_mask_mode (mode);
> }
> --
> 2.40.1
>
>

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31  7:02   ` Kito Cheng
@ 2023-07-31 11:06     ` Maciej W. Rozycki
  2023-07-31 13:46       ` Kito Cheng
  0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2023-07-31 11:06 UTC (permalink / raw)
  To: Kito Cheng; +Cc: juzhe.zhong, Kito.cheng, gcc-patches, Robin Dapp, pan2.li

On Mon, 31 Jul 2023, Kito Cheng via Gcc-patches wrote:

> Pushed, thanks :)

 This breaks compilation:

.../gcc/config/riscv/riscv-v.cc: In function 'void riscv_vector::expand_vec_series(rtx, rtx, rtx)':
.../gcc/config/riscv/riscv-v.cc:1251:16: error: unused variable 'mask_mode' [-Werror=unused-variable]
 1251 |   machine_mode mask_mode = get_mask_mode (mode);
      |                ^~~~~~~~~
.../gcc/config/riscv/riscv-v.cc: In function 'void riscv_vector::expand_vector_init_insert_elems(rtx, const riscv_vector::rvv_builder&, int)':
.../gcc/config/riscv/riscv-v.cc:2320:16: error: unused variable 'mask_mode' [-Werror=unused-variable]
 2320 |   machine_mode mask_mode = get_mask_mode (mode);
      |                ^~~~~~~~~

Please always at the very least build changes and verify that they cause 
no new issues before submitting patches.

  Maciej

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 11:06     ` Maciej W. Rozycki
@ 2023-07-31 13:46       ` Kito Cheng
  2023-07-31 14:03         ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Kito Cheng @ 2023-07-31 13:46 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: juzhe.zhong, Kito.cheng, gcc-patches, Robin Dapp, pan2.li

Hi Maciej:

Sorry for disturbing, pushed a fix for that, and...added
-Werror=unused-variable to my build script to prevent that happen
again :(

On Mon, Jul 31, 2023 at 7:08 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> On Mon, 31 Jul 2023, Kito Cheng via Gcc-patches wrote:
>
> > Pushed, thanks :)
>
>  This breaks compilation:
>
> .../gcc/config/riscv/riscv-v.cc: In function 'void riscv_vector::expand_vec_series(rtx, rtx, rtx)':
> .../gcc/config/riscv/riscv-v.cc:1251:16: error: unused variable 'mask_mode' [-Werror=unused-variable]
>  1251 |   machine_mode mask_mode = get_mask_mode (mode);
>       |                ^~~~~~~~~
> .../gcc/config/riscv/riscv-v.cc: In function 'void riscv_vector::expand_vector_init_insert_elems(rtx, const riscv_vector::rvv_builder&, int)':
> .../gcc/config/riscv/riscv-v.cc:2320:16: error: unused variable 'mask_mode' [-Werror=unused-variable]
>  2320 |   machine_mode mask_mode = get_mask_mode (mode);
>       |                ^~~~~~~~~
>
> Please always at the very least build changes and verify that they cause
> no new issues before submitting patches.
>
>   Maciej

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 13:46       ` Kito Cheng
@ 2023-07-31 14:03         ` Maciej W. Rozycki
  2023-07-31 14:52           ` Kito Cheng
  0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2023-07-31 14:03 UTC (permalink / raw)
  To: Kito Cheng; +Cc: juzhe.zhong, Kito.cheng, gcc-patches, Robin Dapp, pan2.li

On Mon, 31 Jul 2023, Kito Cheng wrote:

> Sorry for disturbing, pushed a fix for that, and...added
> -Werror=unused-variable to my build script to prevent that happen
> again :(

 I just configure with `--enable-werror-always', which we want to keep 
our standards up to anyway, but if you find this infeasible for some 
reason with your workflow, then there's always an option to grep for 
warnings in the build log and diff that against the previous iteration.

  Maciej

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 14:03         ` Maciej W. Rozycki
@ 2023-07-31 14:52           ` Kito Cheng
  2023-07-31 15:20             ` Jeff Law
  2023-07-31 15:50             ` Maciej W. Rozycki
  0 siblings, 2 replies; 11+ messages in thread
From: Kito Cheng @ 2023-07-31 14:52 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Kito Cheng, juzhe.zhong, gcc-patches, Robin Dapp, pan2.li

On Mon, Jul 31, 2023 at 10:03 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> On Mon, 31 Jul 2023, Kito Cheng wrote:
>
> > Sorry for disturbing, pushed a fix for that, and...added
> > -Werror=unused-variable to my build script to prevent that happen
> > again :(
>
>  I just configure with `--enable-werror-always', which we want to keep
> our standards up to anyway,

I rely on the host GCC which is 11 relatively old compared to the
trunk, so --enable-werror-always will get many -Wformat* warning :(

> but if you find this infeasible for some
> reason with your workflow, then there's always an option to grep for
> warnings in the build log and diff that against the previous iteration.

That's a good suggestion! Thanks, let me try to apply myself workflow  :)

>
>   Maciej

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 14:52           ` Kito Cheng
@ 2023-07-31 15:20             ` Jeff Law
  2023-07-31 15:55               ` Maciej W. Rozycki
  2023-07-31 15:50             ` Maciej W. Rozycki
  1 sibling, 1 reply; 11+ messages in thread
From: Jeff Law @ 2023-07-31 15:20 UTC (permalink / raw)
  To: Kito Cheng, Maciej W. Rozycki
  Cc: Kito Cheng, juzhe.zhong, gcc-patches, Robin Dapp, pan2.li



On 7/31/23 08:52, Kito Cheng via Gcc-patches wrote:
> On Mon, Jul 31, 2023 at 10:03 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>>
>> On Mon, 31 Jul 2023, Kito Cheng wrote:
>>
>>> Sorry for disturbing, pushed a fix for that, and...added
>>> -Werror=unused-variable to my build script to prevent that happen
>>> again :(
>>
>>   I just configure with `--enable-werror-always', which we want to keep
>> our standards up to anyway,
> 
> I rely on the host GCC which is 11 relatively old compared to the
> trunk, so --enable-werror-always will get many -Wformat* warning :(
> 
>> but if you find this infeasible for some
>> reason with your workflow, then there's always an option to grep for
>> warnings in the build log and diff that against the previous iteration.
> 
> That's a good suggestion! Thanks, let me try to apply myself workflow  :)
I'm thinking that as part of the CI POC being done by RISE that the base 
AMI image ought to be gcc-13 based and that we should configure the 
toolchains we build with -enable-werror-always.

While we can't necessarily get every developer to embrace this workflow, 
we ought to be catching it quicker than we currently are.

jeff

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 14:52           ` Kito Cheng
  2023-07-31 15:20             ` Jeff Law
@ 2023-07-31 15:50             ` Maciej W. Rozycki
  1 sibling, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2023-07-31 15:50 UTC (permalink / raw)
  To: Kito Cheng; +Cc: Kito Cheng, juzhe.zhong, gcc-patches, Robin Dapp, pan2.li

On Mon, 31 Jul 2023, Kito Cheng wrote:

> >  I just configure with `--enable-werror-always', which we want to keep
> > our standards up to anyway,
> 
> I rely on the host GCC which is 11 relatively old compared to the
> trunk, so --enable-werror-always will get many -Wformat* warning :(

 If building a cross-compiler for upstream submissions or regression runs 
I always bootstrap a native compiler of the same checkout first and then 
use it for the build.  I think it's good practice, and it's needed for the 
Ada frontend anyway.  That's one way to avoid introducing warnings by 
chance, and it takes less than an hour to bootstrap native GCC on decent 
contemporary hardware (and then you don't have to be pedantic, and neither 
I am, and you can keep reusing an older native build for some time).

  Maciej

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 15:20             ` Jeff Law
@ 2023-07-31 15:55               ` Maciej W. Rozycki
  2023-08-09 13:25                 ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2023-07-31 15:55 UTC (permalink / raw)
  To: Jeff Law
  Cc: Kito Cheng, Kito Cheng, juzhe.zhong, gcc-patches, Robin Dapp, pan2.li

On Mon, 31 Jul 2023, Jeff Law wrote:

> > That's a good suggestion! Thanks, let me try to apply myself workflow  :)
> I'm thinking that as part of the CI POC being done by RISE that the base AMI
> image ought to be gcc-13 based and that we should configure the toolchains we
> build with -enable-werror-always.
> 
> While we can't necessarily get every developer to embrace this workflow, we
> ought to be catching it quicker than we currently are.

 I wonder if we should enable the option by default, perhaps under certain 
conditions such as matching the build compiler version, for builds made 
from a Git checkout rather than a release tarball.  I suspect some people 
are simply not aware of this option.

  Maciej

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

* Re: [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC
  2023-07-31 15:55               ` Maciej W. Rozycki
@ 2023-08-09 13:25                 ` Maciej W. Rozycki
  0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2023-08-09 13:25 UTC (permalink / raw)
  To: Jeff Law
  Cc: Kito Cheng, Kito Cheng, juzhe.zhong, gcc-patches, Robin Dapp, pan2.li

On Mon, 31 Jul 2023, Maciej W. Rozycki wrote:

> > > That's a good suggestion! Thanks, let me try to apply myself workflow  :)
> > I'm thinking that as part of the CI POC being done by RISE that the base AMI
> > image ought to be gcc-13 based and that we should configure the toolchains we
> > build with -enable-werror-always.
> > 
> > While we can't necessarily get every developer to embrace this workflow, we
> > ought to be catching it quicker than we currently are.
> 
>  I wonder if we should enable the option by default, perhaps under certain 
> conditions such as matching the build compiler version, for builds made 
> from a Git checkout rather than a release tarball.  I suspect some people 
> are simply not aware of this option.

 Also the Linux kernel community has bots that monitor the relevant 
mailing lists for patches, apply them, build in various configurations, 
and report back any issues, so when you submit a change that doesn't 
compile in some cases, then it's often within minutes that you get a 
notification, even before anyone has a chance to review your submission.  
That also helps maintainers catch such issues before a change gets merged 
anywhere.

 Cf. <https://github.com/intel/lkp-tests/wiki>, 
<https://www.intel.com/content/www/us/en/developer/topic-technology/open/linux-kernel-performance/overview.html>.

 That surely hasn't come for free, someone had to make the infrastructure,
and then with contemporary hardware the Linux kernel often builds within 
seconds, which we don't have the luxury of, but I wonder if it's an 
approach that has been previously considered for GCC.  Overall I think the 
more effort we can offload to automata the less remains for us.

  Maciej

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

end of thread, other threads:[~2023-08-09 13:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-31  6:52 [PATCH] RISC-V: Return machine_mode rather than opt_machine_mode for get_mask_mode, NFC Kito Cheng
2023-07-31  6:58 ` juzhe.zhong
2023-07-31  7:02   ` Kito Cheng
2023-07-31 11:06     ` Maciej W. Rozycki
2023-07-31 13:46       ` Kito Cheng
2023-07-31 14:03         ` Maciej W. Rozycki
2023-07-31 14:52           ` Kito Cheng
2023-07-31 15:20             ` Jeff Law
2023-07-31 15:55               ` Maciej W. Rozycki
2023-08-09 13:25                 ` Maciej W. Rozycki
2023-07-31 15:50             ` Maciej W. Rozycki

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).