Re: [Qemu-devel] [PATCH v3 18/40] target/mips: Add emulation of nanoMIPS 32-bit load and store instructions

[Richard Henderson]

On 07/25/2018 08:46 AM, Aleksandar Markovic wrote:
> Hello, Richard. Sorry for bothering you. One more question.
> 
>> On 07/19/2018 05:54 AM, Stefan Markovic wrote:
>>> +        case NM_ADDIUGP_B:
>>> +            gen_arith_imm(ctx, OPC_ADDIU, rt, 28, u);
>>> +            break;
>>
>> Use gen_op_addr_add, since behaves_like('DADDIU[GP.B]').
> 
> Did you perhaps mean an implementation similar to this would be appropriate:
> 
> case NM_ADDIUGP_B:
>     if (rt != 0) { 
>         uint32_t offset = extract32(ctx->opcode, 0, 18); 
>         if (offset == 0) { 
>             gen_load_gpr(cpu_gpr[rt], 28); 
>         } else { 
>             TCGv t0; 
>             t0 = tcg_temp_new(); 
>             tcg_gen_movi_tl(t0, offset); 
>             gen_op_addr_add(ctx, cpu_gpr[rt], cpu_gpr[28], t0); 
>             tcg_temp_free(t0); 
>         } 
>     }
>     break;
> 
> (this is like NM_ADDIUGP_W implementation)

I have suggested in the past (during v1 or v2 review?) creating

static void gen_op_addr_addi(DisasContext *ctx, TCGv ret, TCGv base,
                             target_long ofs)
{
    tcg_gen_addi_tl(ret, base, ofs);
#ifdef TARGET_MIPS64
    if (ctx->hflags & MIPS_HFLAG_AWRAP) {
        tcg_gen_ext32s_i64(ret, ret);
    }
#endif
}

so that

(1) You need not locally maintain the tcg temporary for offset
    at each such instance,
(2) The special case for offset == 0 is handled automatically
    within tcg_gen_addi_tl.
(3) You do not forget, as you just did here, that the extension
    for AWRAP must happen even for offset == 0.


r~