Re: [Qemu-devel] [PATCH v3 2/8] target/mips: Add all MXU opcodes

[Janeczek, Craig]

To clarify, if MXU is present (a cpu defined to have MXU instructions is 
selected) we will never run any of the other special2 commands? That would 
actually simplify the implementation.

Also I understand the want for clean code, I appreciate the comments.

-----Original Message-----
From: Aleksandar Markovic <address@hidden> 
Sent: Tuesday, August 28, 2018 3:35 PM
To: Janeczek, Craig <address@hidden>; address@hidden
Cc: address@hidden
Subject: Re: [PATCH v3 2/8] target/mips: Add all MXU opcodes

> From: Janeczek, Craig <address@hidden>
> Sent: Tuesday, August 28, 2018 8:54 PM
>
> Subject: RE: [PATCH v3 2/8] target/mips: Add all MXU opcodes
>
> I will re-work each of the mxu_gen_<ins> functions to check for MXUEN and 
> jump over the implementation of the instruction if not enabled.
> 
> I would like to clarify the structure of the switch statement before 
> implementing it.
>
> I was originally planning on checking if there was a MXU hit and MXUEN was 
> set, in that case use the MXU instruction, else use the original switch. This 
> will not work as MXUEN is in a TCGv and cant be used in an if statement at 
> that level. That is why I plan on putting the MXUEN check in each of the 
> mxu_gen_<ins> functions.
> 
> What should the re-worked switch statement look like which runs the 
> mxu_gen_<ins> functions?

I didn't study these code segments in depth, but at this moment it seems to me 
that the cleanest organization would be to leave current 
decode_opc_special2_legacy(env, ctx); as is, and, however, to change the part 
with its invocation:

    case OPC_SPECIAL2:
        decode_opc_special2_legacy(env, ctx);
        break;
to

    case OPC_SPECIAL2:
        if (<MXU is present>) {
                decode_opc_special2_mxu(env, ctx);
        } else if (<LOONGSON_2F>) {
                decode_opc_special2_legacy(env, ctx);
        }
        break;

where decode_opc_special2_mxu(env, ctx); is the function that you will write.

Or something along these lines...

But I may be mistaken regrding details...

Again, overall, this series is very good. We are arguing about some technical 
details, out of which many are, frankly, unimportant. I just want to tell you 
that don't misunderstand my and Richard's critiques - we want this code in 
QEMU, just want it to be nice, clean, and correct, and we definitely appreciate 
your work very much!