Re: [Qemu-devel] [PATCH v5 19/76] target/mips: Add emulation of nanoMIPS 16-bit arithmetic instructions

[Aleksandar Markovic]

Hi, Richard.

> > +    case NM_P16_A2:
> > +        switch (extract32(ctx->opcode, 3, 1)) {
> > +        case NM_ADDIUR2:
> > +            rd = extract32(ctx->opcode, 0, 3) << 2;
> > +            gen_arith_imm(ctx, OPC_ADDIU, rt, rs, rd);
> > +            break;
> > +        case NM_P_ADDIURS5:
> > +            rt = extract32(ctx->opcode, 5, 5);
> > +            if (rt != 0) {
> > +                rs = (sextract32(ctx->opcode, 4, 1) << 3) |
> > +                      extract32(ctx->opcode, 0, 3);
> > +                /* s = sign_extend( s[3] . s[2:0] , from_nbits = 4)*/
> > +                gen_arith_imm(ctx, OPC_ADDIU, rt, rt, rs);
> > +            }
> > +            break;
> > +        }
> 
> Now, these re-uses of RD and RS variables are misleading.
> These are immediates that you are extracting, not register numbers.
> 
> I suggest a "target_long imm;" at the top of the function to handle all such
> that will be required when this function is filled out.

gen_arith_imm() and similar functions have "int" as the type of offset/imm 
parameter (in this case the last parameter) - shouldn't "int" be more 
appropriate type for local variable than "target_long"?

Aleksandar M.