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;