RE: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode intrinsic
Thanks Kito, update it in PATCH v3, and passed riscv/rvv.exp already.
https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626918.html
Pan
-Original Message-
From: Kito Cheng
Sent: Thursday, August 10, 2023 10:21 AM
To: Li, Pan2
Cc: Kito Cheng ; gcc-patches@gcc.gnu.org;
juzhe.zh...@rivai.ai; jeffreya...@gmail.com; Wang, Yanzhang
Subject: Re: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode
intrinsic
Yeah, no further comment from me :)
On Thu, Aug 10, 2023 at 10:16 AM Li, Pan2 wrote:
>
> Thanks kito. It makes sense, should not reach default, may I prepare v3(add
> gcc_unreachable to default) if no more comments?
>
> Pan
>
> -Original Message-
> From: Kito Cheng
> Sent: Thursday, August 10, 2023 10:12 AM
> To: Li, Pan2
> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; jeffreya...@gmail.com;
> Wang, Yanzhang ; Kito Cheng
> Subject: Re: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode
> intrinsic
>
> > +/* Get the frm mode with given CONST_INT rtx, the default mode is
> > + FRM_DYN. */
> > +enum floating_point_rounding_mode
> > +get_frm_mode (rtx operand)
> > +{
> > + gcc_assert (CONST_INT_P (operand));
> > +
> > + switch (INTVAL (operand))
> > +{
> > +case FRM_RNE:
> > + return FRM_RNE;
> > +case FRM_RTZ:
> > + return FRM_RTZ;
> > +case FRM_RDN:
> > + return FRM_RDN;
> > +case FRM_RUP:
> > + return FRM_RUP;
> > +case FRM_RMM:
> > + return FRM_RMM;
> > +case FRM_DYN:
> > + return FRM_DYN;
> > +default:
> > + return FRM_DYN;
>
> Should we put a gcc_unreachable or gcc_assert here? I am not sure if
> another value is valid when it appeared for this operand?
>
> > +}
> > +
> > + gcc_unreachable ();
> > +}
Re: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode intrinsic
Yeah, no further comment from me :)
On Thu, Aug 10, 2023 at 10:16 AM Li, Pan2 wrote:
>
> Thanks kito. It makes sense, should not reach default, may I prepare v3(add
> gcc_unreachable to default) if no more comments?
>
> Pan
>
> -Original Message-
> From: Kito Cheng
> Sent: Thursday, August 10, 2023 10:12 AM
> To: Li, Pan2
> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; jeffreya...@gmail.com;
> Wang, Yanzhang ; Kito Cheng
> Subject: Re: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode
> intrinsic
>
> > +/* Get the frm mode with given CONST_INT rtx, the default mode is
> > + FRM_DYN. */
> > +enum floating_point_rounding_mode
> > +get_frm_mode (rtx operand)
> > +{
> > + gcc_assert (CONST_INT_P (operand));
> > +
> > + switch (INTVAL (operand))
> > +{
> > +case FRM_RNE:
> > + return FRM_RNE;
> > +case FRM_RTZ:
> > + return FRM_RTZ;
> > +case FRM_RDN:
> > + return FRM_RDN;
> > +case FRM_RUP:
> > + return FRM_RUP;
> > +case FRM_RMM:
> > + return FRM_RMM;
> > +case FRM_DYN:
> > + return FRM_DYN;
> > +default:
> > + return FRM_DYN;
>
> Should we put a gcc_unreachable or gcc_assert here? I am not sure if
> another value is valid when it appeared for this operand?
>
> > +}
> > +
> > + gcc_unreachable ();
> > +}
RE: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode intrinsic
Thanks kito. It makes sense, should not reach default, may I prepare v3(add
gcc_unreachable to default) if no more comments?
Pan
-Original Message-
From: Kito Cheng
Sent: Thursday, August 10, 2023 10:12 AM
To: Li, Pan2
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; jeffreya...@gmail.com; Wang,
Yanzhang ; Kito Cheng
Subject: Re: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode
intrinsic
> +/* Get the frm mode with given CONST_INT rtx, the default mode is
> + FRM_DYN. */
> +enum floating_point_rounding_mode
> +get_frm_mode (rtx operand)
> +{
> + gcc_assert (CONST_INT_P (operand));
> +
> + switch (INTVAL (operand))
> +{
> +case FRM_RNE:
> + return FRM_RNE;
> +case FRM_RTZ:
> + return FRM_RTZ;
> +case FRM_RDN:
> + return FRM_RDN;
> +case FRM_RUP:
> + return FRM_RUP;
> +case FRM_RMM:
> + return FRM_RMM;
> +case FRM_DYN:
> + return FRM_DYN;
> +default:
> + return FRM_DYN;
Should we put a gcc_unreachable or gcc_assert here? I am not sure if
another value is valid when it appeared for this operand?
> +}
> +
> + gcc_unreachable ();
> +}
Re: [PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode intrinsic
> +/* Get the frm mode with given CONST_INT rtx, the default mode is
> + FRM_DYN. */
> +enum floating_point_rounding_mode
> +get_frm_mode (rtx operand)
> +{
> + gcc_assert (CONST_INT_P (operand));
> +
> + switch (INTVAL (operand))
> +{
> +case FRM_RNE:
> + return FRM_RNE;
> +case FRM_RTZ:
> + return FRM_RTZ;
> +case FRM_RDN:
> + return FRM_RDN;
> +case FRM_RUP:
> + return FRM_RUP;
> +case FRM_RMM:
> + return FRM_RMM;
> +case FRM_DYN:
> + return FRM_DYN;
> +default:
> + return FRM_DYN;
Should we put a gcc_unreachable or gcc_assert here? I am not sure if
another value is valid when it appeared for this operand?
> +}
> +
> + gcc_unreachable ();
> +}
[PATCH v2] RISC-V: Refactor RVV frm_mode attr for rounding mode intrinsic
From: Pan Li
The frm_mode attr has some assumptions for each define insn as below.
1. The define insn has at least 9 operands.
2. The operands[9] must be frm reg.
3. The operands[9] must be const int.
Actually, the frm operand can be operands[8], operands[9] or
operands[10], and not all the define insn has frm operands.
This patch would like to refactor frm and eliminate the above
assumptions, as well as unblock the underlying rounding mode intrinsic
API support.
After refactor, the default frm will be none, and the selected insn type
will be dyn. For the floating point which honors the frm, we will
set the frm_mode attr explicitly in define_insn.
Signed-off-by: Pan Li
Co-Authored-by: Kito Cheng
gcc/ChangeLog:
* config/riscv/riscv-protos.h (get_frm_mode): Remove operand
assumptions.
* config/riscv/riscv-v.cc (get_frm_mode): New function.
* config/riscv/riscv-vector-builtins.cc
(function_expander::use_ternop_insn):
* config/riscv/vector.md: Set frm_mode attr explicitly.
gcc/ChangeLog:
* config/riscv/riscv-protos.h
(enum floating_point_rounding_mode): Add NONE, DYN_EXIT and DYN_CALL.
(get_frm_mode): New declaration.
* config/riscv/riscv-v.cc (get_frm_mode): New function to get frm mode.
* config/riscv/riscv-vector-builtins.cc
(function_expander::use_ternop_insn): Take care of frm reg.
* config/riscv/riscv.cc (riscv_static_frm_mode_p): Migrate to FRM_XXX.
(riscv_emit_frm_mode_set): Ditto.
(riscv_emit_mode_set): Ditto.
(riscv_frm_adjust_mode_after_call): Ditto.
(riscv_frm_mode_needed): Ditto.
(riscv_frm_mode_after): Ditto.
(riscv_mode_entry): Ditto.
(riscv_mode_exit): Ditto.
* config/riscv/riscv.h (NUM_MODES_FOR_MODE_SWITCHING): Ditto.
* config/riscv/vector.md
(rne,rtz,rdn,rup,rmm,dyn,dyn_exit,dyn_call,none): Removed
(symbol_ref): * config/riscv/vector.md: Set frm_mode attr explicitly.
---
gcc/config/riscv/riscv-protos.h | 4 +
gcc/config/riscv/riscv-v.cc | 29
gcc/config/riscv/riscv-vector-builtins.cc | 22 ++-
gcc/config/riscv/riscv.cc | 46 +++---
gcc/config/riscv/riscv.h | 2 +-
gcc/config/riscv/vector.md| 172 ++
6 files changed, 186 insertions(+), 89 deletions(-)
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 324991e2619..f45653848de 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -344,8 +344,12 @@ enum floating_point_rounding_mode
FRM_DYN = 7, /* Aka 0b111. */
FRM_STATIC_MIN = FRM_RNE,
FRM_STATIC_MAX = FRM_RMM,
+ FRM_DYN_EXIT = 8,
+ FRM_DYN_CALL = 9,
+ FRM_NONE = 10,
};
+enum floating_point_rounding_mode get_frm_mode (rtx);
opt_machine_mode vectorize_related_mode (machine_mode, scalar_mode,
poly_uint64);
unsigned int autovectorize_vector_modes (vec *, bool);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 278452b9e05..9ab6ae17d33 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -112,6 +112,7 @@ public:
{
m_has_fp_rounding_mode_p = true;
m_fp_rounding_mode = mode;
+gcc_assert (mode <= FRM_DYN);
}
void add_output_operand (rtx x, machine_mode mode)
@@ -1513,6 +1514,34 @@ expand_const_vector (rtx target, rtx src)
gcc_unreachable ();
}
+/* Get the frm mode with given CONST_INT rtx, the default mode is
+ FRM_DYN. */
+enum floating_point_rounding_mode
+get_frm_mode (rtx operand)
+{
+ gcc_assert (CONST_INT_P (operand));
+
+ switch (INTVAL (operand))
+{
+case FRM_RNE:
+ return FRM_RNE;
+case FRM_RTZ:
+ return FRM_RTZ;
+case FRM_RDN:
+ return FRM_RDN;
+case FRM_RUP:
+ return FRM_RUP;
+case FRM_RMM:
+ return FRM_RMM;
+case FRM_DYN:
+ return FRM_DYN;
+default:
+ return FRM_DYN;
+}
+
+ gcc_unreachable ();
+}
+
/* Expand a pre-RA RVV data move from SRC to DEST.
It expands move for RVV fractional vector modes. */
bool
diff --git a/gcc/config/riscv/riscv-vector-builtins.cc
b/gcc/config/riscv/riscv-vector-builtins.cc
index 528dca7ae85..abab06c00ed 100644
--- a/gcc/config/riscv/riscv-vector-builtins.cc
+++ b/gcc/config/riscv/riscv-vector-builtins.cc
@@ -3730,17 +3730,29 @@ function_expander::use_ternop_insn (bool vd_accum_p,
insn_code icode)
}
for (int argno = arg_offset; argno < call_expr_nargs (exp); argno++)
-add_input_operand (argno);
+{
+ if (base->has_rounding_mode_operand_p ()
+ && argno == call_expr_nargs (exp) - 2)
+ {
+ /* Since the rounding mode argument position is not consistent with
+the instruction pattern, we need to skip rounding mode argument
+here. */
+ continue;
+ }
+ add_input_operand (argno);
+}
