Re: [PATCH v11 08/46] target/mips: Add emulation of nanoMIPS 16-bit branch instructions

[Faraz Shahbazker]

Hi Philippe,

On 5/29/21 7:50 PM, Philippe Mathieu-Daudé wrote:
> On 5/29/21 3:52 PM, Philippe Mathieu-Daudé wrote:
> > On Mon, Aug 20, 2018 at 8:17 PM Aleksandar Markovic
> > <aleksandar.markovic@rt-rk.com> wrote:
> > > From: Stefan Markovic <smarkovic@wavecomp.com>
> > > +    case OPC_BPOSGE32:
> > > +        tcg_gen_andi_tl(t0, cpu_dspctrl, 0x3F);
> > > +        bcond_compute = 1;
> > > +        btgt = ctx->base.pc_next + insn_bytes + offset;
>
> I think this opcode never worked correctly.
>
> Per the "MIPS® Architecture Extension: nanoMIPS32 DSP Technical
> Reference Manual — Revision 0.04" p. 88 "BPOSGE32C":
>
>    "First, the offset argument is left-shifted by one bit to form
>     a 17-bit signed integer value."
>
> The caller, decode_nanomips_32_48_opc(), doesn't shift the offset:
>
>      case NM_BPOSGE32C:
>          check_dsp_r3(ctx);
>          {
>              int32_t imm = extract32(ctx->opcode, 1, 13) |
>                            extract32(ctx->opcode, 0, 1) << 13;
>
>              gen_compute_branch_nm(ctx, OPC_BPOSGE32, 4, -1, -2,
>                                    imm);
>          }
>          break;

I agree that the left-shift is missing. One must also note that the text 
in the nanoMIPS32 DSP manual is incorrect. This was most probably 
copy-pasted from the microMIPS DSP manual. The effective offset for 
nanoMIPS DSP is supposed to be 15-bit signed, not 17-bit.

- farazS