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

[Richard Henderson]

On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <address@hidden>
> ---
>  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~