Re: [PATCH v8 58/62] target/riscv: floating-point scalar move instructions

[LIU Zhiwei]

On 2020/6/5 5:32, Richard Henderson wrote:
> On 5/21/20 2:44 AM, LIU Zhiwei wrote:
> > +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)) {
> > +        unsigned int ofs = (8 << s->sew);
> > +        unsigned int len = 64 - ofs;
> > +        TCGv_i64 t_nan;
> > +
> > +        vec_element_loadi(s, cpu_fpr[a->rd], a->rs2, 0);
> > +        /* NaN-box f[rd] as necessary for SEW */
> > +        if (len) {
> > +            t_nan = tcg_const_i64(UINT64_MAX);
> > +            tcg_gen_deposit_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> > +                                t_nan, ofs, len);
> > +            tcg_temp_free_i64(t_nan);
> > +        }
> I guess this is correct, but it looks odd.
>
> I would have done
>
>      unsigned int len = 8 << s->sew;
>
>      if (len < 64) {
>          tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
>                          MAKE_64BIT_MASK(len, 64 - len));
>      }
>
> and to match, I might use
>
>      MAKE_64BIT_MASK(32, 32)
>
> in trans_vfmv_s_f.
Thanks. It's much clear this way.

Zhiwei
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~