Re: [Qemu-devel] [PATCH 10/21] target-mips: move CLO, DCLO, CLZ, DCLZ, SDBBP and free special2 in R6

[Aurelien Jarno]

On Fri, May 30, 2014 at 03:47:48PM +0100, Leon Alrae wrote:
> Also consider OPC_SPIM instruction as deleted in R6 because it is overlaping
> with MIPS32R6 SDBBP.
> 
> Signed-off-by: Leon Alrae <address@hidden>
> ---
>  disas/mips.c            |    5 ++
>  target-mips/translate.c |  120 
> ++++++++++++++++++++++++-----------------------
>  2 files changed, 66 insertions(+), 59 deletions(-)
> 
> diff --git a/disas/mips.c b/disas/mips.c
> index db8d9ee..8cb9032 100644
> --- a/disas/mips.c
> +++ b/disas/mips.c
> @@ -1217,6 +1217,11 @@ const struct mips_opcode mips_builtin_opcodes[] =
>     them first.  The assemblers uses a hash table based on the
>     instruction name anyhow.  */
>  /* name,    args,    match,      mask,       pinfo,                  
> membership */
> +{"clz",     "U,s",      0x00000050, 0xfc0007ff, WR_d|RD_s,            0, 
> I32R6},
> +{"clo",     "U,s",      0x00000051, 0xfc0007ff, WR_d|RD_s,            0, 
> I32R6},
> +{"dclz",    "U,s",      0x00000052, 0xfc0007ff, WR_d|RD_s,            0, 
> I64R6},
> +{"dclo",    "U,s",      0x00000053, 0xfc0007ff, WR_d|RD_s,            0, 
> I64R6},
> +{"sdbbp",   "B",        0x0000000e, 0xfc00003f, TRAP,                 0, 
> I32R6},
>  {"mul",     "d,s,t",    0x00000098, 0xfc0007ff, WR_d|RD_s|RD_t,       0, 
> I32R6},
>  {"muh",     "d,s,t",    0x000000d8, 0xfc0007ff, WR_d|RD_s|RD_t,       0, 
> I32R6},
>  {"mulu",    "d,s,t",    0x00000099, 0xfc0007ff, WR_d|RD_s|RD_t,       0, 
> I32R6},
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 03df2a2..2e94375 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -233,6 +233,12 @@ enum {
>      R6_OPC_DMOD   = OPC_DDIV   | (3 << 6),
>      R6_OPC_DDIVU  = OPC_DDIVU  | (2 << 6),
>      R6_OPC_DMODU  = OPC_DDIVU  | (3 << 6),
> +
> +    R6_OPC_CLZ      = 0x10 | OPC_SPECIAL,
> +    R6_OPC_CLO      = 0x11 | OPC_SPECIAL,
> +    R6_OPC_DCLZ     = 0x12 | OPC_SPECIAL,
> +    R6_OPC_DCLO     = 0x13 | OPC_SPECIAL,
> +    R6_OPC_SDBBP    = 0x0e | OPC_SPECIAL,
>  };
>  
>  /* Multiplication variants of the vr54xx. */
> @@ -3267,19 +3273,23 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
>      gen_load_gpr(t0, rs);
>      switch (opc) {
>      case OPC_CLO:
> +    case R6_OPC_CLO:
>          gen_helper_clo(cpu_gpr[rd], t0);
>          opn = "clo";
>          break;
>      case OPC_CLZ:
> +    case R6_OPC_CLZ:
>          gen_helper_clz(cpu_gpr[rd], t0);
>          opn = "clz";
>          break;
>  #if defined(TARGET_MIPS64)
>      case OPC_DCLO:
> +    case R6_OPC_DCLO:
>          gen_helper_dclo(cpu_gpr[rd], t0);
>          opn = "dclo";
>          break;
>      case OPC_DCLZ:
> +    case R6_OPC_DCLZ:
>          gen_helper_dclz(cpu_gpr[rd], t0);
>          opn = "dclz";
>          break;
> @@ -14704,12 +14714,13 @@ static void gen_mipsdsp_accinsn(DisasContext *ctx, 
> uint32_t op1, uint32_t op2,
>  
>  static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx)
>  {
> -    int rs, rt, rd;
> +    int rs, rt, rd, sa;
>      uint32_t op1, op2;
>  
>      rs = (ctx->opcode >> 21) & 0x1f;
>      rt = (ctx->opcode >> 16) & 0x1f;
>      rd = (ctx->opcode >> 11) & 0x1f;
> +    sa = (ctx->opcode >> 6) & 0x1f;
>  
>      op1 = MASK_SPECIAL(ctx->opcode);
>      switch (op1) {
> @@ -14736,7 +14747,31 @@ static void decode_opc_special_r6(CPUMIPSState *env, 
> DisasContext *ctx)
>      case OPC_SELNEZ:
>          gen_cond_move(ctx, op1, rd, rs, rt);
>          break;
> +    case R6_OPC_CLO:
> +    case R6_OPC_CLZ:
> +        if (rt == 0 && sa == 1) {
> +            /* Major opcode and function field is shared with preR6 
> MFHI/MTHI.
> +               We need additionally to check other fields */
> +            gen_cl(ctx, op1, rd, rs);
> +        } else {
> +            generate_exception(ctx, EXCP_RI);
> +        }
> +        break;
> +    case R6_OPC_SDBBP:
> +        generate_exception(ctx, EXCP_DBp);
> +        break;
>  #if defined(TARGET_MIPS64)
> +    case R6_OPC_DCLO:
> +    case R6_OPC_DCLZ:
> +        if (rt == 0 && sa == 1) {
> +            /* Major opcode and function field is shared with preR6 
> MFHI/MTHI.
> +               We need additionally to check other fields */
> +            check_mips_64(ctx);
> +            gen_cl(ctx, op1, rd, rs);
> +        } else {
> +            generate_exception(ctx, EXCP_RI);
> +        }
> +        break;
>      case OPC_DMULT ... OPC_DDIVU:
>          op2 = MASK_R6_MULDIV(ctx->opcode);
>          switch (op2) {
> @@ -14822,6 +14857,16 @@ static void decode_opc_special_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>          gen_muldiv(ctx, op1, 0, rs, rt);
>          break;
>  #endif
> +    case OPC_SPIM:
> +#ifdef MIPS_STRICT_STANDARD
> +        MIPS_INVAL("SPIM");
> +        generate_exception(ctx, EXCP_RI);
> +#else
> +        /* Implemented as RI exception for now. */
> +        MIPS_INVAL("spim (unofficial)");
> +        generate_exception(ctx, EXCP_RI);
> +#endif
> +        break;
>      default:            /* Invalid */
>          MIPS_INVAL("special_legacy");
>          generate_exception(ctx, EXCP_RI);
> @@ -14916,16 +14961,6 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *ctx)
>      case OPC_BREAK:
>          generate_exception(ctx, EXCP_BREAK);
>          break;
> -    case OPC_SPIM:
> -#ifdef MIPS_STRICT_STANDARD
> -        MIPS_INVAL("SPIM");
> -        generate_exception(ctx, EXCP_RI);
> -#else
> -        /* Implemented as RI exception for now. */
> -        MIPS_INVAL("spim (unofficial)");
> -        generate_exception(ctx, EXCP_RI);
> -#endif
> -        break;
>      case OPC_SYNC:
>          /* Treat as NOP. */
>          break;
> @@ -15015,19 +15050,6 @@ static void decode_opc_special(CPUMIPSState *env, 
> DisasContext *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("special2_r6");
> -        generate_exception(ctx, EXCP_RI);
> -        break;
> -    }
> -}
> -
>  static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>  {
>      int rs, rt, rd;
> @@ -15056,34 +15078,6 @@ static void decode_opc_special2_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>          check_insn(ctx, INSN_LOONGSON2F);
>          gen_loongson_integer(ctx, op1, rd, rs, rt);
>          break;
> -#if defined(TARGET_MIPS64)
> -    case OPC_DMULT_G_2F:
> -    case OPC_DMULTU_G_2F:
> -    case OPC_DDIV_G_2F:
> -    case OPC_DDIVU_G_2F:
> -    case OPC_DMOD_G_2F:
> -    case OPC_DMODU_G_2F:
> -        check_insn(ctx, INSN_LOONGSON2F);
> -        gen_loongson_integer(ctx, op1, rd, rs, rt);
> -        break;
> -#endif
> -    default:            /* Invalid */
> -        MIPS_INVAL("special2_legacy");
> -        generate_exception(ctx, EXCP_RI);
> -        break;
> -    }
> -}
> -
> -static void decode_opc_special2(CPUMIPSState *env, DisasContext *ctx)
> -{
> -    int rs, rd;
> -    uint32_t op1;
> -
> -    rs = (ctx->opcode >> 21) & 0x1f;
> -    rd = (ctx->opcode >> 11) & 0x1f;
> -
> -    op1 = MASK_SPECIAL2(ctx->opcode);
> -    switch (op1) {
>      case OPC_CLO:
>      case OPC_CLZ:
>          check_insn(ctx, ISA_MIPS32);
> @@ -15108,13 +15102,20 @@ static void decode_opc_special2(CPUMIPSState *env, 
> DisasContext *ctx)
>          check_mips_64(ctx);
>          gen_cl(ctx, op1, rd, rs);
>          break;
> +    case OPC_DMULT_G_2F:
> +    case OPC_DMULTU_G_2F:
> +    case OPC_DDIV_G_2F:
> +    case OPC_DDIVU_G_2F:
> +    case OPC_DMOD_G_2F:
> +    case OPC_DMODU_G_2F:
> +        check_insn(ctx, INSN_LOONGSON2F);
> +        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);
> -        }
> +    default:            /* Invalid */
> +        MIPS_INVAL("special2_legacy");
> +        generate_exception(ctx, EXCP_RI);
> +        break;
>      }
>  }
>  
> @@ -15793,7 +15794,8 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx)
>          decode_opc_special(env, ctx);
>          break;
>      case OPC_SPECIAL2:
> -        decode_opc_special2(env, ctx);
> +        check_insn_opc_removed(ctx, ISA_MIPS32R6);

As it concerns all SPECIAL2 instructions, it can probably be folded into
decode_opc_special2_legacy.

> +        decode_opc_special2_legacy(env, ctx);
>          break;
>      case OPC_SPECIAL3:
>          decode_opc_special3(env, ctx);

Besides the small nitpick above:

Reviewed-by: Aurelien Jarno <address@hidden>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net