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~

Reply via email to