Re: [PATCH v2 7/7] target/mips: Convert Loongson [D]MULT[U].G opcodes to decodetree

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 31/8/23 21:29, Richard Henderson wrote:
> On 8/31/23 13:30, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Convert the following opcodes to decodetree:
>>
>> - MULT.G - multiply 32-bit signed integers
>> - MULTU.G - multiply 32-bit unsigned integers
>> - DMULT.G - multiply 64-bit signed integers
>> - DMULTU.G - multiply 64-bit unsigned integers
>>
>> Now that all opcodes from the extension have been converted, we
>> can remove completely gen_loongson_integer() and its 2 calls in
>> decode_opc_special2_legacy() and decode_opc_special3_legacy().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
>> +    if (is_double) {
>> +        if (TARGET_LONG_BITS != 64) {
>> +            return false;
>> +        }
>> +        check_mips_64(s);
>> +    }
> 
> This preserves existing behaviour vs
> 
>> -#if defined(TARGET_MIPS64)
>> -    case OPC_DMULT_G_2E:
>> -    case OPC_DMULT_G_2F:
>> -        tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
>> -        break;
>> -    case OPC_DMULTU_G_2E:
>> -    case OPC_DMULTU_G_2F:
>> -        tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
>> -        break;
>> -#endif
> 
> this ifdef.  But it doesn't seem quite right.
> 
> It's a behaviour change between qemu-system-mips and qemu-system-mips64 
> for the same cpu. Returning false allows another insn to match instead.  
> But we have identified the insn, it just isn't legal.

Indeed.

> Anyway, aren't all of these loongson cpus 64-bit?

The Loongson-1 cores family is 32-bit but AFAICT it doesn't implement
the Loongson Extension, which appeared with the Loongson-2 family,
which is 64-bit. QEMU only implement the Loongson-2/3 families, both
64-bit.

I'll post a cleanup patch on top, since this series is already fully
reviewed.

Thanks!

Phil.