Re: [Qemu-devel] [PATCH] RISCV: support riscv vector extension 0.7.1

[LIU Zhiwei]

On 2019/12/28 9:14, Richard Henderson wrote:
> On 12/25/19 8:36 PM, LIU Zhiwei wrote:
> > struct {
> >
> >          uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
> >          target_ulong vxrm;
> >          target_ulong vxsat;
> >          target_ulong vl;
> >          target_ulong vstart;
> >          target_ulong vtype;
> >      } vext;
> >
> > Is it OK?
> I don't think there's a good reason for the vext structure -- I would drop
> that.  Otherwise it looks good.
>
> > However, there are still some differences from SVE.
> >
> > 1)cpu_env must be used as a parameter for helper function.
> >
> >      The helpers need  use env->vext.vl and env->vext.vstart.  Thus it will be
> > difficult to use out of line tcg_gen_gvec_ool.
> Sure.  That's also true of any of the fp operations, which will want to
> accumulate ieee exceptions.
>
> See tcg_gen_gvec_*_ptr(), which allows you to pass in cpu_env.
Thanks. The tcg_gen_gvec_*_ptr is good.
> > 2）simd_desc is not proper.
> >
> >      I also need to transfer some members of DisasContext to helpers.
> >
> >      (Data, Vlmax, Mlen) is my current choice. Vlmax is the num of elements of
> > this operation, so it will defined as ctx->lmul * ctx->vlen / ctx->sew;
> The oprsz & maxsz parameters to tcg_gen_gvec_* should be given (ctx->lmul *
> ctx->vlen).  The sew parameter should be implied by the helper function called,
> each helper function using a different type.  Therefore vlmax can be trivially
> computed within the helper from oprsz / sizeof(type).
It's clear that the oprsz & maxsz paramenters should be given (ctx->lmul 
* ctx->vlen) for tcg_gen_gvec_add.

However It's not clear when use tcg_gen_gvec_*_ptr or tcg_gen_gvec_ool. 
I think the meaning of oprsz is the
the bits of active elements.  Therefore , oprsz is  8 * env->vext.vl in 
RISC-V and it can't be fetched  from
TB_FLAGS like SVE.

Probably oprsz field will be not be used in RISC-V vector extension.
> > Data is reserved to expand.  Mlen is mask length for one elment, so it will
> > defined as ctx->sew/ctx->lmul. As with Mlen, a active element will
> >
> > be selected by
> >
> >      static inline int vext_elem_mask(void *v0, int mlen, int index)
> >      {
> >          int idx = (index * mlen) / 8;
> >          int pos = (index * mlen) % 8;
> >
> >          return (v0[idx] >> pos) & 0x1;
> >      }
> >
> >      So I may have to implement vext_desc instead of use the simd_desc, which
> > will be another redundant. Maybe a better way to mask elements?
> I think you will want to define your own vext_desc, building upon simd_desc,
> such that lg2(mlen) is passed in the first N bits of simd_data.
Good. It's a good way to use the tcg_gen_gvec_*_ptr or tcg_gen_gvec_ool API.

Best Regards,
Zhiwei
> r~