I would prefer to introduce an enum template argument and refactor
existing code later :)
On Wed, Aug 16, 2023 at 11:40 AM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> That should work as well, but may require some changes to existing codes like
> declaration, etc.
> I am OK for both the enum or inherit, and will start with the CVT parts, then
> refactor the existing frm class.
>
> Do you have any suggestion for the decision making?
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.ch...@gmail.com>
> Sent: Wednesday, August 16, 2023 11:30 AM
> To: Li, Pan2 <pan2...@intel.com>
> Cc: juzhe.zh...@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>; Wang,
> Yanzhang <yanzhang.w...@intel.com>
> Subject: Re: [PATCH v1] RISC-V: Support RVV VFCVT.X.F.V rounding mode
> intrinsic API
>
> Or using an enum value rather than bool?
>
> I am thinking we could also simplify/remove most other frm classes,
> some practical example:
>
>
> diff --git a/gcc/config/riscv/riscv-vector-builtins-bases.cc
> b/gcc/config/riscv/riscv-vector-builtins-bases.cc
> index 2074dac0f16..ace63e963a5 100644
> --- a/gcc/config/riscv/riscv-vector-builtins-bases.cc
> +++ b/gcc/config/riscv/riscv-vector-builtins-bases.cc
> @@ -58,6 +58,11 @@ enum lst_type
> LST_INDEXED,
> };
>
> +enum frm_op_type
> +{
> + NO_FRM,
> + HAS_FRM
> +};
> /* Helper function to fold vleff and vlsegff. */
> static gimple *
> fold_fault_load (gimple_folder &f)
> @@ -256,41 +261,22 @@ public:
> vremu/vsadd/vsaddu/vssub/vssubu
> vfadd/vfsub/
> */
> -template<rtx_code CODE>
> +template<rtx_code CODE, enum frm_op_type FRM_OP = NO_FRM>
> class binop : public function_base
> {
> public:
> - rtx expand (function_expander &e) const override
> + bool has_rounding_mode_operand_p () const override
> {
> - switch (e.op_info->op)
> - {
> - case OP_TYPE_vx:
> - case OP_TYPE_vf:
> - return e.use_exact_insn (code_for_pred_scalar (CODE, e.vector_mode
> ()));
> - case OP_TYPE_vv:
> - return e.use_exact_insn (code_for_pred (CODE, e.vector_mode ()));
> - default:
> - gcc_unreachable ();
> - }
> + return FRM_OP == HAS_FRM;
> }
> -};
> -
> -/* Implements below instructions for now.
> - - vfadd
> - - vfsub
> - - vfmul
> - - vfdiv
> -*/
> -template<rtx_code CODE>
> -class binop_frm : public function_base
> -{
> -public:
> - bool has_rounding_mode_operand_p () const override { return true; }
>
> rtx expand (function_expander &e) const override
> {
> switch (e.op_info->op)
> {
> + case OP_TYPE_vx:
> + gcc_assert (FRM_OP == NO_FRM);
> + gcc_fallthrough ();
> case OP_TYPE_vf:
> return e.use_exact_insn (code_for_pred_scalar (CODE, e.vector_mode
> ()));
> case OP_TYPE_vv:
> @@ -1648,10 +1634,15 @@ public:
> };
>
> /* Implements vfcvt.x. */
> -template<int UNSPEC>
> +template<int UNSPEC, enum frm_op_type FRM_OP = NO_FRM>
> class vfcvt_x : public function_base
> {
> public:
> + bool has_rounding_mode_operand_p () const override
> + {
> + return FRM_OP == HAS_FRM;
> + }
> +
> rtx expand (function_expander &e) const override
> {
> return e.use_exact_insn (code_for_pred_fcvt_x_f (UNSPEC, e.arg_mode (0)));
> @@ -2389,8 +2380,8 @@ static CONSTEXPR const viota viota_obj;
> static CONSTEXPR const vid vid_obj;
> static CONSTEXPR const binop<PLUS> vfadd_obj;
> static CONSTEXPR const binop<MINUS> vfsub_obj;
> -static CONSTEXPR const binop_frm<PLUS> vfadd_frm_obj;
> -static CONSTEXPR const binop_frm<MINUS> vfsub_frm_obj;
> +static CONSTEXPR const binop<PLUS, HAS_FRM> vfadd_frm_obj;
> +static CONSTEXPR const binop<MINUS, HAS_FRM> vfsub_frm_obj;
> static CONSTEXPR const reverse_binop<MINUS> vfrsub_obj;
> static CONSTEXPR const reverse_binop_frm<MINUS> vfrsub_frm_obj;
> static CONSTEXPR const widen_binop<PLUS> vfwadd_obj;
> @@ -2398,9 +2389,9 @@ static CONSTEXPR const widen_binop_frm<PLUS>
> vfwadd_frm_obj;
> static CONSTEXPR const widen_binop<MINUS> vfwsub_obj;
> static CONSTEXPR const widen_binop_frm<MINUS> vfwsub_frm_obj;
> static CONSTEXPR const binop<MULT> vfmul_obj;
> -static CONSTEXPR const binop_frm<MULT> vfmul_frm_obj;
> +static CONSTEXPR const binop<MULT, HAS_FRM> vfmul_frm_obj;
> static CONSTEXPR const binop<DIV> vfdiv_obj;
> -static CONSTEXPR const binop_frm<DIV> vfdiv_frm_obj;
> +static CONSTEXPR const binop<DIV, HAS_FRM> vfdiv_frm_obj;
> static CONSTEXPR const reverse_binop<DIV> vfrdiv_obj;
> static CONSTEXPR const reverse_binop_frm<DIV> vfrdiv_frm_obj;
> static CONSTEXPR const widen_binop<MULT> vfwmul_obj;
