Sure thing, echo on below part. I think we need one place to put something like summary for this, for example, a table to indicate some information about this (aka rounding mode needed or not). I will try to summarize one draft then. > Check SPIKE implementation, make sure which API needs rounding mode, which API doesn't need rounding mode. > Do not trust the rvv-intrinsic-doc since it's often wrong. > You should check doc too, if doc is wrong, you should not only correct GCC implementation but also make a fix PR to the doc. Pan From: juzhe.zhong@rivai.ai Sent: Thursday, June 29, 2023 10:44 AM To: Kito.cheng ; Li, Pan2 Cc: gcc-patches ; Wang, Yanzhang ; jeffreyalaw Subject: Re: Re: [PATCH v1] RISC-V: Allow rounding mode control for RVV floating-point add Hi, Pan. I think the last step is to support dynamic mode switching which may need to change the mode-switching PASS. After this done, I suggest you go over all rounding mode API (including fixed-point and floating-point.) Check SPIKE implementation, make sure which API needs rounding mode, which API doesn't need rounding mode. Do not trust the rvv-intrinsic-doc since it's often wrong. You should check doc too, if doc is wrong, you should not only correct GCC implementation but also make a fix PR to the doc. Thanks. ________________________________ juzhe.zhong@rivai.ai From: Kito Cheng Date: 2023-06-29 10:35 To: Li, Pan2 CC: juzhe.zhong@rivai.ai; gcc-patches; Wang, Yanzhang; jeffreyalaw Subject: Re: [PATCH v1] RISC-V: Allow rounding mode control for RVV floating-point add LGTM, thanks! On Tue, Jun 27, 2023 at 3:02 PM Li, Pan2 > wrote: > > Ack, thanks Juzhe. > > > > Pan > > > > From: juzhe.zhong@rivai.ai > > Sent: Tuesday, June 27, 2023 3:00 PM > To: Li, Pan2 >; gcc-patches > > Cc: Kito.cheng >; Li, Pan2 >; Wang, Yanzhang >; jeffreyalaw > > Subject: Re: [PATCH v1] RISC-V: Allow rounding mode control for RVV floating-point add > > > > LGTM. > > You can go ahead to implement rounding mode of floating-point by mode-switching: > > > > Suggest you implement rounding mode for floating-poing as follows: > > > > 1st step: Implement mode-switching for floating-point rounding mode except DYNAMIC which should be totally same as fixed-point. > > 2nd step: Support DYNAMIC rounding mode on mode-switching which may need to modify the mode-switching PASS. > > > > Thanks. > > ________________________________ > > juzhe.zhong@rivai.ai > > > > From: pan2.li > > Date: 2023-06-27 14:06 > > To: gcc-patches > > CC: juzhe.zhong; kito.cheng; pan2.li; yanzhang.wang; jeffreyalaw > > Subject: [PATCH v1] RISC-V: Allow rounding mode control for RVV floating-point add > > From: Pan Li > > > > > According to the doc as below, we need to support the rounding mode of > > the RVV floating-point, both the static and dynamice frm. > > > > https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/226 > > > > For tracking and development friendly, We will take some steps to support > > all rounding modes for the RVV floating-point rounding modes. > > > > 1. Allow rounding mode control by one intrinsic (aka this patch), vfadd. > > 2. Support static rounding mode control by mode switch, like fixed-point. > > 3. Support dynamice round mode control by mode switch. > > 4. Support the rest floating-point instructions for frm. > > > > Please *NOTE* this patch only allow the rounding mode control for the > > vfadd intrinsic API, and the related frm will be coverred by step 2. > > > > Signed-off-by: Pan Li > > > Co-Authored by: Juzhe-Zhong > > > > > gcc/ChangeLog: > > > > * config/riscv/riscv-protos.h (enum floating_point_rounding_mode): > > Add macro for static frm min and max. > > * config/riscv/riscv-vector-builtins-bases.cc > > (class binop_frm): New class for floating-point with frm. > > (BASE): Add vfadd for frm. > > * config/riscv/riscv-vector-builtins-bases.h: Likewise. > > * config/riscv/riscv-vector-builtins-functions.def > > (vfadd_frm): Likewise. > > * config/riscv/riscv-vector-builtins-shapes.cc > > (struct alu_frm_def): New struct for alu with frm. > > (SHAPE): Add alu with frm. > > * config/riscv/riscv-vector-builtins-shapes.h: Likewise. > > * config/riscv/riscv-vector-builtins.cc > > (function_checker::report_out_of_range_and_not): New function > > for report out of range and not val. > > (function_checker::require_immediate_range_or): New function > > for checking in range or one val. > > * config/riscv/riscv-vector-builtins.h: Add function decl. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/rvv/base/float-point-frm-error.c: New test. > > * gcc.target/riscv/rvv/base/float-point-frm.c: New test. > > --- > > gcc/config/riscv/riscv-protos.h | 2 + > > .../riscv/riscv-vector-builtins-bases.cc | 25 +++++++ > > .../riscv/riscv-vector-builtins-bases.h | 1 + > > .../riscv/riscv-vector-builtins-functions.def | 2 + > > .../riscv/riscv-vector-builtins-shapes.cc | 68 +++++++++++++++++++ > > .../riscv/riscv-vector-builtins-shapes.h | 1 + > > gcc/config/riscv/riscv-vector-builtins.cc | 41 +++++++++++ > > gcc/config/riscv/riscv-vector-builtins.h | 4 ++ > > .../riscv/rvv/base/float-point-frm-error.c | 15 ++++ > > .../riscv/rvv/base/float-point-frm.c | 30 ++++++++ > > 10 files changed, 189 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm-error.c > > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm.c > > > > diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h > > index f686edab3d1..bee64eee504 100644 > > --- a/gcc/config/riscv/riscv-protos.h > > +++ b/gcc/config/riscv/riscv-protos.h > > @@ -278,6 +278,8 @@ enum floating_point_rounding_mode > > FRM_RUP = 3, /* Aka 0b011. */ > > FRM_RMM = 4, /* Aka 0b100. */ > > FRM_DYN = 7, /* Aka 0b111. */ > > + FRM_STATIC_MIN = FRM_RNE, > > + FRM_STATIC_MAX = FRM_RMM, > > }; > > opt_machine_mode vectorize_related_mode (machine_mode, scalar_mode, > > diff --git a/gcc/config/riscv/riscv-vector-builtins-bases.cc b/gcc/config/riscv/riscv-vector-builtins-bases.cc > > index 5c8deda900d..1b4c2c6ad66 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins-bases.cc > > +++ b/gcc/config/riscv/riscv-vector-builtins-bases.cc > > @@ -281,6 +281,29 @@ public: > > } > > }; > > +/* Implements below instructions for now. > > + - vfadd > > +*/ > > +template > > +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_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 (); > > + } > > + } > > +}; > > + > > /* Implements vrsub. */ > > class vrsub : public function_base > > { > > @@ -2006,6 +2029,7 @@ static CONSTEXPR const viota viota_obj; > > static CONSTEXPR const vid vid_obj; > > static CONSTEXPR const binop vfadd_obj; > > static CONSTEXPR const binop vfsub_obj; > > +static CONSTEXPR const binop_frm vfadd_frm_obj; > > static CONSTEXPR const reverse_binop vfrsub_obj; > > static CONSTEXPR const widen_binop vfwadd_obj; > > static CONSTEXPR const widen_binop vfwsub_obj; > > @@ -2231,6 +2255,7 @@ BASE (vmsof) > > BASE (viota) > > BASE (vid) > > BASE (vfadd) > > +BASE (vfadd_frm) > > BASE (vfsub) > > BASE (vfrsub) > > BASE (vfwadd) > > diff --git a/gcc/config/riscv/riscv-vector-builtins-bases.h b/gcc/config/riscv/riscv-vector-builtins-bases.h > > index 62ff38a2811..54a81eab269 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins-bases.h > > +++ b/gcc/config/riscv/riscv-vector-builtins-bases.h > > @@ -145,6 +145,7 @@ extern const function_base *const viota; > > extern const function_base *const vid; > > extern const function_base *const vfadd; > > extern const function_base *const vfadd; > > +extern const function_base *const vfadd_frm; > > extern const function_base *const vfsub; > > extern const function_base *const vfsub; > > extern const function_base *const vfrsub; > > diff --git a/gcc/config/riscv/riscv-vector-builtins-functions.def b/gcc/config/riscv/riscv-vector-builtins-functions.def > > index 89aff27bf26..035c9e4252f 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins-functions.def > > +++ b/gcc/config/riscv/riscv-vector-builtins-functions.def > > @@ -289,6 +289,8 @@ DEF_RVV_FUNCTION (vfadd, alu, full_preds, f_vvf_ops) > > DEF_RVV_FUNCTION (vfsub, alu, full_preds, f_vvv_ops) > > DEF_RVV_FUNCTION (vfsub, alu, full_preds, f_vvf_ops) > > DEF_RVV_FUNCTION (vfrsub, alu, full_preds, f_vvf_ops) > > +DEF_RVV_FUNCTION (vfadd_frm, alu_frm, full_preds, f_vvv_ops) > > +DEF_RVV_FUNCTION (vfadd_frm, alu_frm, full_preds, f_vvf_ops) > > // 13.3. Vector Widening Floating-Point Add/Subtract Instructions > > DEF_RVV_FUNCTION (vfwadd, widen_alu, full_preds, f_wvv_ops) > > diff --git a/gcc/config/riscv/riscv-vector-builtins-shapes.cc b/gcc/config/riscv/riscv-vector-builtins-shapes.cc > > index c8daae01f91..69a67106418 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins-shapes.cc > > +++ b/gcc/config/riscv/riscv-vector-builtins-shapes.cc > > @@ -226,6 +226,73 @@ struct alu_def : public build_base > > } > > }; > > +/* alu_frm_def class. */ > > +struct alu_frm_def : public build_base > > +{ > > + /* Normalize vf_frm to vf. */ > > + static void normalize_base_name (char *to, const char *from, int limit) > > + { > > + strncpy (to, from, limit - 1); > > + char *suffix = strstr (to, "_frm"); > > + > > + if (suffix) > > + *suffix = '\0'; > > + > > + to[limit - 1] = '\0'; > > + } > > + > > + char *get_name (function_builder &b, const function_instance &instance, > > + bool overloaded_p) const override > > + { > > + char base_name[16] = {}; > > + > > + /* Return nullptr if it can not be overloaded. */ > > + if (overloaded_p && !instance.base->can_be_overloaded_p (instance.pred)) > > + return nullptr; > > + > > + normalize_base_name (base_name, instance.base_name, sizeof (base_name)); > > + > > + b.append_base_name (base_name); > > + > > + /* vop_ --> vop__. */ > > + if (!overloaded_p) > > + { > > + b.append_name (operand_suffixes[instance.op_info->op]); > > + b.append_name (type_suffixes[instance.type.index].vector); > > + } > > + > > + /* According to rvv-intrinsic-doc, it does not add "_m" suffix > > + for vop_m C++ overloaded API. */ > > + if (overloaded_p && instance.pred == PRED_TYPE_m) > > + return b.finish_name (); > > + > > + b.append_name (predication_suffixes[instance.pred]); > > + > > + /* According to rvv-intrinsic-doc, it does not add "_rm" suffix > > + for vop_rm C++ overloaded API. */ > > + if (!overloaded_p) > > + b.append_name ("_rm"); > > + > > + return b.finish_name (); > > + } > > + > > + bool check (function_checker &c) const override > > + { > > + gcc_assert (c.any_type_float_p ()); > > + > > + /* Check whether rounding mode argument is a valid immediate. */ > > + if (c.base->has_rounding_mode_operand_p ()) > > + { > > + unsigned int frm_num = c.arg_num () - 2; > > + > > + return c.require_immediate_range_or (frm_num, FRM_STATIC_MIN, > > + FRM_STATIC_MAX, FRM_DYN); > > + } > > + > > + return true; > > + } > > +}; > > + > > /* widen_alu_def class. Handle vwadd/vwsub. Unlike > > vadd.vx/vadd.vv/vwmul.vv/vwmul.vx, vwadd.vv/vwadd.vx/vwadd.wv/vwadd.wx has > > 'OP' suffix in overloaded API. */ > > @@ -743,6 +810,7 @@ SHAPE(vsetvl, vsetvlmax) > > SHAPE(loadstore, loadstore) > > SHAPE(indexed_loadstore, indexed_loadstore) > > SHAPE(alu, alu) > > +SHAPE(alu_frm, alu_frm) > > SHAPE(widen_alu, widen_alu) > > SHAPE(no_mask_policy, no_mask_policy) > > SHAPE(return_mask, return_mask) > > diff --git a/gcc/config/riscv/riscv-vector-builtins-shapes.h b/gcc/config/riscv/riscv-vector-builtins-shapes.h > > index 6a51713c12c..15fef8342ec 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins-shapes.h > > +++ b/gcc/config/riscv/riscv-vector-builtins-shapes.h > > @@ -29,6 +29,7 @@ extern const function_shape *const vsetvlmax; > > extern const function_shape *const loadstore; > > extern const function_shape *const indexed_loadstore; > > extern const function_shape *const alu; > > +extern const function_shape *const alu_frm; > > extern const function_shape *const widen_alu; > > extern const function_shape *const no_mask_policy; > > extern const function_shape *const return_mask; > > diff --git a/gcc/config/riscv/riscv-vector-builtins.cc b/gcc/config/riscv/riscv-vector-builtins.cc > > index 466e36d50b7..648c765a5d1 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins.cc > > +++ b/gcc/config/riscv/riscv-vector-builtins.cc > > @@ -3852,6 +3852,23 @@ function_checker::report_out_of_range (unsigned int argno, HOST_WIDE_INT actual, > > actual, argno + 1, fndecl, min, max); > > } > > +/* Report that LOCATION has a call to FNDECL in which argument ARGNO has > > + the value ACTUAL, whereas the function requires a value in the range > > + [MIN, MAX] or OR_VAL. ARGNO counts from zero. */ > > +void > > +function_checker::report_out_of_range_and_not (unsigned int argno, > > + HOST_WIDE_INT actual, > > + HOST_WIDE_INT min, > > + HOST_WIDE_INT max, > > + HOST_WIDE_INT or_val) const > > +{ > > + error_at (location, > > + "passing %wd to argument %d of %qE, which expects" > > + " a value in the range [%wd, %wd] or %wd", > > + actual, argno + 1, fndecl, min, max, or_val); > > +} > > + > > + > > /* Check that argument ARGNO is an integer constant expression and > > store its value in VALUE_OUT if so. The caller should first > > check that argument ARGNO exists. */ > > @@ -3893,6 +3910,30 @@ function_checker::require_immediate_range (unsigned int argno, > > return true; > > } > > +/* Check that argument REL_ARGNO is an integer constant expression in the > > + range [MIN, MAX] or OR_VAL. REL_ARGNO counts from the end of the > > + predication arguments. */ > > +bool > > +function_checker::require_immediate_range_or (unsigned int argno, > > + HOST_WIDE_INT min, > > + HOST_WIDE_INT max, > > + HOST_WIDE_INT or_val) const > > +{ > > + gcc_assert (min >= 0 && min <= max); > > + gcc_assert (argno < m_nargs); > > + > > + tree arg = m_args[argno]; > > + HOST_WIDE_INT actual = tree_to_uhwi (arg); > > + > > + if (!IN_RANGE (actual, min, max) && actual != or_val) > > + { > > + report_out_of_range_and_not (argno, actual, min, max, or_val); > > + return false; > > + } > > + > > + return true; > > +} > > + > > /* Perform semantic checks on the call. Return true if the call is valid, > > otherwise report a suitable error. */ > > bool > > diff --git a/gcc/config/riscv/riscv-vector-builtins.h b/gcc/config/riscv/riscv-vector-builtins.h > > index b0c3a42d820..e358a8e4d91 100644 > > --- a/gcc/config/riscv/riscv-vector-builtins.h > > +++ b/gcc/config/riscv/riscv-vector-builtins.h > > @@ -442,6 +442,8 @@ public: > > bool check (void); > > bool require_immediate (unsigned int, HOST_WIDE_INT, HOST_WIDE_INT) const; > > + bool require_immediate_range_or (unsigned int, HOST_WIDE_INT, > > + HOST_WIDE_INT, HOST_WIDE_INT) const; > > private: > > bool require_immediate_range (unsigned int, HOST_WIDE_INT, > > @@ -449,6 +451,8 @@ private: > > void report_non_ice (unsigned int) const; > > void report_out_of_range (unsigned int, HOST_WIDE_INT, HOST_WIDE_INT, > > HOST_WIDE_INT) const; > > + void report_out_of_range_and_not (unsigned int, HOST_WIDE_INT, HOST_WIDE_INT, > > + HOST_WIDE_INT, HOST_WIDE_INT) const; > > /* The type of the resolved function. */ > > tree m_fntype; > > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm-error.c b/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm-error.c > > new file mode 100644 > > index 00000000000..4ebaa15ab0b > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm-error.c > > @@ -0,0 +1,15 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-march=rv64gcv -mabi=lp64 -O3 -Wno-psabi" } */ > > + > > +#include "riscv_vector.h" > > + > > +typedef float float32_t; > > + > > +void test_float_point_frm_error (float32_t *out, vfloat32m1_t op1, vfloat32m1_t op2, size_t vl) > > +{ > > + vfloat32m1_t v1 = __riscv_vfadd_vv_f32m1_rm (op1, op2, 5, vl); /* { dg-error {passing 5 to argument 3 of '__riscv_vfadd_vv_f32m1_rm', which expects a value in the range \[0, 4\] or 7} } */ > > + vfloat32m1_t v2 = __riscv_vfadd_vv_f32m1_rm (v1, v1, 6, vl); /* { dg-error {passing 6 to argument 3 of '__riscv_vfadd_vv_f32m1_rm', which expects a value in the range \[0, 4\] or 7} } */ > > + vfloat32m1_t v3 = __riscv_vfadd_vv_f32m1_rm (v2, v2, 8, vl); /* { dg-error {passing 8 to argument 3 of '__riscv_vfadd_vv_f32m1_rm', which expects a value in the range \[0, 4\] or 7} } */ > > + > > + __riscv_vse32_v_f32m1 (out, v3, vl); > > +} > > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm.c b/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm.c > > new file mode 100644 > > index 00000000000..95271b2c822 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm.c > > @@ -0,0 +1,30 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-march=rv64gcv -mabi=lp64 -O3 -Wno-psabi" } */ > > + > > +#include "riscv_vector.h" > > + > > +typedef float float32_t; > > + > > +vfloat32m1_t > > +test_riscv_vfadd_vv_f32m1_rm (vfloat32m1_t op1, vfloat32m1_t op2, size_t vl) { > > + return __riscv_vfadd_vv_f32m1_rm (op1, op2, 0, vl); > > +} > > + > > +vfloat32m1_t > > +test_vfadd_vv_f32m1_m_rm(vbool32_t mask, vfloat32m1_t op1, vfloat32m1_t op2, > > + size_t vl) { > > + return __riscv_vfadd_vv_f32m1_m_rm(mask, op1, op2, 0, vl); > > +} > > + > > +vfloat32m1_t > > +test_vfadd_vf_f32m1_rm(vfloat32m1_t op1, float32_t op2, size_t vl) { > > + return __riscv_vfadd_vf_f32m1_rm(op1, op2, 0, vl); > > +} > > + > > +vfloat32m1_t > > +test_vfadd_vf_f32m1_m_rm(vbool32_t mask, vfloat32m1_t op1, float32_t op2, > > + size_t vl) { > > + return __riscv_vfadd_vf_f32m1_m_rm(mask, op1, op2, 0, vl); > > +} > > + > > +/* { dg-final { scan-assembler-times {vfadd\.v[vf]\s+v[0-9]+,\s*v[0-9]+,\s*[fav]+[0-9]+} 4 } } */ > > -- > > 2.34.1 > > > >