Re: [RFC v3 15/71] target/riscv: introduce more imm value modes in translator functions

[Richard Henderson]

On 8/6/20 3:46 AM, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
> 
> Immediate value in translator function is extended not only
> zero-extended and sign-extended but with more modes to be applicable
> with multiple formats of vector instructions.
> 
> * IMM_ZX:         Zero-extended
> * IMM_SX:         Sign-extended
> * IMM_TRUNC_SEW:  Truncate to log(SEW) bit
> * IMM_TRUNC_2SEW: Truncate to log(2*SEW) bit
> 
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 115 ++++++++++++++----------
>  1 file changed, 66 insertions(+), 49 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c 
> b/target/riscv/insn_trans/trans_rvv.inc.c
> index c2d0865bb9b..0a4dd875e96 100644
> --- a/target/riscv/insn_trans/trans_rvv.inc.c
> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
> @@ -1281,8 +1281,32 @@ static void tcg_gen_gvec_rsubs(unsigned vece, uint32_t 
> dofs, uint32_t aofs,
>  
>  GEN_OPIVX_GVEC_TRANS(vrsub_vx, rsubs)
>  
> +enum {
> +    IMM_ZX,         /* Zero-extended */
> +    IMM_SX,         /* Sign-extended */
> +    IMM_TRUNC_SEW,  /* Truncate to log(SEW) bits */
> +    IMM_TRUNC_2SEW, /* Truncate to log(2*SEW) bits */
> +};

Better to name the enumeration and use it...

> +
> +static int64_t extract_imm(DisasContext *s, uint32_t imm, int imm_mode)

... here.

> +{
> +    switch (imm_mode) {
> +    case IMM_ZX:
> +        return extract64(imm, 0, 5);
> +    case IMM_SX:
> +        return sextract64(imm, 0, 5);
> +    case IMM_TRUNC_SEW:
> +        return extract64(imm, 0, 5) & ((1 << (s->sew + 3)) - 1);
> +    case IMM_TRUNC_2SEW:
> +        return extract64(imm, 0, 5) & ((2 << (s->sew + 3)) - 1);

The extract is redundant with the &.

Alternately, put sew into the extract, like so:

    return extract64(imm, 0, s->sew + 3);
and
    return extract64(imm, 0, s->sew + 4);


> +    default:
> +        g_assert_not_reached();
> +        break;

Unreachable break.

>  static bool opivi_trans(uint32_t vd, uint32_t imm, uint32_t vs2, uint32_t vm,
> -                        gen_helper_opivx *fn, DisasContext *s, int zx)
> +                        gen_helper_opivx *fn, DisasContext *s, int imm_mode)

Use the enum.

>  static inline bool
>  do_opivi_gvec(DisasContext *s, arg_rmrr *a, GVecGen2iFn *gvec_fn,
> -              gen_helper_opivx *fn, int zx)
> +              gen_helper_opivx *fn, int imm_mode)

Use the enum.


r~