On 05/24/2010 09:19 AM, Nathan Froyd wrote: > +enum { > + OPC_ADD_S = FOP(0, FMT_S),
Adding the enumeration is good. Naming the enumeration and using it in appropriate places is even better. > @@ -5937,8 +6031,8 @@ static void gen_farith (DisasContext *ctx, uint32_t op1, > enum { BINOP, CMPOP, OTHEROP } optype = OTHEROP; > uint32_t func = ctx->opcode & 0x3f; > > - switch (ctx->opcode & FOP(0x3f, 0x1f)) { > - case FOP(0, 16): > + switch (opc) { > + case OPC_ADD_S: For instance, "opc" would seem to be a good candidate for a variable to be switched to the enumeration type. ... Except that I can't seem to find the definition of "opc" at this point in patch 3? It looks like the argument "op1" should be what's used here. Is this a case of patches being split incorrectly? r~