On 2015-06-23 16:38, Yongbok Kim wrote:
> microMIPS32 Release 6 POOL16A/ POOL16C instructions
>
> Signed-off-by: Yongbok Kim <yongbok....@imgtec.com>
> ---
> target-mips/translate.c | 122
> +++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 107 insertions(+), 15 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 4bce6e8..82764e7 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -13163,6 +13163,102 @@ static void gen_pool16c_insn(DisasContext *ctx)
> }
> }
>
> +static inline void gen_movep(DisasContext *ctx, int enc_dest, int enc_rt,
> + int enc_rs)
> +{
> + int rd, rs, re, rt;
> + static const int rd_enc[] = { 5, 5, 6, 4, 4, 4, 4, 4 };
> + static const int re_enc[] = { 6, 7, 7, 21, 22, 5, 6, 7 };
> + static const int rs_rt_enc[] = { 0, 17, 2, 3, 16, 18, 19, 20 };
> + rd = rd_enc[enc_dest];
> + re = re_enc[enc_dest];
> + rs = rs_rt_enc[enc_rs];
> + rt = rs_rt_enc[enc_rt];
> + gen_arith(ctx, OPC_ADDU, rd, rs, 0);
> + gen_arith(ctx, OPC_ADDU, re, rt, 0);
Do we really want to call ADDU here? This could be as simple as:
if (rs) {
tcg_gen_mov_tl(rd, rs);
} else {
tcg_gen_movi_tl(rd, 0);
}
if (rt) {
tcg_gen_mov_tl(re, rt);
} else {
tcg_gen_movi_tl(re, 0);
}
> +}
> +
> +static void gen_pool16c_r6_insn(DisasContext *ctx)
> +{
> + int rt = mmreg((ctx->opcode >> 7) & 0x7);
> + int rs = mmreg((ctx->opcode >> 4) & 0x7);
> +
> + switch (ctx->opcode & 0xf) {
> + case R6_NOT16:
> + gen_logic(ctx, OPC_NOR, rt, rs, 0);
> + break;
> + case R6_AND16:
> + gen_logic(ctx, OPC_AND, rt, rt, rs);
> + break;
> + case R6_LWM16:
> + {
> + int lwm_converted = 0x11 + extract32(ctx->opcode, 8, 2);
> + int offset = extract32(ctx->opcode, 4, 4);
> + gen_ldst_multiple(ctx, LWM32, lwm_converted, 29, offset << 2);
> + }
> + break;
> + case R6_JRC16: /* JRCADDIUSP */
> + if ((ctx->opcode >> 4) & 1) {
> + /* JRCADDIUSP */
> + int imm = extract32(ctx->opcode, 5, 5);
> + gen_compute_branch(ctx, OPC_JR, 2, 31, 0, 0, 0);
> + gen_arith_imm(ctx, OPC_ADDIU, 29, 29, imm << 2);
> + } else {
> + /* JRC16 */
> + int rs = extract32(ctx->opcode, 5, 5);
> + gen_compute_branch(ctx, OPC_JR, 2, rs, 0, 0, 0);
> + }
> + break;
> + case MOVEP ... MOVEP_07:
> + case MOVEP_0C ... MOVEP_0F:
> + {
> + int enc_dest = uMIPS_RD(ctx->opcode);
> + int enc_rt = uMIPS_RS2(ctx->opcode);
> + int enc_rs = (ctx->opcode & 3) | ((ctx->opcode >> 1) & 4);
> + gen_movep(ctx, enc_dest, enc_rt, enc_rs);
> + }
> + break;
> + case R6_XOR16:
> + gen_logic(ctx, OPC_XOR, rt, rt, rs);
> + break;
> + case R6_OR16:
> + gen_logic(ctx, OPC_OR, rt, rt, rs);
> + break;
> + case R6_SWM16:
> + {
> + int swm_converted = 0x11 + extract32(ctx->opcode, 8, 2);
> + int offset = extract32(ctx->opcode, 4, 4);
> + gen_ldst_multiple(ctx, SWM32, swm_converted, 29, offset << 2);
> + }
> + break;
> + case JALRC16: /* BREAK16, SDBBP16 */
> + switch (ctx->opcode & 0x3f) {
> + case JALRC16:
> + case JALRC16 + 0x20:
> + /* JALRC16 */
> + gen_compute_branch(ctx, OPC_JALR, 2, (ctx->opcode >> 5) & 0x1f,
> + 31, 0, 0);
> + break;
> + case R6_BREAK16:
> + /* BREAK16 */
> + generate_exception(ctx, EXCP_BREAK);
> + break;
> + case R6_SDBBP16:
> + /* SDBBP16 */
> + if (ctx->hflags & MIPS_HFLAG_SBRI) {
> + generate_exception(ctx, EXCP_RI);
> + } else {
> + generate_exception(ctx, EXCP_DBp);
> + }
> + break;
> + }
> + break;
> + default:
> + generate_exception(ctx, EXCP_RI);
> + break;
> + }
> +}
> +
> static void gen_ldxs (DisasContext *ctx, int base, int index, int rd)
> {
> TCGv t0 = tcg_temp_new();
> @@ -15168,8 +15264,11 @@ static int decode_micromips_opc (CPUMIPSState *env,
> DisasContext *ctx)
> opc = OPC_SUBU;
> break;
> }
> -
> - gen_arith(ctx, opc, rd, rs1, rs2);
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + gen_arith(ctx, opc, rs1, rd, rs2);
> + } else {
> + gen_arith(ctx, opc, rd, rs1, rs2);
> + }
That change is correct, but it would be nice to add a comment saying
that while the manual still describe the instruction as rd = rs + rt,
the register number location in the instruction encoding has changed.
> }
> break;
> case POOL16B:
> @@ -15193,7 +15292,11 @@ static int decode_micromips_opc (CPUMIPSState *env,
> DisasContext *ctx)
> }
> break;
> case POOL16C:
> - gen_pool16c_insn(ctx);
> + if (ctx->insn_flags & ISA_MIPS32R6) {
> + gen_pool16c_r6_insn(ctx);
> + } else {
> + gen_pool16c_insn(ctx);
> + }
> break;
> case LWGP16:
> {
> @@ -15213,18 +15316,7 @@ static int decode_micromips_opc (CPUMIPSState *env,
> DisasContext *ctx)
> int enc_dest = uMIPS_RD(ctx->opcode);
> int enc_rt = uMIPS_RS2(ctx->opcode);
> int enc_rs = uMIPS_RS1(ctx->opcode);
> - int rd, rs, re, rt;
> - static const int rd_enc[] = { 5, 5, 6, 4, 4, 4, 4, 4 };
> - static const int re_enc[] = { 6, 7, 7, 21, 22, 5, 6, 7 };
> - static const int rs_rt_enc[] = { 0, 17, 2, 3, 16, 18, 19, 20 };
> -
> - rd = rd_enc[enc_dest];
> - re = re_enc[enc_dest];
> - rs = rs_rt_enc[enc_rs];
> - rt = rs_rt_enc[enc_rt];
> -
> - gen_arith(ctx, OPC_ADDU, rd, rs, 0);
> - gen_arith(ctx, OPC_ADDU, re, rt, 0);
> + gen_movep(ctx, enc_dest, enc_rt, enc_rs);
> }
> break;
> case LBU16:
Besides the comments above, this all looks correct.
Reviewed-by: Aurelien Jarno <aurel...@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net
