Re: [RFC v4 36/70] target/riscv: rvv-1.0: floating-point move instruction

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
>  static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f *a)
>  {
>      if (require_rvv(s) &&
> +        has_ext(s, RVF) &&
>          vext_check_isa_ill(s) &&
>          require_align(a->rd, 1 << s->lmul) &&
>          (s->sew != 0)) {
> +        TCGv_i64 t1 = tcg_temp_local_new_i64();
> +        /* NaN-box f[rs1] */
> +        do_nanbox(s, t1, cpu_fpr[a->rs1]);

Don't you need to check

  s->sew == MO_64 ? has_ext(s, RVD) : has_ext(s, RVF)

?

It might be worth folding that into it's own helper function, which also
incorporates the s->sew != 0 check.  E.g.

static bool require_rvf(Disascontext *s)
{
    switch (s->sew) {
    case MO_16:
    case MO_32:
        return has_ext(s, RVF);
    case MO_64:
        return has_ext(s, RVD);
    default:
        return false;
    }
}

> +        TCGv_i64 t1 = tcg_temp_local_new_i64();
> +        /* NaN-box f[rs1] */
> +        do_nanbox(s, t1, cpu_fpr[a->rs1]);
> +
>          if (s->vl_eq_vlmax) {
>              tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
> -                                 MAXSZ(s), MAXSZ(s), cpu_fpr[a->rs1]);
> +                                 MAXSZ(s), MAXSZ(s), t1);
>              mark_vs_dirty(s);
>          } else {
>              TCGv_ptr dest;

Recall that local temps get written to the stack at branches.

You should avoid the local temp by computing do_nanbox on both arms of this IF.
 In the else branch, do_nanbox should be after the brcond.


r~