Re: [Qemu-devel] [PATCH v4 06/55] target/mips: Add emulation of misc nanoMIPS 16-bit instructions

[Richard Henderson]

On 07/24/2018 10:31 AM, Aleksandar Markovic wrote:
> +    /* make sure instructions are on a halfword boundary */
> +    if (ctx->base.pc_next & 0x1) {
> +        TCGv tmp = tcg_const_tl(ctx->base.pc_next);
> +        tcg_gen_st_tl(tmp, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));
> +        tcg_temp_free(tmp);
> +        return 2;
> +    }

You've lost the actual raising of the exception since v3.

> +    case NM_P16_MV:
> +        {
> +            int rt1 = NANOMIPS_EXTRACT_RD5(ctx->opcode);
> +            if (rt1 != 0) {
> +                /* MOVE */
> +                int rs1 = NANOMIPS_EXTRACT_RS5(ctx->opcode);
> +                gen_arith(ctx, OPC_ADDU, rt1, rs1, 0);

Why are you creating rt1 and rs1 variables?
I see you took my shadowed variables comment to heart, and renamed them, but
why not just overwrite the contents of the existing outer variable?

> +        case NM_P_ADDIURS5:
> +            {
> +                int rt1 = extract32(ctx->opcode, 5, 5);

Likewise.

> +    case NM_P16_4X4:
> +        {
> +            int rt1 = (extract32(ctx->opcode, 9, 1) << 3) |
> +                      extract32(ctx->opcode, 5, 3);
> +            int rs1 = (extract32(ctx->opcode, 4, 1) << 3) |
> +                      extract32(ctx->opcode, 0, 3);

Likewise.


r~