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

[LIU Zhiwei]

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_...@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~