[PATCH 0/3] Fix RVV calling incorrect RFV/RVD check functions bug

[PATCH 0/3] Fix RVV calling incorrect RFV/RVD check functions bug

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

For vector widening and narrowing floating-point instructions, we should
use require_scale_rvf() instead of require_rvf() to check whether the
correspond RVF/RVD is enabled if either source or destination
floating-point operand is double-width of SEW. Otherwise, illegal
instruction exception should be raised.

e.g. For SEW=16, if the source/destination floating-point operand is
double-width of SEW, RVF needs to be enabled. Otherwise, an illegal
instruction exception will be raised. Similarly, for SEW=32, RVD
needs to be enabled.

Frank Chang (3):
  target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
    widening fp insns
  target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
    widening fp/int type-convert insns
  target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
    narrowing fp/int type-convert insns

 target/riscv/insn_trans/trans_rvv.c.inc | 78 ++++++++++++++++++-------
 1 file changed, 57 insertions(+), 21 deletions(-)

--
2.31.1

[PATCH 1/3] target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for widening fp insns

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

Vector widening floating-point instructions should use
require_scale_rvf() instead of require_rvf() to check whether RVF/RVD is
enabled.
---
 target/riscv/insn_trans/trans_rvv.c.inc | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 5e3f7fdb77..8d92243f2b 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2254,7 +2254,8 @@ GEN_OPFVF_TRANS(vfrsub_vf,  opfvf_check)
 static bool opfvv_widen_check(DisasContext *s, arg_rmrr *a)
 {
     return require_rvv(s) &&
-           require_rvf(s) &&
+           require_scale_rvf(s) &&
+           (s->sew != MO_8) &&
            vext_check_isa_ill(s) &&
            vext_check_dss(s, a->rd, a->rs1, a->rs2, a->vm);
 }
@@ -2292,7 +2293,8 @@ GEN_OPFVV_WIDEN_TRANS(vfwsub_vv, opfvv_widen_check)
 static bool opfvf_widen_check(DisasContext *s, arg_rmrr *a)
 {
     return require_rvv(s) &&
-           require_rvf(s) &&
+           require_scale_rvf(s) &&
+           (s->sew != MO_8) &&
            vext_check_isa_ill(s) &&
            vext_check_ds(s, a->rd, a->rs2, a->vm);
 }
@@ -2321,7 +2323,8 @@ GEN_OPFVF_WIDEN_TRANS(vfwsub_vf)
 static bool opfwv_widen_check(DisasContext *s, arg_rmrr *a)
 {
     return require_rvv(s) &&
-           require_rvf(s) &&
+           require_scale_rvf(s) &&
+           (s->sew != MO_8) &&
            vext_check_isa_ill(s) &&
            vext_check_dds(s, a->rd, a->rs1, a->rs2, a->vm);
 }
@@ -2359,7 +2362,8 @@ GEN_OPFWV_WIDEN_TRANS(vfwsub_wv)
 static bool opfwf_widen_check(DisasContext *s, arg_rmrr *a)
 {
     return require_rvv(s) &&
-           require_rvf(s) &&
+           require_scale_rvf(s) &&
+           (s->sew != MO_8) &&
            vext_check_isa_ill(s) &&
            vext_check_dd(s, a->rd, a->rs2, a->vm);
 }
-- 
2.31.1

Re: [PATCH 1/3] target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for widening fp insns

[Alistair Francis]

On Wed, Dec 29, 2021 at 12:15 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Vector widening floating-point instructions should use
> require_scale_rvf() instead of require_rvf() to check whether RVF/RVD is
> enabled.

Missing Signed off by line

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 5e3f7fdb77..8d92243f2b 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -2254,7 +2254,8 @@ GEN_OPFVF_TRANS(vfrsub_vf,  opfvf_check)
>  static bool opfvv_widen_check(DisasContext *s, arg_rmrr *a)
>  {
>      return require_rvv(s) &&
> -           require_rvf(s) &&
> +           require_scale_rvf(s) &&
> +           (s->sew != MO_8) &&
>             vext_check_isa_ill(s) &&
>             vext_check_dss(s, a->rd, a->rs1, a->rs2, a->vm);
>  }
> @@ -2292,7 +2293,8 @@ GEN_OPFVV_WIDEN_TRANS(vfwsub_vv, opfvv_widen_check)
>  static bool opfvf_widen_check(DisasContext *s, arg_rmrr *a)
>  {
>      return require_rvv(s) &&
> -           require_rvf(s) &&
> +           require_scale_rvf(s) &&
> +           (s->sew != MO_8) &&
>             vext_check_isa_ill(s) &&
>             vext_check_ds(s, a->rd, a->rs2, a->vm);
>  }
> @@ -2321,7 +2323,8 @@ GEN_OPFVF_WIDEN_TRANS(vfwsub_vf)
>  static bool opfwv_widen_check(DisasContext *s, arg_rmrr *a)
>  {
>      return require_rvv(s) &&
> -           require_rvf(s) &&
> +           require_scale_rvf(s) &&
> +           (s->sew != MO_8) &&
>             vext_check_isa_ill(s) &&
>             vext_check_dds(s, a->rd, a->rs1, a->rs2, a->vm);
>  }
> @@ -2359,7 +2362,8 @@ GEN_OPFWV_WIDEN_TRANS(vfwsub_wv)
>  static bool opfwf_widen_check(DisasContext *s, arg_rmrr *a)
>  {
>      return require_rvv(s) &&
> -           require_rvf(s) &&
> +           require_scale_rvf(s) &&
> +           (s->sew != MO_8) &&
>             vext_check_isa_ill(s) &&
>             vext_check_dd(s, a->rd, a->rs2, a->vm);
>  }
> --
> 2.31.1
>
>

[PATCH 2/3] target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for widening fp/int type-convert insns

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

vfwcvt.xu.f.v, vfwcvt.x.f.v, vfwcvt.rtz.xu.f.v and vfwcvt.rtz.x.f.v
convert single-width floating-point to double-width integer.
Therefore, should use require_rvf() to check whether RVF/RVD is enabled.

vfwcvt.f.xu.v, vfwcvt.f.x.v convert single-width integer to double-width
floating-point, and vfwcvt.f.f.v convert double-width floating-point to
single-width floating-point. Therefore, should use require_scale_rvf() to
check whether RVF/RVD is enabled.
---
 target/riscv/insn_trans/trans_rvv.c.inc | 34 ++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 8d92243f2b..f1b44ccad2 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2613,16 +2613,27 @@ GEN_OPFV_CVT_TRANS(vfcvt_rtz_x_f_v, vfcvt_x_f_v, RISCV_FRM_RTZ)
 static bool opfv_widen_check(DisasContext *s, arg_rmr *a)
 {
     return require_rvv(s) &&
-           require_scale_rvf(s) &&
-           (s->sew != MO_8) &&
            vext_check_isa_ill(s) &&
            vext_check_ds(s, a->rd, a->rs2, a->vm);
 }
 
-#define GEN_OPFV_WIDEN_TRANS(NAME, HELPER, FRM)                    \
+static bool opxfv_widen_check(DisasContext *s, arg_rmr *a)
+{
+    return opfv_widen_check(s, a) &&
+           require_rvf(s);
+}
+
+static bool opffv_widen_check(DisasContext *s, arg_rmr *a)
+{
+    return opfv_widen_check(s, a) &&
+           require_scale_rvf(s) &&
+           (s->sew != MO_8);
+}
+
+#define GEN_OPFV_WIDEN_TRANS(NAME, CHECK, HELPER, FRM)             \
 static bool trans_##NAME(DisasContext *s, arg_rmr *a)              \
 {                                                                  \
-    if (opfv_widen_check(s, a)) {                                  \
+    if (CHECK(s, a)) {                                             \
         if (FRM != RISCV_FRM_DYN) {                                \
             gen_set_rm(s, RISCV_FRM_DYN);                          \
         }                                                          \
@@ -2649,12 +2660,17 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a)              \
     return false;                                                  \
 }
 
-GEN_OPFV_WIDEN_TRANS(vfwcvt_xu_f_v, vfwcvt_xu_f_v, RISCV_FRM_DYN)
-GEN_OPFV_WIDEN_TRANS(vfwcvt_x_f_v, vfwcvt_x_f_v, RISCV_FRM_DYN)
-GEN_OPFV_WIDEN_TRANS(vfwcvt_f_f_v, vfwcvt_f_f_v, RISCV_FRM_DYN)
+GEN_OPFV_WIDEN_TRANS(vfwcvt_xu_f_v, opxfv_widen_check, vfwcvt_xu_f_v,
+                     RISCV_FRM_DYN)
+GEN_OPFV_WIDEN_TRANS(vfwcvt_x_f_v, opxfv_widen_check, vfwcvt_x_f_v,
+                     RISCV_FRM_DYN)
+GEN_OPFV_WIDEN_TRANS(vfwcvt_f_f_v, opffv_widen_check, vfwcvt_f_f_v,
+                     RISCV_FRM_DYN)
 /* Reuse the helper functions from vfwcvt.xu.f.v and vfwcvt.x.f.v */
-GEN_OPFV_WIDEN_TRANS(vfwcvt_rtz_xu_f_v, vfwcvt_xu_f_v, RISCV_FRM_RTZ)
-GEN_OPFV_WIDEN_TRANS(vfwcvt_rtz_x_f_v, vfwcvt_x_f_v, RISCV_FRM_RTZ)
+GEN_OPFV_WIDEN_TRANS(vfwcvt_rtz_xu_f_v, opxfv_widen_check, vfwcvt_xu_f_v,
+                     RISCV_FRM_RTZ)
+GEN_OPFV_WIDEN_TRANS(vfwcvt_rtz_x_f_v, opxfv_widen_check, vfwcvt_x_f_v,
+                     RISCV_FRM_RTZ)
 
 static bool opfxv_widen_check(DisasContext *s, arg_rmr *a)
 {
-- 
2.31.1

[PATCH 3/3] target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for narrowing fp/int type-convert insns

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

vfncvt.f.xu.w, vfncvt.f.x.w convert double-width integer to single-width
floating-point. Therefore, should use require_rvf() to check whether
RVF/RVD is enabled.

vfncvt.f.f.w, vfncvt.rod.f.f.w convert double-width floating-point to
single-width integer. Therefore, should use require_scale_rvf() to check
whether RVF/RVD is enabled.
---
 target/riscv/insn_trans/trans_rvv.c.inc | 32 ++++++++++++++++++-------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index f1b44ccad2..6c285c958b 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2719,17 +2719,29 @@ GEN_OPFXV_WIDEN_TRANS(vfwcvt_f_x_v)
 static bool opfv_narrow_check(DisasContext *s, arg_rmr *a)
 {
     return require_rvv(s) &&
-           require_rvf(s) &&
-           (s->sew != MO_64) &&
            vext_check_isa_ill(s) &&
            /* OPFV narrowing instructions ignore vs1 check */
            vext_check_sd(s, a->rd, a->rs2, a->vm);
 }
 
-#define GEN_OPFV_NARROW_TRANS(NAME, HELPER, FRM)                   \
+static bool opfxv_narrow_check(DisasContext *s, arg_rmr *a)
+{
+    return opfv_narrow_check(s, a) &&
+           require_rvf(s) &&
+           (s->sew != MO_64);
+}
+
+static bool opffv_narrow_check(DisasContext *s, arg_rmr *a)
+{
+    return opfv_narrow_check(s, a) &&
+           require_scale_rvf(s) &&
+           (s->sew != MO_8);
+}
+
+#define GEN_OPFV_NARROW_TRANS(NAME, CHECK, HELPER, FRM)            \
 static bool trans_##NAME(DisasContext *s, arg_rmr *a)              \
 {                                                                  \
-    if (opfv_narrow_check(s, a)) {                                 \
+    if (CHECK(s, a)) {                                             \
         if (FRM != RISCV_FRM_DYN) {                                \
             gen_set_rm(s, RISCV_FRM_DYN);                          \
         }                                                          \
@@ -2756,11 +2768,15 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a)              \
     return false;                                                  \
 }
 
-GEN_OPFV_NARROW_TRANS(vfncvt_f_xu_w, vfncvt_f_xu_w, RISCV_FRM_DYN)
-GEN_OPFV_NARROW_TRANS(vfncvt_f_x_w, vfncvt_f_x_w, RISCV_FRM_DYN)
-GEN_OPFV_NARROW_TRANS(vfncvt_f_f_w, vfncvt_f_f_w, RISCV_FRM_DYN)
+GEN_OPFV_NARROW_TRANS(vfncvt_f_xu_w, opfxv_narrow_check, vfncvt_f_xu_w,
+                      RISCV_FRM_DYN)
+GEN_OPFV_NARROW_TRANS(vfncvt_f_x_w, opfxv_narrow_check, vfncvt_f_x_w,
+                      RISCV_FRM_DYN)
+GEN_OPFV_NARROW_TRANS(vfncvt_f_f_w, opffv_narrow_check, vfncvt_f_f_w,
+                      RISCV_FRM_DYN)
 /* Reuse the helper function from vfncvt.f.f.w */
-GEN_OPFV_NARROW_TRANS(vfncvt_rod_f_f_w, vfncvt_f_f_w, RISCV_FRM_ROD)
+GEN_OPFV_NARROW_TRANS(vfncvt_rod_f_f_w, opffv_narrow_check, vfncvt_f_f_w,
+                      RISCV_FRM_ROD)
 
 static bool opxfv_narrow_check(DisasContext *s, arg_rmr *a)
 {
-- 
2.31.1

Re: [PATCH 0/3] Fix RVV calling incorrect RFV/RVD check functions bug

[Frank Chang]

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

<frank.chang@sifive.com> 於 2021年12月29日 週三 上午10:13寫道：

> From: Frank Chang <frank.chang@sifive.com>
>
> For vector widening and narrowing floating-point instructions, we should
> use require_scale_rvf() instead of require_rvf() to check whether the
> correspond RVF/RVD is enabled if either source or destination
> floating-point operand is double-width of SEW. Otherwise, illegal
> instruction exception should be raised.
>
> e.g. For SEW=16, if the source/destination floating-point operand is
> double-width of SEW, RVF needs to be enabled. Otherwise, an illegal
> instruction exception will be raised. Similarly, for SEW=32, RVD
> needs to be enabled.
>
> Frank Chang (3):
>   target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
>     widening fp insns
>   target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
>     widening fp/int type-convert insns
>   target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
>     narrowing fp/int type-convert insns


Oops, I misspell the word "function" in the patch titles.
I will correct it in my next patchset.

Regards,
Frank Chang


>


>  target/riscv/insn_trans/trans_rvv.c.inc | 78 ++++++++++++++++++-------
>  1 file changed, 57 insertions(+), 21 deletions(-)
>
> --
> 2.31.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 2070 bytes --]

Re: [PATCH 0/3] Fix RVV calling incorrect RFV/RVD check functions bug

[Frank Chang]

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

On Wed, Dec 29, 2021 at 10:43 AM Frank Chang <frank.chang@sifive.com> wrote:

> <frank.chang@sifive.com> 於 2021年12月29日 週三 上午10:13寫道：
>
>> From: Frank Chang <frank.chang@sifive.com>
>>
>> For vector widening and narrowing floating-point instructions, we should
>> use require_scale_rvf() instead of require_rvf() to check whether the
>> correspond RVF/RVD is enabled if either source or destination
>> floating-point operand is double-width of SEW. Otherwise, illegal
>> instruction exception should be raised.
>>
>> e.g. For SEW=16, if the source/destination floating-point operand is
>> double-width of SEW, RVF needs to be enabled. Otherwise, an illegal
>> instruction exception will be raised. Similarly, for SEW=32, RVD
>> needs to be enabled.
>>
>> Frank Chang (3):
>>   target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
>>     widening fp insns
>>   target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
>>     widening fp/int type-convert insns
>>   target/riscv: rvv-1.0: Call the correct RVF/RVD check funtion for
>>     narrowing fp/int type-convert insns
>
>
> Oops, I misspell the word "function" in the patch titles.
> I will correct it in my next patchset.
>
> Regards,
> Frank Chang
>

I also missed the 'Signed-off-by' in this patch series.
Will also fix it in my next-version patchset.

Regards,
Frank Chang


>
>
>>
>
>
>>  target/riscv/insn_trans/trans_rvv.c.inc | 78 ++++++++++++++++++-------
>>  1 file changed, 57 insertions(+), 21 deletions(-)
>>
>> --
>> 2.31.1
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 2825 bytes --]