Re: [Qemu-devel] [PATCH v2 06/22] target-mips: split decode_opc_special* into *_r6 and *_legacy

[Aurelien Jarno]

On Wed, Jun 11, 2014 at 04:19:36PM +0100, Leon Alrae wrote:
> For better code readability and to avoid 'if' statements for all R6 and preR6
> instructions whose opcodes are the same - decode_opc_special* functions are
> split into functions with _r6 and _legacy suffixes.
> 
> *_r6 functions will contain instructions which were introduced in R6.
> *_legacy functions will contain instructions which were removed in R6.
> 
> Signed-off-by: Leon Alrae <address@hidden>
> ---
> v2:
> * imm contains shifted value
> ---
>  target-mips/translate.c |  227 
> +++++++++++++++++++++++++++++++++--------------
>  1 files changed, 159 insertions(+), 68 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index c67e92d..87bb6f7 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -14431,6 +14431,70 @@ static void gen_mipsdsp_accinsn(DisasContext *ctx, 
> uint32_t op1, uint32_t op2,
>  
>  /* End MIPSDSP functions. */
>  
> +static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    int rs, rt, rd;
> +    uint32_t op1;
> +
> +    rs = (ctx->opcode >> 21) & 0x1f;
> +    rt = (ctx->opcode >> 16) & 0x1f;
> +    rd = (ctx->opcode >> 11) & 0x1f;
> +
> +    op1 = MASK_SPECIAL(ctx->opcode);
> +    switch (op1) {
> +    case OPC_SELEQZ:
> +    case OPC_SELNEZ:
> +        gen_cond_move(ctx, op1, rd, rs, rt);
> +        break;
> +    default:            /* Invalid */
> +        MIPS_INVAL("special_r6");
> +        generate_exception(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +
> +static void decode_opc_special_legacy(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    int rs, rt, rd;
> +    uint32_t op1;
> +
> +    rs = (ctx->opcode >> 21) & 0x1f;
> +    rt = (ctx->opcode >> 16) & 0x1f;
> +    rd = (ctx->opcode >> 11) & 0x1f;
> +
> +    op1 = MASK_SPECIAL(ctx->opcode);
> +    switch (op1) {
> +    case OPC_MOVN:         /* Conditional move */
> +    case OPC_MOVZ:
> +        check_insn(ctx, ISA_MIPS4 | ISA_MIPS32 |
> +                   INSN_LOONGSON2E | INSN_LOONGSON2F);
> +        gen_cond_move(ctx, op1, rd, rs, rt);
> +        break;
> +    case OPC_MFHI:          /* Move from HI/LO */
> +    case OPC_MFLO:
> +        gen_HILO(ctx, op1, rs & 3, rd);
> +        break;
> +    case OPC_MTHI:
> +    case OPC_MTLO:          /* Move to HI/LO */
> +        gen_HILO(ctx, op1, rd & 3, rs);
> +        break;
> +    case OPC_MOVCI:
> +        check_insn(ctx, ISA_MIPS4 | ISA_MIPS32);
> +        if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> +            check_cp1_enabled(ctx);
> +            gen_movci(ctx, rd, rs, (ctx->opcode >> 18) & 0x7,
> +                      (ctx->opcode >> 16) & 1);
> +        } else {
> +            generate_exception_err(ctx, EXCP_CpU, 1);
> +        }
> +        break;
> +    default:            /* Invalid */
> +        MIPS_INVAL("special_legacy");
> +        generate_exception(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +
>  static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
>  {
>      int rs, rt, rd, sa;
> @@ -14463,18 +14527,6 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>              break;
>          }
>          break;
> -    case OPC_MOVN:         /* Conditional move */
> -    case OPC_MOVZ:
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
> -        check_insn(ctx, ISA_MIPS4 | ISA_MIPS32 |
> -                   INSN_LOONGSON2E | INSN_LOONGSON2F);
> -        gen_cond_move(ctx, op1, rd, rs, rt);
> -        break;
> -    case OPC_SELEQZ:
> -    case OPC_SELNEZ:
> -        check_insn(ctx, ISA_MIPS32R6);
> -        gen_cond_move(ctx, op1, rd, rs, rt);
> -        break;
>      case OPC_ADD ... OPC_SUBU:
>          gen_arith(ctx, op1, rd, rs, rt);
>          break;
> @@ -14529,16 +14581,6 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>      case OPC_TNE:
>          gen_trap(ctx, op1, rs, rt, -1);
>          break;
> -    case OPC_MFHI:          /* Move from HI/LO */
> -    case OPC_MFLO:
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
> -        gen_HILO(ctx, op1, rs & 3, rd);
> -        break;
> -    case OPC_MTHI:
> -    case OPC_MTLO:          /* Move to HI/LO */
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
> -        gen_HILO(ctx, op1, rd & 3, rs);
> -        break;
>      case OPC_PMON:          /* Pmon entry point, also R4010 selsl */
>  #ifdef MIPS_STRICT_STANDARD
>          MIPS_INVAL("PMON / selsl");
> @@ -14568,18 +14610,6 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>          /* Treat as NOP. */
>          break;
>  
> -    case OPC_MOVCI:
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
> -        check_insn(ctx, ISA_MIPS4 | ISA_MIPS32);
> -        if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> -            check_cp1_enabled(ctx);
> -            gen_movci(ctx, rd, rs, (ctx->opcode >> 18) & 0x7,
> -                      (ctx->opcode >> 16) & 1);
> -        } else {
> -            generate_exception_err(ctx, EXCP_CpU, 1);
> -        }
> -        break;
> -
>  #if defined(TARGET_MIPS64)
>          /* MIPS64 specific opcodes */
>      case OPC_DSLL:
> @@ -14661,14 +14691,29 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>          gen_muldiv(ctx, op1, 0, rs, rt);
>          break;
>  #endif
> +    default:
> +        if (ctx->insn_flags & ISA_MIPS32R6) {
> +            decode_opc_special_r6(env, ctx);
> +        } else {
> +            decode_opc_special_legacy(env, ctx);
> +        }
> +    }
> +}
> +
> +static void decode_opc_special2_r6(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    uint32_t op1;
> +
> +    op1 = MASK_SPECIAL2(ctx->opcode);
> +    switch (op1) {
>      default:            /* Invalid */
> -        MIPS_INVAL("special");
> +        MIPS_INVAL("special2_r6");
>          generate_exception(ctx, EXCP_RI);
>          break;
>      }
>  }
>  
> -static void decode_opc_special2(CPUMIPSState *env, DisasContext *ctx)
> +static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>  {
>      int rs, rt, rd;
>      uint32_t op1;
> @@ -14681,14 +14726,30 @@ static void decode_opc_special2(CPUMIPSState *env, 
> DisasContext *ctx)
>      switch (op1) {
>      case OPC_MADD ... OPC_MADDU: /* Multiply and add/sub */
>      case OPC_MSUB ... OPC_MSUBU:
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
>          check_insn(ctx, ISA_MIPS32);
>          gen_muldiv(ctx, op1, rd & 3, rs, rt);
>          break;
>      case OPC_MUL:
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
>          gen_arith(ctx, op1, rd, rs, rt);
>          break;
> +    default:            /* Invalid */
> +        MIPS_INVAL("special2_legacy");
> +        generate_exception(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +
> +static void decode_opc_special2(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    int rs, rt, rd;
> +    uint32_t op1;
> +
> +    rs = (ctx->opcode >> 21) & 0x1f;
> +    rt = (ctx->opcode >> 16) & 0x1f;
> +    rd = (ctx->opcode >> 11) & 0x1f;
> +
> +    op1 = MASK_SPECIAL2(ctx->opcode);
> +    switch (op1) {
>      case OPC_CLO:
>      case OPC_CLZ:
>          check_insn(ctx, ISA_MIPS32);
> @@ -14732,30 +14793,78 @@ static void decode_opc_special2(CPUMIPSState *env, 
> DisasContext *ctx)
>          gen_loongson_integer(ctx, op1, rd, rs, rt);
>          break;
>  #endif
> +    default:
> +        if (ctx->insn_flags & ISA_MIPS32R6) {
> +            decode_opc_special2_r6(env, ctx);
> +        } else {
> +            decode_opc_special2_legacy(env, ctx);
> +        }
> +    }
> +}
> +
> +static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    int rs, rt;
> +    uint32_t op1;
> +    int16_t imm;
> +
> +    rs = (ctx->opcode >> 21) & 0x1f;
> +    rt = (ctx->opcode >> 16) & 0x1f;
> +    imm = (int16_t)ctx->opcode >> 7;
> +
> +    op1 = MASK_SPECIAL3(ctx->opcode);
> +    switch (op1) {
> +    case R6_OPC_SC:
> +        gen_st_cond(ctx, op1, rt, rs, imm);
> +        break;
> +    case R6_OPC_LL:
> +        gen_ld(ctx, op1, rt, rs, imm);
> +        break;
> +    default:            /* Invalid */
> +        MIPS_INVAL("special3_r6");
> +        generate_exception(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +
> +static void decode_opc_special3_legacy(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    uint32_t op1;
> +#if defined(TARGET_MIPS64)
> +    int rd = (ctx->opcode >> 11) & 0x1f;
> +    int rs = (ctx->opcode >> 21) & 0x1f;
> +    int rt = (ctx->opcode >> 16) & 0x1f;
> +#endif
> +
> +    op1 = MASK_SPECIAL3(ctx->opcode);
> +    switch (op1) {
> +#if defined(TARGET_MIPS64)
> +    case OPC_DDIV_G_2E ... OPC_DDIVU_G_2E:
> +    case OPC_DMULT_G_2E ... OPC_DMULTU_G_2E:
> +    case OPC_DMOD_G_2E ... OPC_DMODU_G_2E:
> +        check_insn(ctx, INSN_LOONGSON2E);
> +        gen_loongson_integer(ctx, op1, rd, rs, rt);
> +        break;
> +#endif
>      default:            /* Invalid */
> -        MIPS_INVAL("special2");
> +        MIPS_INVAL("special3_legacy");
>          generate_exception(ctx, EXCP_RI);
>          break;
>      }
>  }
> +
>  static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx)
>  {
>      int rs, rt, rd, sa;
>      uint32_t op1, op2;
> -    int16_t imm;
>  
>      rs = (ctx->opcode >> 21) & 0x1f;
>      rt = (ctx->opcode >> 16) & 0x1f;
>      rd = (ctx->opcode >> 11) & 0x1f;
>      sa = (ctx->opcode >> 6) & 0x1f;
> -    imm = (int16_t)ctx->opcode;
>  
>      op1 = MASK_SPECIAL3(ctx->opcode);
>      switch (op1) {
> -    case R6_OPC_LL:
> -        check_insn(ctx, ISA_MIPS32R6);
> -        gen_ld(ctx, op1, rt, rs, imm >> 7);
> -        break;
>      case OPC_EXT:
>      case OPC_INS:
>          check_insn(ctx, ISA_MIPS32R2);
> @@ -15060,19 +15169,6 @@ static void decode_opc_special3(CPUMIPSState *env, 
> DisasContext *ctx)
>              break;
>          }
>          break;
> -    case R6_OPC_SC: /* OPC_DMOD_G_2E */
> -        if (ctx->insn_flags & ISA_MIPS32R6) {
> -            gen_st_cond(ctx, op1, rt, rs, imm >> 7);
> -        } else {
> -#if defined(TARGET_MIPS64)
> -            check_insn(ctx, INSN_LOONGSON2E);
> -            gen_loongson_integer(ctx, op1, rd, rs, rt);
> -#else
> -            /* Invalid in MIPS32 */
> -            generate_exception(ctx, EXCP_RI);
> -#endif
> -        }
> -        break;
>  #if defined(TARGET_MIPS64)
>      case OPC_DEXTM ... OPC_DEXT:
>      case OPC_DINSM ... OPC_DINS:
> @@ -15086,13 +15182,6 @@ static void decode_opc_special3(CPUMIPSState *env, 
> DisasContext *ctx)
>          op2 = MASK_DBSHFL(ctx->opcode);
>          gen_bshfl(ctx, op2, rt, rd);
>          break;
> -    case OPC_DDIV_G_2E ... OPC_DDIVU_G_2E:
> -    case OPC_DMULT_G_2E ... OPC_DMULTU_G_2E:
> -    case OPC_DMODU_G_2E:
> -        check_insn_opc_removed(ctx, ISA_MIPS32R6);
> -        check_insn(ctx, INSN_LOONGSON2E);
> -        gen_loongson_integer(ctx, op1, rd, rs, rt);
> -        break;
>      case OPC_ABSQ_S_QH_DSP:
>          op2 = MASK_ABSQ_S_QH(ctx->opcode);
>          switch (op2) {
> @@ -15322,9 +15411,11 @@ static void decode_opc_special3(CPUMIPSState *env, 
> DisasContext *ctx)
>          break;
>  #endif
>      default:            /* Invalid */
> -        MIPS_INVAL("special3");
> -        generate_exception(ctx, EXCP_RI);
> -        break;
> +        if (ctx->insn_flags & ISA_MIPS32R6) {
> +            decode_opc_special3_r6(env, ctx);
> +        } else {
> +            decode_opc_special3_legacy(env, ctx);
> +        }
>      }
>  }
>  

Reviewed-by: Aurelien Jarno <address@hidden>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net