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

[Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Fri, Jul 22, 2016 at 11:05:54AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> 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 <nikunj@linux.vnet.ibm.com>
>> >> ---
>> >>  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                                                             \
>
> Hrm.. you're using C99 designated initializers, which means I'm pretty
> sure you can just leave out the pad field, since you don't care about
> it's value.  That should avoid the need for an ifdef.

Sure then, will update accordingly.

Regards,
Nikunj