On 20/06/2014 21:50, Aurelien Jarno wrote:
> The patch subject is a bit misleading, as it also includes the AUI family.
Thanks for pointing this out.
>> +#if defined(TARGET_MIPS64)
>> + case R6_OPC_LDPC: /* bits 18 and 19 are part of immediate */
>> + case R6_OPC_LDPC + (1 << 16):
>> + case R6_OPC_LDPC + (2 << 16):
>> + case R6_OPC_LDPC + (3 << 16):
>> + check_mips_64(ctx);
>> + offset = (((int32_t)ctx->opcode << 14)) >> 11;
>
> This will overflow the 32-bits type. I guess you want:
>
> offset = (((int32_t)ctx->opcode << 13)) >> 10;
I think original code is correct (LDPC offset's size is 18 bits so it
won't overflow). However, I just noticed that the comment is misleading
(there should be 'bits 16 and 17' instead of 'bits 18 and 19').
> I do wonder if we shouldn't use sextract32() instead of open coding that
> now that it is available:
>
> offset = sextract32(ctx->opcode, 0, 19) << 3;
This looks better, thanks for the suggestion (but since the offset's
size is 18, third argument will be 18, not 19).
>> + addr = addr_add(ctx, (ctx->pc & ~0x7), offset);
>
> Why do we need to mask the low 3 bits of the PC? It doesn't appear in
> the manual version I have (MD00087 version 6.00).
It doesn't appear in LDPC pseudo-code, but few lines below there is a
restriction: "LDPC is naturally aligned, by specification".
For load doubleword we need to make the address aligned to 8-byte boundary.
You can also refer to MIPS64 Volume-I (MD00083 version 6.01):
5.1.3.1 PC relative loads (Release 6)
"LDPC: Loads a 64-bit doubleword from a PC relative address, formed by
adding the PC, aligned to 8-bytes by masking off the low 3 bits, to a
sign-extended 18-bit immediate, shifted left by 3 bits, for a 21-bit span."
>> +#if defined(TARGET_MIPS64)
>> + case OPC_DAHI:
>> + check_insn(ctx, ISA_MIPS32R6);
>> + check_mips_64(ctx);
>> + if (rs != 0) {
>> + tcg_gen_addi_i64(cpu_gpr[rs], cpu_gpr[rs], (int64_t)imm <<
>> 32);
>
> Small nitpicking: even if it is guarded by #ifdef, in theory the _tl
> type should be used there, to match the register type.
I'll correct it.
Thanks,
Leon
