[PATCH v5 56/60] target/riscv: floating-point scalar move instructions

LIU Zhiwei posted 60 patches 5 years, 11 months ago
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Palmer Dabbelt <palmer@dabbelt.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
There is a newer version of this series
[PATCH v5 56/60] target/riscv: floating-point scalar move instructions
Posted by LIU Zhiwei 5 years, 11 months ago
Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/helper.h                   |  9 +++++
 target/riscv/insn32.decode              |  2 ++
 target/riscv/insn_trans/trans_rvv.inc.c | 47 +++++++++++++++++++++++++
 target/riscv/vector_helper.c            | 36 +++++++++++++++++++
 4 files changed, 94 insertions(+)

diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 41cecd266c..7a689a5c07 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1111,3 +1111,12 @@ DEF_HELPER_3(vmv_s_x_b, void, ptr, tl, env)
 DEF_HELPER_3(vmv_s_x_h, void, ptr, tl, env)
 DEF_HELPER_3(vmv_s_x_w, void, ptr, tl, env)
 DEF_HELPER_3(vmv_s_x_d, void, ptr, tl, env)
+
+DEF_HELPER_2(vfmv_f_s_b, i64, ptr, env)
+DEF_HELPER_2(vfmv_f_s_h, i64, ptr, env)
+DEF_HELPER_2(vfmv_f_s_w, i64, ptr, env)
+DEF_HELPER_2(vfmv_f_s_d, i64, ptr, env)
+DEF_HELPER_3(vfmv_s_f_b, void, ptr, i64, env)
+DEF_HELPER_3(vfmv_s_f_h, void, ptr, i64, env)
+DEF_HELPER_3(vfmv_s_f_w, void, ptr, i64, env)
+DEF_HELPER_3(vfmv_s_f_d, void, ptr, i64, env)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 7e1efeec05..bfdce0979c 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -557,6 +557,8 @@ viota_m         010110 . ..... 10000 010 ..... 1010111 @r2_vm
 vid_v           010110 . 00000 10001 010 ..... 1010111 @r1_vm
 vext_x_v        001100 1 ..... ..... 010 ..... 1010111 @r
 vmv_s_x         001101 1 00000 ..... 110 ..... 1010111 @r2
+vfmv_f_s        001100 1 ..... 00000 001 ..... 1010111 @r2rd
+vfmv_s_f        001101 1 00000 ..... 101 ..... 1010111 @r2
 
 vsetvli         0 ........... ..... 111 ..... 1010111  @r2_zimm
 vsetvl          1000000 ..... ..... 111 ..... 1010111  @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
index 7720ffecde..99cd45b0aa 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -2269,3 +2269,50 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
     }
     return false;
 }
+
+/* Floating-Point Scalar Move Instructions */
+typedef void (* gen_helper_vfmv_f_s)(TCGv_i64, TCGv_ptr, TCGv_env);
+static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
+{
+    if (vext_check_isa_ill(s, RVV)) {
+        TCGv_ptr src2;
+        gen_helper_vfmv_f_s fns[4] = {
+            gen_helper_vfmv_f_s_b, gen_helper_vfmv_f_s_h,
+            gen_helper_vfmv_f_s_w, gen_helper_vfmv_f_s_d
+        };
+
+        src2 = tcg_temp_new_ptr();
+        tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
+
+        fns[s->sew](cpu_fpr[a->rd], src2, cpu_env);
+
+        tcg_temp_free_ptr(src2);
+        return true;
+    }
+    return false;
+}
+
+typedef void (* gen_helper_vfmv_s_f)(TCGv_ptr, TCGv_i64, TCGv_env);
+static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
+{
+    if (vext_check_isa_ill(s, RVV | RVF) ||
+        vext_check_isa_ill(s, RVV | RVD)) {
+        TCGv_ptr dest;
+        TCGv_i64 src1;
+        gen_helper_vfmv_s_f fns[4] = {
+            gen_helper_vfmv_s_f_b, gen_helper_vfmv_s_f_h,
+            gen_helper_vfmv_s_f_w, gen_helper_vfmv_s_f_d
+        };
+
+        src1 = tcg_temp_new_i64();
+        dest = tcg_temp_new_ptr();
+        tcg_gen_addi_ptr(dest, cpu_env, vreg_ofs(s, a->rd));
+
+        fns[s->sew](dest, src1, cpu_env);
+
+        tcg_temp_free_i64(src1);
+        tcg_temp_free_ptr(dest);
+        return true;
+    }
+    return false;
+}
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 66ee69da99..3235c3fbe1 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4475,3 +4475,39 @@ GEN_VEXT_VMV_S_X(vmv_s_x_b, uint8_t, H1, clearb)
 GEN_VEXT_VMV_S_X(vmv_s_x_h, uint16_t, H2, clearh)
 GEN_VEXT_VMV_S_X(vmv_s_x_w, uint32_t, H4, clearl)
 GEN_VEXT_VMV_S_X(vmv_s_x_d, uint64_t, H8, clearq)
+
+/* Floating-Point Scalar Move Instructions */
+#define GEN_VEXT_VFMV_S_F(NAME, ETYPE, H, CLEAR_FN)                     \
+void HELPER(NAME)(void *vd, uint64_t s1, CPURISCVState *env)            \
+{                                                                       \
+    if (env->vl == 0) {                                                 \
+        return;                                                         \
+    }                                                                   \
+    *((ETYPE *)vd + H(0)) = s1;                                         \
+    CLEAR_FN(vd, 1, sizeof(ETYPE), env_archcpu(env)->cfg.vlen / 8);     \
+}
+GEN_VEXT_VFMV_S_F(vfmv_s_f_b, uint8_t, H1, clearb)
+GEN_VEXT_VFMV_S_F(vfmv_s_f_h, uint16_t, H2, clearh)
+GEN_VEXT_VFMV_S_F(vfmv_s_f_w, uint32_t, H4, clearl)
+GEN_VEXT_VFMV_S_F(vfmv_s_f_d, uint64_t, H8, clearq)
+
+uint64_t HELPER(vfmv_f_s_b)(void *vs2, CPURISCVState *env)
+{
+    return deposit64(-1ULL, 0, 8, *((uint8_t *)vs2 + H1(0)));
+}
+uint64_t HELPER(vfmv_f_s_h)(void *vs2, CPURISCVState *env)
+{
+    return deposit64(-1ULL, 0, 16, *((uint16_t *)vs2 + H2(0)));
+}
+uint64_t HELPER(vfmv_f_s_w)(void *vs2, CPURISCVState *env)
+{
+    return deposit64(-1ULL, 0, 32, *((uint32_t *)vs2 + H4(0)));
+}
+uint64_t HELPER(vfmv_f_s_d)(void *vs2, CPURISCVState *env)
+{
+    if (env->misa & RVD) {
+        return *((uint64_t *)vs2);
+    } else {
+        return deposit64(*((uint64_t *)vs2), 32, 32, 0xffffffff);
+    }
+}
-- 
2.23.0


Re: [PATCH v5 56/60] target/riscv: floating-point scalar move instructions
Posted by Richard Henderson 5 years, 11 months ago
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/helper.h                   |  9 +++++
>  target/riscv/insn32.decode              |  2 ++
>  target/riscv/insn_trans/trans_rvv.inc.c | 47 +++++++++++++++++++++++++
>  target/riscv/vector_helper.c            | 36 +++++++++++++++++++
>  4 files changed, 94 insertions(+)
> 
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 41cecd266c..7a689a5c07 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1111,3 +1111,12 @@ DEF_HELPER_3(vmv_s_x_b, void, ptr, tl, env)
>  DEF_HELPER_3(vmv_s_x_h, void, ptr, tl, env)
>  DEF_HELPER_3(vmv_s_x_w, void, ptr, tl, env)
>  DEF_HELPER_3(vmv_s_x_d, void, ptr, tl, env)
> +
> +DEF_HELPER_2(vfmv_f_s_b, i64, ptr, env)
> +DEF_HELPER_2(vfmv_f_s_h, i64, ptr, env)
> +DEF_HELPER_2(vfmv_f_s_w, i64, ptr, env)
> +DEF_HELPER_2(vfmv_f_s_d, i64, ptr, env)
> +DEF_HELPER_3(vfmv_s_f_b, void, ptr, i64, env)
> +DEF_HELPER_3(vfmv_s_f_h, void, ptr, i64, env)
> +DEF_HELPER_3(vfmv_s_f_w, void, ptr, i64, env)
> +DEF_HELPER_3(vfmv_s_f_d, void, ptr, i64, env)
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 7e1efeec05..bfdce0979c 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -557,6 +557,8 @@ viota_m         010110 . ..... 10000 010 ..... 1010111 @r2_vm
>  vid_v           010110 . 00000 10001 010 ..... 1010111 @r1_vm
>  vext_x_v        001100 1 ..... ..... 010 ..... 1010111 @r
>  vmv_s_x         001101 1 00000 ..... 110 ..... 1010111 @r2
> +vfmv_f_s        001100 1 ..... 00000 001 ..... 1010111 @r2rd
> +vfmv_s_f        001101 1 00000 ..... 101 ..... 1010111 @r2
>  
>  vsetvli         0 ........... ..... 111 ..... 1010111  @r2_zimm
>  vsetvl          1000000 ..... ..... 111 ..... 1010111  @r
> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
> index 7720ffecde..99cd45b0aa 100644
> --- a/target/riscv/insn_trans/trans_rvv.inc.c
> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
> @@ -2269,3 +2269,50 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
>      }
>      return false;
>  }
> +
> +/* Floating-Point Scalar Move Instructions */
> +typedef void (* gen_helper_vfmv_f_s)(TCGv_i64, TCGv_ptr, TCGv_env);
> +static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
> +{
> +    if (vext_check_isa_ill(s, RVV)) {
> +        TCGv_ptr src2;
> +        gen_helper_vfmv_f_s fns[4] = {
> +            gen_helper_vfmv_f_s_b, gen_helper_vfmv_f_s_h,
> +            gen_helper_vfmv_f_s_w, gen_helper_vfmv_f_s_d
> +        };
> +
> +        src2 = tcg_temp_new_ptr();
> +        tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
> +
> +        fns[s->sew](cpu_fpr[a->rd], src2, cpu_env);
> +
> +        tcg_temp_free_ptr(src2);
> +        return true;
> +    }
> +    return false;
> +}

SEW == MO_8 should raise illegal instruction exception.

Need a check for fp enabled.  Presumably

    if (s->mstatus_fs == 0 || !has_ext(s, RVF)) {
        return false;
    }

Need to mark_fs_dirty().

Like integer vmv.x.s, this can be done inline.  The nan-boxing is trivial as well.

For 0.8, we will have to validate the nan-boxing for SEW=MO_64 && !RVD.  That's
still not hard to do inline.



> +
> +typedef void (* gen_helper_vfmv_s_f)(TCGv_ptr, TCGv_i64, TCGv_env);
> +static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
> +{
> +    if (vext_check_isa_ill(s, RVV | RVF) ||
> +        vext_check_isa_ill(s, RVV | RVD)) {
> +        TCGv_ptr dest;
> +        TCGv_i64 src1;
> +        gen_helper_vfmv_s_f fns[4] = {
> +            gen_helper_vfmv_s_f_b, gen_helper_vfmv_s_f_h,
> +            gen_helper_vfmv_s_f_w, gen_helper_vfmv_s_f_d
> +        };
> +
> +        src1 = tcg_temp_new_i64();
> +        dest = tcg_temp_new_ptr();
> +        tcg_gen_addi_ptr(dest, cpu_env, vreg_ofs(s, a->rd));
> +
> +        fns[s->sew](dest, src1, cpu_env);
> +
> +        tcg_temp_free_i64(src1);
> +        tcg_temp_free_ptr(dest);
> +        return true;
> +    }
> +    return false;
> +}

Again, SEW == MO_8 is illegal.  Missing fp enable check.

I don't believe RVD without RVF is legal; you should not need to check for both.

Missing nan-boxing for SEW==MO_64 && FLEN==32 (!RVD).  Which I think should be
done here inline, so that the uint64_t passed to the helper is always correct.


r~

Re: [PATCH v5 56/60] target/riscv: floating-point scalar move instructions
Posted by LIU Zhiwei 5 years, 11 months ago

On 2020/3/15 12:39, Richard Henderson wrote:
> On 3/12/20 7:58 AM, LIU Zhiwei wrote:
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/helper.h                   |  9 +++++
>>   target/riscv/insn32.decode              |  2 ++
>>   target/riscv/insn_trans/trans_rvv.inc.c | 47 +++++++++++++++++++++++++
>>   target/riscv/vector_helper.c            | 36 +++++++++++++++++++
>>   4 files changed, 94 insertions(+)
>>
>> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
>> index 41cecd266c..7a689a5c07 100644
>> --- a/target/riscv/helper.h
>> +++ b/target/riscv/helper.h
>> @@ -1111,3 +1111,12 @@ DEF_HELPER_3(vmv_s_x_b, void, ptr, tl, env)
>>   DEF_HELPER_3(vmv_s_x_h, void, ptr, tl, env)
>>   DEF_HELPER_3(vmv_s_x_w, void, ptr, tl, env)
>>   DEF_HELPER_3(vmv_s_x_d, void, ptr, tl, env)
>> +
>> +DEF_HELPER_2(vfmv_f_s_b, i64, ptr, env)
>> +DEF_HELPER_2(vfmv_f_s_h, i64, ptr, env)
>> +DEF_HELPER_2(vfmv_f_s_w, i64, ptr, env)
>> +DEF_HELPER_2(vfmv_f_s_d, i64, ptr, env)
>> +DEF_HELPER_3(vfmv_s_f_b, void, ptr, i64, env)
>> +DEF_HELPER_3(vfmv_s_f_h, void, ptr, i64, env)
>> +DEF_HELPER_3(vfmv_s_f_w, void, ptr, i64, env)
>> +DEF_HELPER_3(vfmv_s_f_d, void, ptr, i64, env)
>> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
>> index 7e1efeec05..bfdce0979c 100644
>> --- a/target/riscv/insn32.decode
>> +++ b/target/riscv/insn32.decode
>> @@ -557,6 +557,8 @@ viota_m         010110 . ..... 10000 010 ..... 1010111 @r2_vm
>>   vid_v           010110 . 00000 10001 010 ..... 1010111 @r1_vm
>>   vext_x_v        001100 1 ..... ..... 010 ..... 1010111 @r
>>   vmv_s_x         001101 1 00000 ..... 110 ..... 1010111 @r2
>> +vfmv_f_s        001100 1 ..... 00000 001 ..... 1010111 @r2rd
>> +vfmv_s_f        001101 1 00000 ..... 101 ..... 1010111 @r2
>>   
>>   vsetvli         0 ........... ..... 111 ..... 1010111  @r2_zimm
>>   vsetvl          1000000 ..... ..... 111 ..... 1010111  @r
>> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
>> index 7720ffecde..99cd45b0aa 100644
>> --- a/target/riscv/insn_trans/trans_rvv.inc.c
>> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
>> @@ -2269,3 +2269,50 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
>>       }
>>       return false;
>>   }
>> +
>> +/* Floating-Point Scalar Move Instructions */
>> +typedef void (* gen_helper_vfmv_f_s)(TCGv_i64, TCGv_ptr, TCGv_env);
>> +static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
>> +{
>> +    if (vext_check_isa_ill(s, RVV)) {
>> +        TCGv_ptr src2;
>> +        gen_helper_vfmv_f_s fns[4] = {
>> +            gen_helper_vfmv_f_s_b, gen_helper_vfmv_f_s_h,
>> +            gen_helper_vfmv_f_s_w, gen_helper_vfmv_f_s_d
>> +        };
>> +
>> +        src2 = tcg_temp_new_ptr();
>> +        tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
>> +
>> +        fns[s->sew](cpu_fpr[a->rd], src2, cpu_env);
>> +
>> +        tcg_temp_free_ptr(src2);
>> +        return true;
>> +    }
>> +    return false;
>> +}
> SEW == MO_8 should raise illegal instruction exception.
I agree. But I didn't find a reference in Section 17.3 both in v0.7.1 
and v0.8.

Perhaps I should refer

"If the current SEW does not correspond to a supported IEEE floating-point
type, an illegal instruction exception is raised."(Section 14)


> Need a check for fp enabled.  Presumably
>
>      if (s->mstatus_fs == 0 || !has_ext(s, RVF)) {
>          return false;
>      }
>
> Need to mark_fs_dirty().
Yes, I should.
>
> Like integer vmv.x.s, this can be done inline.  The nan-boxing is trivial as well.
>
> For 0.8, we will have to validate the nan-boxing for SEW=MO_64 && !RVD.  That's
> still not hard to do inline.
>
I see it. Thanks.
>
>> +
>> +typedef void (* gen_helper_vfmv_s_f)(TCGv_ptr, TCGv_i64, TCGv_env);
>> +static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
>> +{
>> +    if (vext_check_isa_ill(s, RVV | RVF) ||
>> +        vext_check_isa_ill(s, RVV | RVD)) {
>> +        TCGv_ptr dest;
>> +        TCGv_i64 src1;
>> +        gen_helper_vfmv_s_f fns[4] = {
>> +            gen_helper_vfmv_s_f_b, gen_helper_vfmv_s_f_h,
>> +            gen_helper_vfmv_s_f_w, gen_helper_vfmv_s_f_d
>> +        };
>> +
>> +        src1 = tcg_temp_new_i64();
>> +        dest = tcg_temp_new_ptr();
>> +        tcg_gen_addi_ptr(dest, cpu_env, vreg_ofs(s, a->rd));
>> +
>> +        fns[s->sew](dest, src1, cpu_env);
There is a mistake here.

fns[s->sew](dest, cpu_fpr[a->rs1], cpu_env);
>> +
>> +        tcg_temp_free_i64(src1);
>> +        tcg_temp_free_ptr(dest);
>> +        return true;
>> +    }
>> +    return false;
>> +}
> Again, SEW == MO_8 is illegal.  Missing fp enable check.
>
> I don't believe RVD without RVF is legal; you should not need to check for both.
Reasonable.
>
> Missing nan-boxing for SEW==MO_64 && FLEN==32 (!RVD).  Which I think should be
> done here inline, so that the uint64_t passed to the helper is always correct.
I think all float registers have been NAN-boxed in QEMU target/riscv.

As float registers are  always 64bits.  If FLEN is 32, a float register 
has been NAN-boxed in FLW or VFMV.F.S

Should I NAN-boxed the float register explicitly here ?

Zhiwei
>
> r~


Re: [PATCH v5 56/60] target/riscv: floating-point scalar move instructions
Posted by Richard Henderson 5 years, 11 months ago
On 3/14/20 11:13 PM, LIU Zhiwei wrote:
>> SEW == MO_8 should raise illegal instruction exception.
> I agree. But I didn't find a reference in Section 17.3 both in v0.7.1 and v0.8.
> 
> Perhaps I should refer
> 
> "If the current SEW does not correspond to a supported IEEE floating-point
> type, an illegal instruction exception is raised."(Section 14)

Yes, that's the rule I was thinking of.

>> Missing nan-boxing for SEW==MO_64 && FLEN==32 (!RVD).  Which I think should be
>> done here inline, so that the uint64_t passed to the helper is always correct.
> I think all float registers have been NAN-boxed in QEMU target/riscv.
> 
> As float registers are  always 64bits.  If FLEN is 32, a float register has
> been NAN-boxed in FLW or VFMV.F.S
> 
> Should I NAN-boxed the float register explicitly here ?

Hmm, I see what you mean -- RVF is supposed to have already boxed all of the
values.  Except that it doesn't at the moment.  I remember now that we were
talking about this some months ago; I thought it had been taken care of, but
hasn't.

I think we should explicitly do it here, with a comment.


r~

Re: [PATCH v5 56/60] target/riscv: floating-point scalar move instructions
Posted by LIU Zhiwei 5 years, 10 months ago

On 2020/3/15 12:39, Richard Henderson wrote:
> On 3/12/20 7:58 AM, LIU Zhiwei wrote:
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/helper.h                   |  9 +++++
>>   target/riscv/insn32.decode              |  2 ++
>>   target/riscv/insn_trans/trans_rvv.inc.c | 47 +++++++++++++++++++++++++
>>   target/riscv/vector_helper.c            | 36 +++++++++++++++++++
>>   4 files changed, 94 insertions(+)
>>
>> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
>> index 41cecd266c..7a689a5c07 100644
>> --- a/target/riscv/helper.h
>> +++ b/target/riscv/helper.h
>> @@ -1111,3 +1111,12 @@ DEF_HELPER_3(vmv_s_x_b, void, ptr, tl, env)
>>   DEF_HELPER_3(vmv_s_x_h, void, ptr, tl, env)
>>   DEF_HELPER_3(vmv_s_x_w, void, ptr, tl, env)
>>   DEF_HELPER_3(vmv_s_x_d, void, ptr, tl, env)
>> +
>> +DEF_HELPER_2(vfmv_f_s_b, i64, ptr, env)
>> +DEF_HELPER_2(vfmv_f_s_h, i64, ptr, env)
>> +DEF_HELPER_2(vfmv_f_s_w, i64, ptr, env)
>> +DEF_HELPER_2(vfmv_f_s_d, i64, ptr, env)
>> +DEF_HELPER_3(vfmv_s_f_b, void, ptr, i64, env)
>> +DEF_HELPER_3(vfmv_s_f_h, void, ptr, i64, env)
>> +DEF_HELPER_3(vfmv_s_f_w, void, ptr, i64, env)
>> +DEF_HELPER_3(vfmv_s_f_d, void, ptr, i64, env)
>> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
>> index 7e1efeec05..bfdce0979c 100644
>> --- a/target/riscv/insn32.decode
>> +++ b/target/riscv/insn32.decode
>> @@ -557,6 +557,8 @@ viota_m         010110 . ..... 10000 010 ..... 1010111 @r2_vm
>>   vid_v           010110 . 00000 10001 010 ..... 1010111 @r1_vm
>>   vext_x_v        001100 1 ..... ..... 010 ..... 1010111 @r
>>   vmv_s_x         001101 1 00000 ..... 110 ..... 1010111 @r2
>> +vfmv_f_s        001100 1 ..... 00000 001 ..... 1010111 @r2rd
>> +vfmv_s_f        001101 1 00000 ..... 101 ..... 1010111 @r2
>>   
>>   vsetvli         0 ........... ..... 111 ..... 1010111  @r2_zimm
>>   vsetvl          1000000 ..... ..... 111 ..... 1010111  @r
>> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
>> index 7720ffecde..99cd45b0aa 100644
>> --- a/target/riscv/insn_trans/trans_rvv.inc.c
>> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
>> @@ -2269,3 +2269,50 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
>>       }
>>       return false;
>>   }
>> +
>> +/* Floating-Point Scalar Move Instructions */
>> +typedef void (* gen_helper_vfmv_f_s)(TCGv_i64, TCGv_ptr, TCGv_env);
>> +static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
>> +{
>> +    if (vext_check_isa_ill(s, RVV)) {
>> +        TCGv_ptr src2;
>> +        gen_helper_vfmv_f_s fns[4] = {
>> +            gen_helper_vfmv_f_s_b, gen_helper_vfmv_f_s_h,
>> +            gen_helper_vfmv_f_s_w, gen_helper_vfmv_f_s_d
>> +        };
>> +
>> +        src2 = tcg_temp_new_ptr();
>> +        tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
>> +
>> +        fns[s->sew](cpu_fpr[a->rd], src2, cpu_env);
>> +
>> +        tcg_temp_free_ptr(src2);
>> +        return true;
>> +    }
>> +    return false;
>> +}
> SEW == MO_8 should raise illegal instruction exception.
>
> Need a check for fp enabled.  Presumably
>
>      if (s->mstatus_fs == 0 || !has_ext(s, RVF)) {
>          return false;
>      }
Hi  Richard,

Two questions here. I don't find the answer in the specification.

1. Should  I check RVF if the instruction uses float register,  such as 
all float point instructions and some other instructions?

2. Should I check mstatus_fs if the instruction uses float registers, or 
just for instructions that write float point register?

Zhiwei

> Need to mark_fs_dirty().
>
> Like integer vmv.x.s, this can be done inline.  The nan-boxing is trivial as well.
>
> For 0.8, we will have to validate the nan-boxing for SEW=MO_64 && !RVD.  That's
> still not hard to do inline.
>
>
>
>> +
>> +typedef void (* gen_helper_vfmv_s_f)(TCGv_ptr, TCGv_i64, TCGv_env);
>> +static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
>> +{
>> +    if (vext_check_isa_ill(s, RVV | RVF) ||
>> +        vext_check_isa_ill(s, RVV | RVD)) {
>> +        TCGv_ptr dest;
>> +        TCGv_i64 src1;
>> +        gen_helper_vfmv_s_f fns[4] = {
>> +            gen_helper_vfmv_s_f_b, gen_helper_vfmv_s_f_h,
>> +            gen_helper_vfmv_s_f_w, gen_helper_vfmv_s_f_d
>> +        };
>> +
>> +        src1 = tcg_temp_new_i64();
>> +        dest = tcg_temp_new_ptr();
>> +        tcg_gen_addi_ptr(dest, cpu_env, vreg_ofs(s, a->rd));
>> +
>> +        fns[s->sew](dest, src1, cpu_env);
>> +
>> +        tcg_temp_free_i64(src1);
>> +        tcg_temp_free_ptr(dest);
>> +        return true;
>> +    }
>> +    return false;
>> +}
> Again, SEW == MO_8 is illegal.  Missing fp enable check.
>
> I don't believe RVD without RVF is legal; you should not need to check for both.
>
> Missing nan-boxing for SEW==MO_64 && FLEN==32 (!RVD).  Which I think should be
> done here inline, so that the uint64_t passed to the helper is always correct.
>
>
> r~


Re: [PATCH v5 56/60] target/riscv: floating-point scalar move instructions
Posted by Richard Henderson 5 years, 10 months ago
On 3/16/20 11:01 PM, LIU Zhiwei wrote:
> Two questions here. I don't find the answer in the specification.
> 
> 1. Should  I check RVF if the instruction uses float register,  such as all
> float point instructions and some other instructions?

I would think so, but even the 0.8 spec isn't clear.


> 2. Should I check mstatus_fs if the instruction uses float registers, or just
> for instructions that write float point register?

Definitely, just like the regular fp instructions.

This trap is how the kernel implements lazy fp context switching, so if you
allow access to fp when disabled you may be accessing values from a different
process.


r~