Re: [Qemu-devel] [PATCH v3 04/40] target/mips: Add decode_nanomips_opc() function

[Aleksandar Markovic]

> From: Richard Henderson <address@hidden>
> Sent: Thursday, July 19, 2018 6:39 PM
> On 07/19/2018 05:54 AM, Stefan Markovic wrote:
> >          decode_opc(env, ctx);
> >      } else if (ctx->insn_flags & ASE_MICROMIPS) {
> > -        ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> > -        insn_bytes = decode_micromips_opc(env, ctx);
> > +        if (env->insn_flags & ISA_NANOMIPS32) {
>
> Be consistent and use ctx->insn_flags.
>
> > +            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> > +            insn_bytes = decode_nanomips_opc(env, ctx);
> > +        } else {
> > +            ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> > +            insn_bytes = decode_micromips_opc(env, ctx);
> > +        }
> >      } else if (ctx->insn_flags & ASE_MIPS16) {
> >          ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
>
> Do you really want to nest nanoMIPS within microMIPS?
>
> I would have thought a better structure was
>
>   } else if (ctx->insn_flags & ISA_NANOMIPS32) {
>       ...
>   } else if (ctx->insn_flags & ASE_MICROMIPS) {
>       ...
>   } else if (ctx->insn_flags & ASE_MIPS16) {
>
>
> r~

Hi, Richard,

This will be fixed in the way you described in v4.

Aleksandar