Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[Nikunj A Dadhania]

David Gibson <address@hidden> writes:

> [ Unknown signature status ]
> On Mon, Jul 18, 2016 at 10:35:17PM +0530, Nikunj A Dadhania wrote:
>> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
>> indirect opcode table and corresponding parsing routines.
>> 
>> EO (11:12) Expanded opcode field
>> Formats: XX1
>> 
>> EO (11:15) Expanded opcode field
>> Formats: VX, X, XX2
>> 
>> Signed-off-by: Nikunj A Dadhania <address@hidden>
>> ---
>>  target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
>>  target-ppc/translate_init.c | 103 
>> ++++++++++++++++++++++++++++++++------------
>>  2 files changed, 136 insertions(+), 40 deletions(-)
>> 
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 6c5a4a6..733d68d 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -40,6 +40,7 @@
>>  /* Include definitions for instructions classes and implementations flags */
>>  //#define PPC_DEBUG_DISAS
>>  //#define DO_PPC_STATISTICS
>> +//#define PPC_DUMP_CPU
>>  
>>  #ifdef PPC_DEBUG_DISAS
>>  #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
>> @@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, 
>> PPC_NONE)
>>  #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)    
>>   \
>>  GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
>>  
>> +#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)   
>>   \
>> +GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
>> +
>>  typedef struct opcode_t {
>> -    unsigned char opc1, opc2, opc3;
>> +    unsigned char opc1, opc2, opc3, opc4;
>>  #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
>> -    unsigned char pad[5];
>> +    unsigned char pad[4];
>>  #else
>> -    unsigned char pad[1];
>> +    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */
>
> IIUC the point here is to align entries to the wordsize.  If the
> worsize is 32-bit you shouldn't need any extra padding here.

You are right, the reason I had added this here is to keep the code
clean in the GEN_OPCODEx

#define GEN_OPCODE(name, op1, op2, op3, op4, invl, _typ, _typ2)   \
{                                                                  \
    .opc1 = op1,                                                   \
    .opc2 = op2,                                                   \
    .opc3 = op3,                                                   \
    .opc4 = 0xff,                                                  \
#if HOST_LONG_BITS == 64                                           \
    .pad  = { 0, },                                                \
#endif                                                             \
    .handler = {                                                   \
        .inval1  = invl,                                           \
        .type = _typ,                                              \
        .type2 = _typ2,                                            \
        .handler = &gen_##name,                                    \
        .oname = stringify(name),                                  \
    },                                                             \
    .oname = stringify(name),                                      \
}

I am fine with both the approach, but thought of the current one as
cleaner, we would waste 4byte per opcode in 32-bit case.

Regards
Nikunj