Re: [Qemu-devel] [PATCH v5 26/76] target/mips: Add emulation of some common nanoMIPS 32-bit instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> +    case NM_ADDIUPC:
> +        if (rt != 0) {
> +            int32_t offset = sextract32(ctx->opcode, 0, 1) << 21 |
> +                             extract32(ctx->opcode, 1, 20) << 1;
> +            target_long addr = addr_add(ctx, ctx->base.pc_next + 4, offset);
> +            tcg_gen_movi_tl(cpu_gpr[rt], (int32_t)addr);

addr_add has already done any required sign-extend.
Including an extra one is wrong for nanomips64.

> +        case NM_SEQI:
> +            {
> +                TCGv t0 = tcg_temp_new();
> +                TCGv t1 = tcg_temp_new();
> +                TCGv t2 = tcg_temp_local_new();
> +
> +                gen_load_gpr(t0, rs);
> +                tcg_gen_movi_tl(t1, extract32(ctx->opcode, 0, 12));
> +                tcg_gen_setcond_tl(TCG_COND_EQ, t2, t0, t1);
> +                gen_store_gpr(t2, rt);
> +
> +                tcg_temp_free(t0);
> +                tcg_temp_free(t1);
> +                tcg_temp_free(t2);
> +            }
> +            break;

You only need one temporary.

  TCGv t0 = tcg_temp_new();
  int imm = extract32(ctx->opcode, 0, 12);

  gen_load_gpr(t0, rs);
  tcg_gen_setcondi_tl(TCG_COND_EQ, t0, t0, imm);
  gen_store_gpr(t0, rt);

  tcg_temp_free(t0);

Do not use tcg_temp_local_new unless you really need it in order to cross a
basic block boundary (branch or label).

> +                    } else if (rt == 0 && shift == 6) {
> +                        /* SYNC */
> +                        check_insn(ctx, ISA_MIPS2);

Cut and paste error from elsewhere?
Why would you need this ISA check?


r~