Re: [Qemu-devel] [PATCH 07/18] target/mips: convert to DisasJumpType

[Richard Henderson]

On 04/20/2018 08:55 AM, Emilio G. Cota wrote:
> +#define DISAS_STOP       DISAS_TARGET_0
> +#define DISAS_EXCP       DISAS_TARGET_1
> +#define DISAS_EXIT       DISAS_TARGET_2

DISAS_EXCP should be unused and therefore not exist.

> @@ -8301,7 +8298,7 @@ static void gen_cp0 (CPUMIPSState *env, DisasContext 
> *ctx, uint32_t opc, int rt,
>                  check_insn(ctx, ISA_MIPS2);
>                  gen_helper_eret(cpu_env);
>              }
> -            ctx->bstate = BS_EXCP;
> +            ctx->is_jmp = DISAS_EXCP;

Should be DISAS_EXIT -- helper_eret has changed both PC and hflags.

> @@ -8316,7 +8313,7 @@ static void gen_cp0 (CPUMIPSState *env, DisasContext 
> *ctx, uint32_t opc, int rt,
>              generate_exception_end(ctx, EXCP_RI);
>          } else {
>              gen_helper_deret(cpu_env);
> -            ctx->bstate = BS_EXCP;
> +            ctx->is_jmp = DISAS_EXCP;

Likewise.

>          }
>          break;
>      case OPC_WAIT:
> @@ -8331,7 +8328,7 @@ static void gen_cp0 (CPUMIPSState *env, DisasContext 
> *ctx, uint32_t opc, int rt,
>          save_cpu_state(ctx, 1);
>          ctx->pc -= 4;
>          gen_helper_wait(cpu_env);
> -        ctx->bstate = BS_EXCP;
> +        ctx->is_jmp = DISAS_EXCP;

Should be DISAS_NORETURN, since helper_wait exits via raise_exception.

> -    if (cs->singlestep_enabled && ctx.bstate != BS_BRANCH) {
> -        save_cpu_state(&ctx, ctx.bstate != BS_EXCP);
> +    if (cs->singlestep_enabled && ctx.is_jmp != DISAS_NORETURN) {
> +        save_cpu_state(&ctx, ctx.is_jmp != DISAS_EXCP);

Should be != DISAS_EXIT here, since for exit we have ensured that PC has
already been updated.  C.f. the switch below...

> +        case DISAS_EXIT:
>              tcg_gen_exit_tb(0);
>              break;

Otherwise,
Reviewed-by: Richard Henderson <address@hidden>


r~