Re: [Qemu-devel] [PATCH v5 32/76] target/mips: Add emulation of misc nanoMIPS instructions (p_lsx)

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +static void gen_p_lsx(DisasContext *ctx, int rd, int rs, int rt)
> +{
> +    TCGv t0, t1;
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +
> +    gen_load_gpr(t0, rs);
> +    gen_load_gpr(t1, rt);
> +
> +    if ((extract32(ctx->opcode, 6, 1)) == 1) {
> +        /* PP.LSXS instructions require shifting */
> +        switch (extract32(ctx->opcode, 7, 4)) {
> +        case NM_LHXS:
> +        case NM_SHXS:
> +        case NM_LHUXS:
> +            tcg_gen_shli_tl(t0, t0, 1);
> +            break;
> +        case NM_LWXS:
> +        case NM_SWXS:
> +        case NM_LWC1XS:
> +        case NM_SWC1XS:
> +            tcg_gen_shli_tl(t0, t0, 2);
> +            break;
> +        case NM_LDC1XS:
> +        case NM_SDC1XS:
> +            tcg_gen_shli_tl(t0, t0, 3);
> +            break;
> +        }
> +    }
> +    gen_op_addr_add(ctx, t0, t0, t1);
> +
> +    switch (extract32(ctx->opcode, 7, 4)) {
> +    case NM_LBX:
> +        tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx,
> +                           MO_SB);
> +        gen_store_gpr(t0, rd);
> +        break;
> +    case NM_LHX:
> +    /*case NM_LHXS:*/
> +        tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx,
> +                           MO_TESW);

Elsewhere in translate you use ctx->default_tcg_memop_mask.

You probably should tidy this up with a table or another switch that computes
the TCGMemOp for the size and sign of the operation.  Then you don't have to
modify quite so many places fixing this.  Something like

    unsigned opc = extract32(ctx->opcode, 7, 4);
    TCGMemOp mop;

    switch (opc) {
    case NM_LBX:
        mop = MO_SB;
        break;
    ...
    }
    mop |= ctx->default_tcg_memop_mask;

    if (extract32(ctx->opcode, 6, 1)) {
        tcg_gen_shli_tl(t0, t0, mop & MO_SIZE);
    }
    gen_op_addr_add(ctx, t0, t0, t1);

    switch (opc) {
    case NM_LBX: // and all other integer loads
       tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, mop);
       gen_store_gpr(t0, rd);
       break;
    case NM_SBX: // and all other integer stores
       gen_load_gpr(t1, td);
       tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, mop);
       break;
    case NM_LWC1X: // and all other fp ops
       if (ctx->CP0_Config1...) {
          ...
       }
    }


r~