Re: [Qemu-devel] [PATCH v3 21/40] target/mips: Implement DSP ASE support for nanoMIPS

[Richard Henderson]

On 07/19/2018 05:54 AM, Stefan Markovic wrote:
> +    case OPC_APPEND:
> +    {
> +        TCGv t0;
> +
> +        t0 = tcg_temp_new();
> +        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;
> +    case OPC_MODSUB:

Indentation.  Probably should hoist t0 to the top of the function anyway.

> +    case OPC_SHILO:
> +    {
> +        TCGv t0;
> +        TCGv t1;
> +        t0 = tcg_temp_new();
> +        t1 = tcg_temp_new();
> +
> +        int16_t imm = (ctx->opcode >> 16) & 0x3F;
> +
> +        tcg_gen_movi_tl(t0, rd >> 3);
> +        tcg_gen_movi_tl(t1, imm);
> +
> +        gen_helper_shilo(t0, t1, cpu_env);
> +    }
> +    break;
> +    case OPC_MULEQ_S_W_PHL:

Indentation.  Shadowing outer t1 (and t0 if you move the one above).
And for future cleanup, helper_shilo is much easier to implement inline.

> +    case OPC_SHLL_S_W:
> +    {
> +        TCGv t0;
> +        t0 = tcg_temp_new();
> +        tcg_gen_movi_tl(t0, rd);
> +
> +        check_dsp(ctx);
> +        gen_helper_shll_s_w(cpu_gpr[rt], t0, v1_t, cpu_env);
> +        break;
> +    }
> +    break;
> +    case OPC_REPL_PH:
> +    check_dsp(ctx);
> +    {

Indentation.  I won't mention any more, but please fix them all.

> -    NM_LWUX     = 0x07,
> -    NM_SWC1X    = 0x0b,
> -    NM_SDC1X    = 0x0f,
> +    NM_LWUX     = 0x07,
> +    NM_SWC1X    = 0x0b,
> +    NM_SDC1X    = 0x0f,

What has changed here?  Actually, looking further down in the patch, something
really odd -- and large -- has happened.  Did you rearrange functions or
something?  Whatever it is, it should be folded back into the patch that
introduced them.


r~