Re: [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6

[Yongbok Kim <yongbok.kim@imgtec.com>]

On 29/05/2015 13:33, Leon Alrae wrote:
> On 27/05/2015 14:29, Yongbok Kim wrote:
>> @@ -2143,7 +2146,8 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>>          t1 = tcg_const_tl(pc_relative_pc(ctx));
>>          gen_op_addr_add(ctx, t0, t0, t1);
>>          tcg_temp_free(t1);
>> -        tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
>> +        tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ |
>> +                           ctx->default_tcg_memop_mask);
>>          gen_store_gpr(t0, rt);
>>          opn = "ldpc";
>>          break;
>> @@ -2152,22 +2156,26 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>>          t1 = tcg_const_tl(pc_relative_pc(ctx));
>>          gen_op_addr_add(ctx, t0, t0, t1);
>>          tcg_temp_free(t1);
>> -        tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL);
>> +        tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL |
>> +                           ctx->default_tcg_memop_mask);
>>          gen_store_gpr(t0, rt);
>>          opn = "lwpc";
>>          break;
> 
> As I can see in other places you skipped load/store instructions not
> present in R6 spec (like pre-R6 DSP or microMIPS loads/stores), which
> probably is fine. However, IIUC these two instructions LWPC and LDPC are
> from mips16 ASE, so probably you want to skip them as well? (note that
> for R6 we’ve got R6_OPC_LWPC and R6_OPC_LDPC and they are naturally
> aligned).

Nice finding!

> 
> Apart from that,
> 
> Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>
> 
> BTW these OPC_LWPC and OPC_LDPC are very confusing... I presume these
> fake instructions were created in order to reuse gen_ldst() function for
> mips16 M16_OPC_LWPC and I64_LDPC instructions. It would be nice to clean
> this up at some point (the same as OPC_JALRC).
> 
> Leon
> 

Yes, indeed. I have removed those fictional branch instructions similar to
these before, still we have more to remove.

Regards,
Yongbok