Re: [RFC v4 18/70] target/riscv: rvv-1.0: stride load and store instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.chang@sifive.com wrote:
>  /*
> + * Vector unit-stride, strided, unit-stride segment, strided segment
> + * store check function.
> + *
> + * Rules to be checked here:
> + *   1. EMUL must within the range: 1/8 <= EMUL <= 8. (Section 7.3)
> + *   2. Destination vector register number is multiples of EMUL.
> + *      (Section 3.3.2, 7.3)
> + *   3. The EMUL setting must be such that EMUL * NFIELDS ≤ 8. (Section 7.8)
> + *   4. Vector register numbers accessed by the segment load or store
> + *      cannot increment past 31. (Section 7.8)
> + */
> +static bool vext_check_store(DisasContext *s, int vd, int nf, uint8_t eew)
> +{
> +    int8_t emul = ctzl(eew) - (s->sew + 3) + s->lmul;

I think eew should be passed as log, as MemOp and MO_{8,16,32,64} constants.
This is already the scale used by sew.

> +#define GEN_VEXT_TRANS(NAME, EEW, SEQ, ARGTYPE, OP, CHECK)   \
> +static bool trans_##NAME(DisasContext *s, arg_##ARGTYPE * a) \
> +{                                                            \
> +    if (CHECK(s, a, EEW)) {                                  \
> +        return OP(s, a, SEQ);                                \
> +    }                                                        \
> +    return false;                                            \

That also makes EEW and SEQ identical, so that...

> +GEN_VEXT_TRANS(vlse8_v,  8,  0, rnfvm, ld_stride_op, ld_stride_check)
> +GEN_VEXT_TRANS(vlse16_v, 16, 1, rnfvm, ld_stride_op, ld_stride_check)
> +GEN_VEXT_TRANS(vlse32_v, 32, 2, rnfvm, ld_stride_op, ld_stride_check)
> +GEN_VEXT_TRANS(vlse64_v, 64, 3, rnfvm, ld_stride_op, ld_stride_check)

... this does not need to replicate those constants.

> -#define GEN_VEXT_LD_ELEM(NAME, MTYPE, ETYPE, H, LDSUF)     \
> +#define GEN_VEXT_LD_ELEM(NAME, ETYPE, H, LDSUF)            \
>  static void NAME(CPURISCVState *env, abi_ptr addr,         \
>                   uint32_t idx, void *vd, uintptr_t retaddr)\
>  {                                                          \
> -    MTYPE data;                                            \
> +    ETYPE data;                                            \
>      ETYPE *cur = ((ETYPE *)vd + H(idx));                   \
>      data = cpu_##LDSUF##_data_ra(env, addr, retaddr);      \
>      *cur = data;                                           \
>  }                                                          \

Since there's no extension to be done between MTYPE and ETYPE anymore, you can
also drop the "data" variable completely and store directly into *cur.


r~