Re: [Qemu-devel] [PATCH v8 52/87] target/mips: Add handling of branch delay slots for nanoMIPS

[Aleksandar Markovic]

> From: Aleksandar Markovic <address@hidden>
> Sent: Monday, August 13, 2018 7:53 PM
> 
> Subject: [PATCH v8 52/87] target/mips: Add handling of branch delay slots for 
> nanoMIPS
> 
> From: Matthew Fortune <address@hidden>
> 
> ISA mode bit (LSB of address) is no longer required but is also
> masked to allow for tools transition. The flag has_isa_mode has the
> key role in the implementation.
> 
> Signed-off-by: Yongbok Kim <address@hidden>
> Signed-off-by: Aleksandar Markovic <address@hidden>
> Signed-off-by: Stefan Markovic <address@hidden>
> ---
>  target/mips/translate.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 3282fca..e4427e4 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1471,6 +1471,7 @@ typedef struct DisasContext {
>      bool mrp;
>      bool nan2008;
>      bool abs2008;
> +    bool has_isa_mode;
>  } DisasContext;
> 
>  #define DISAS_STOP       DISAS_TARGET_0
> @@ -4674,7 +4675,7 @@ static void gen_compute_branch (DisasContext *ctx, 
> uint32_t opc,
> 
>      if (blink > 0) {
>          int post_delay = insn_bytes + delayslot_size;
> -        int lowbit = !!(ctx->hflags & MIPS_HFLAG_M16);
> +        int lowbit = ctx->has_isa_mode && !!(ctx->hflags & MIPS_HFLAG_M16);
> 
>          tcg_gen_movi_tl(cpu_gpr[blink],
>                          ctx->base.pc_next + post_delay + lowbit);
> @@ -11171,7 +11172,7 @@ static void gen_compute_compact_branch(DisasContext 
> *ctx, uint32_t opc,
>      int bcond_compute = 0;
>      TCGv t0 = tcg_temp_new();
>      TCGv t1 = tcg_temp_new();
> -    int m16_lowbit = (ctx->hflags & MIPS_HFLAG_M16) != 0;
> +    int m16_lowbit = ctx->has_isa_mode && ((ctx->hflags & MIPS_HFLAG_M16) != 
> 0);
> 
>      if (ctx->hflags & MIPS_HFLAG_BMASK) {
>  #ifdef MIPS_DEBUG_DISAS
> @@ -24970,6 +24971,7 @@ static void 
> mips_tr_init_disas_context(DisasContextBase *dcbase, > CPUState *cs)
>      ctx->mrp = (env->CP0_Config5 >> CP0C5_MRP) & 1;
>      ctx->nan2008 = (env->active_fpu.fcr31 >> FCR31_NAN2008) & 1;
>      ctx->abs2008 = (env->active_fpu.fcr31 >> FCR31_ABS2008) & 1;
> +    ctx->has_isa_mode = ((env->CP0_Config3 >> CP0C3_MMAR) & 0x7) < 3;
>      restore_cpu_state(env, ctx);
>  #ifdef CONFIG_USER_ONLY
>          ctx->mem_idx = MIPS_HFLAG_UM;
> --
> 2.7.4
> 

This patch should be renamed to reflect its real content. Also, the code 
segments in question should be re-examined in follow-up clean ups. That said, 
it isn't still show-stopper:

Reviewed-by: Aleksandar Markovic <address@hidden>