Re: [Qemu-devel] [PATCH v6 38/77] target/mips: Add emulation of DSP ASE for nanoMIPS - part 1

[Aleksandar Markovic]

> 
> From: Stefan Markovic <address@hidden>
> Sent: Thursday, August 2, 2018 4:16 PM
> Subject: [PATCH v6 38/77] target/mips: Add emulation of DSP ASE for nanoMIPS 
> - part 1
> 
> From: Stefan Markovic <address@hidden>
> 
> Add emulation of DSP ASE instructions for nanoMIPS - part 1.
> 
> Signed-off-by: Aleksandar Markovic <address@hidden>
> Signed-off-by: Stefan Markovic <address@hidden>
> ---
>  target/mips/translate.c | 579 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 579 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c

> +    case OPC_ADDQH_R_PH:
> +        switch (extract32(ctx->opcode, 10, 1)) {
> +        case 0:
> +            /* ADDQH_PH */
> +            gen_helper_addqh_ph(v1_t, v1_t, v2_t);
> +            gen_store_gpr(v1_t, ret);
> +            break;

check_dsp(ctx) or check_dspr2(ctx) missing, whatever is needed. Here and in 
many places in this patch.

Also, constants OPC_ADDQH_R_W should not be used, but their corresponding 
NM_XXX, which should be defined in one of preceding patches for DSP opcodes.

> +    case OPC_APPEND:
> +        {
> +            gen_load_gpr(t0, rs);
> +
> +            if (rd != 0) {
> +                tcg_gen_deposit_tl(cpu_gpr[rt], t0, cpu_gpr[rt], rd, 32 - 
> rd);
> +            }
> +            tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
> +        }
> +        break;

Indentation not needed, checkdsp()/checkdspr2().

> +    case OPC_SHRA_R_W:
> +        {
> +            tcg_gen_movi_tl(t0, rd);
> +
> +            check_dsp(ctx);
> +            gen_helper_shra_r_w(v1_t, t0, v1_t);
> +            gen_store_gpr(v1_t, rt);
> +        }
> +        break;

Indentation not needed, checkdsp()/checkdspr2().

> +    case OPC_SHRA_R_PH:
> +        {
> +            tcg_gen_movi_tl(t0, rd >> 1);
> +
> +            switch (extract32(ctx->opcode, 10, 1)) {
> +            case 0:
> +                /* SHRA_PH */
> +                check_dsp(ctx);
> +                gen_helper_shra_ph(v1_t, t0, v1_t);
> +                break;
> +                gen_store_gpr(v1_t, rt);
> +            case 1:
> +                /* SHRA_R_PH */
> +                check_dsp(ctx);
> +                gen_helper_shra_r_ph(v1_t, t0, v1_t);
> +                gen_store_gpr(v1_t, rt);
> +                break;
> +            }
> +        }
> +        break;

Indentation not needed.

> +    case OPC_SHLL_S_PH:
> +        {
> +            tcg_gen_movi_tl(t0, rd >> 1);
> +
> +            switch (extract32(ctx->opcode, 10, 2)) {
> +            case 0:
> +                /* SHLL_PH */
> +                check_dsp(ctx);
> +                gen_helper_shll_ph(v1_t, t0, v1_t, cpu_env);
> +                gen_store_gpr(v1_t, rt);
> +                break;
> +            case 2:
> +                /* SHLL_S_PH */
> +                check_dsp(ctx);
> +                gen_helper_shll_s_ph(v1_t, t0, v1_t, cpu_env);
> +                gen_store_gpr(v1_t, rt);
> +                break;
> +            }
> +        }
> +        break;

Missing default case, indentation not needed.

>  static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx)
>  {
>      uint16_t insn;
> @@ -17587,6 +18160,12 @@ static int decode_nanomips_32_48_opc(CPUMIPSState 
> *env, > DisasContext *ctx)
>          case NM_POOL32A0:
>              gen_pool32a0_nanomips_insn(env, ctx);
>              break;
> +        case NM_POOL32A5:
> +            {
> +                int32_t op1 = (ctx->opcode >> 3) & 0x7F;

extract32().

Aleksandar M.