Re: [PATCH v6 57/61] target/riscv: floating-point scalar move instructions

[LIU Zhiwei]

On 2020/3/28 11:44, Richard Henderson wrote:
> On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> > +/* Floating-Point Scalar Move Instructions */
> > +static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
> > +{
> > +    if (!s->vill && has_ext(s, RVF) &&
> > +        (s->mstatus_fs != 0) && (s->sew != 0)) {
> > +#ifdef HOST_WORDS_BIGENDIAN
> > +        int ofs = vreg_ofs(s, a->rs2) + ((7 >> s->sew) << s->sew);
> > +#else
> > +        int ofs = vreg_ofs(s, a->rs2);
> > +#endif
> Use endian_ofs from patch 55.
Yes, I forgot it.
> > +        switch (s->sew) {
> > +        case MO_8:
> > +            tcg_gen_ld8u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> > +            tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> > +                            0xffffffffffffff00ULL);
> > +            break;
> MO_8 should be illegal.
Yes, it has been checked in s->sew != 0.
> > +        case MO_16:
> > +            tcg_gen_ld16u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> > +            tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> > +                            0xffffffffffff0000ULL);
> > +            break;
> > +        case MO_32:
> > +            tcg_gen_ld32u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> > +            tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> > +                            0xffffffff00000000ULL);
> > +            break;
> > +        default:
> > +            if (has_ext(s, RVD)) {
> > +                tcg_gen_ld_i64(cpu_fpr[a->rd], cpu_env, ofs);
> > +            } else {
> > +                tcg_gen_ld32u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> > +                tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> > +                                0xffffffff00000000ULL);
> > +            }
> > +            break;
> Maybe better with MO_64 and default: g_assert_not_reached().
>
> > +static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
> > +{
> > +    if (!s->vill && has_ext(s, RVF) && (s->sew != 0)) {
> > +        TCGv_ptr dest;
> > +        TCGv_i64 src1;
> > +        static gen_helper_vfmv_s_f * const fns[3] = {
> > +            gen_helper_vfmv_s_f_h,
> > +            gen_helper_vfmv_s_f_w,
> > +            gen_helper_vfmv_s_f_d
> You shouldn't need to duplicate the vmv_s_x_* helpers.
All these helpers will be away in v7.

Zhiwei
> r~