On 12/06/2015 15:02, Yongbok Kim wrote:
> add new microMIPS32 Release 6 POOL32I/POOL32C type instructions
>
> Signed-off-by: Yongbok Kim <[email protected]>
> ---
> target-mips/translate.c | 36 ++++++++++++++++++++++++++++++++----
> 1 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 3d9145c..5be2a9c 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -14412,8 +14412,16 @@ static void decode_micromips32_opc (CPUMIPSState
> *env, DisasContext *ctx,
> mips32_op = OPC_TGEIU;
> goto do_trapi;
> case TNEI:
> - mips32_op = OPC_TNEI;
> - goto do_trapi;
> + /* SYNCI */
I think the comment can be improved a bit because I still need to figure out
from the actual code which block implements SYNCI. I would suggest sticking to
existing style we have for single R6 and pre-R6 instructions which share
opcode, for example:
case OPC_BC1EQZ: /* OPC_BC1ANY2 */
if (ctx->insn_flags & ISA_MIPS32R6) {
/* OPC_BC1EQZ */
...
} else {
/* OPC_BC1ANY2 */
...
}
break;
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + /* Break the TB to be able to sync copied instructions
> + immediately */
> + ctx->bstate = BS_STOP;
> + } else {
> + mips32_op = OPC_TNEI;
> + goto do_trapi;
> + }
> + break;
> case TEQI:
> check_insn_opc_removed(ctx, ISA_MIPS32R6);
> mips32_op = OPC_TEQI;
> @@ -14537,10 +14545,18 @@ static void decode_micromips32_opc (CPUMIPSState
> *env, DisasContext *ctx,
> check_insn(ctx, ISA_MIPS3);
> check_mips_64(ctx);
> mips32_op = OPC_LLD;
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + gen_ld(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 9));
> + break;
> + }
> goto do_ld_lr;
> #endif
> case LL:
> mips32_op = OPC_LL;
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + gen_ld(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 9));
> + break;
> + }
> goto do_ld_lr;
> do_ld_lr:
> gen_ld(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 12));
> @@ -14549,17 +14565,29 @@ static void decode_micromips32_opc (CPUMIPSState
> *env, DisasContext *ctx,
> gen_st(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 12));
> break;
> case SC:
> - gen_st_cond(ctx, OPC_SC, rt, rs, SIMM(ctx->opcode, 0, 12));
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + gen_st_cond(ctx, OPC_SC, rt, rs, SIMM(ctx->opcode, 0, 9));
> + } else {
> + gen_st_cond(ctx, OPC_SC, rt, rs, SIMM(ctx->opcode, 0, 12));
> + }
> break;
> #if defined(TARGET_MIPS64)
> case SCD:
> check_insn(ctx, ISA_MIPS3);
> check_mips_64(ctx);
> - gen_st_cond(ctx, OPC_SCD, rt, rs, SIMM(ctx->opcode, 0, 12));
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + gen_st_cond(ctx, OPC_SCD, rt, rs, SIMM(ctx->opcode, 0, 9));
> + } else {
> + gen_st_cond(ctx, OPC_SCD, rt, rs, SIMM(ctx->opcode, 0, 12));
> + }
Instead of adding new "if R6" to each of these instructions and calling the
same gen_st_cond/gen_ld function with just different hardcoded value, I think
it would be cleaner to reuse offset variable and set it just once (9 in R6 or
12 in pre-R6) at the beginning of case POOL32C. Thus here we would have just:
gen_st_cond(ctx, OPC_SCD, rt, rs, offset);
Thanks,
Leon
> break;
> #endif
> case PREF:
> /* Treat as no-op */
> + if ((ctx->insn_flags & ISA_MIPS32R6) && (rt >= 24)) {
> + /* hint codes 24-31 are reserved and signal RI */
> + generate_exception(ctx, EXCP_RI);
> + }
> break;
> default:
> MIPS_INVAL("pool32c");
>
