Re: [Qemu-devel] [PATCH v9 39/84] target/mips: Add emulation of nanoMIPS 32-bit branch instructions

[Richard Henderson]

On 08/16/2018 07:57 AM, Aleksandar Markovic wrote:
> +    } else {
> +        /* Conditional compact branch */
> +        TCGLabel *fs = gen_new_label();
> +        save_cpu_state(ctx, 0);

I will note that save_cpu_state(ctx, 0) is concerned about updating the BMASK
bits for branch delay slots, which nanomips does not have.  The function could
probably usefully be renamed so that's clearer.


> +    /* unconditional branch to register */
> +    tcg_gen_mov_tl(cpu_PC, btarget);
> +    if (ctx->base.singlestep_enabled) {
> +        save_cpu_state(ctx, 0);

Likewise.

> +    } else {
> +        /* Conditional compact branch */
> +        TCGLabel *fs = gen_new_label();
> +        save_cpu_state(ctx, 0);

Likewise.
>      case NM_MOVE_BALC:
> +        {
> +            TCGv t0 = tcg_temp_new();
> +            int32_t s = sextract32(ctx->opcode, 0, 1) << 21 |
> +                        extract32(ctx->opcode, 1, 20) << 1;
> +            rd = (extract32(ctx->opcode, 24, 1)) == 0 ? 4 : 5;
> +            rt = decode_gpr_gpr4_zero(extract32(ctx->opcode, 25, 1) << 3 |
> +                            extract32(ctx->opcode, 21, 3));
> +            gen_load_gpr(t0, rt);
> +            tcg_gen_mov_tl(cpu_gpr[rd], t0);
> +            gen_compute_branch(ctx, OPC_BGEZAL, 4, 0, 0, s, 0);
> +            tcg_temp_free(t0);
> +        }
>          break;
>      case NM_P_BAL:
> +        {
> +            int32_t s = sextract32(ctx->opcode, 0, 1) << 25 |
> +                        extract32(ctx->opcode, 1, 24) << 1;
> +
> +            if ((extract32(ctx->opcode, 25, 1)) == 0) {
> +                /* BC */
> +                gen_compute_branch(ctx, OPC_BEQ, 4, 0, 0, s, 0);
> +            } else {
> +                /* BALC */
> +                gen_compute_branch(ctx, OPC_BGEZAL, 4, 0, 0, s, 0);
> +            }
> +        }
>          break;
>      case NM_P_J:
> +        switch (extract32(ctx->opcode, 12, 4)) {
> +        case NM_JALRC:
> +        case NM_JALRC_HB:
> +            gen_compute_branch(ctx, OPC_JALR, 4, rs, rt, 0, 0);
> +            break;

All of these branches need to be using the new functions that you just added.
There are even more incorrect uses to follow.


r~