[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-mips: Fix accumulator selection for MIPS
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH] target-mips: Fix accumulator selection for MIPS16 and microMIPS |
Date: |
Mon, 21 Jan 2013 20:00:15 +0000 |
On Sun, Jan 20, 2013 at 7:27 PM, Richard Sandiford
<address@hidden> wrote:
> Add accumulator arguments to gen_HILO and gen_muldiv, rather than
> extracting the accumulator directly from ctx->opcode. The extraction
> was only right for the standard encoding: MIPS16 doesn't have access
> to the DSP registers, while microMIPS encodes the accumulator register
> in a different field (bits 14 and 15).
>
> Passing the accumulator register is probably an over-generalisation
> for division and 64-bit multiplication, which never access anything
> other than HI and LO, and which always pass 0 as the new argument.
> Separating them felt a bit fussy though.
>
> Signed-off-by: Richard Sandiford <address@hidden>
> ---
> target-mips/translate.c | 135
> ++++++++++++++++++++----------------------------
> 1 file changed, 57 insertions(+), 78 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 206ba83..47528d7 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -2571,10 +2571,9 @@ static void gen_shift (CPUMIPSState *env, DisasContext
> *ctx, uint32_t opc,
> }
>
> /* Arithmetic on HI/LO registers */
> -static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
> +static void gen_HILO (DisasContext *ctx, uint32_t opc, int acc, int reg)
Please remove the extra space between function name and '('.
> {
> const char *opn = "hilo";
> - unsigned int acc;
>
> if (reg == 0 && (opc == OPC_MFHI || opc == OPC_MFLO)) {
> /* Treat as NOP. */
> @@ -2582,12 +2581,6 @@ static void gen_HILO (DisasContext *ctx, uint32_t opc,
> int reg)
> return;
> }
>
> - if (opc == OPC_MFHI || opc == OPC_MFLO) {
> - acc = ((ctx->opcode) >> 21) & 0x03;
> - } else {
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - }
> -
> if (acc != 0) {
> check_dsp(ctx);
> }
> @@ -2651,11 +2644,10 @@ static void gen_HILO (DisasContext *ctx, uint32_t
> opc, int reg)
> }
>
> static void gen_muldiv (DisasContext *ctx, uint32_t opc,
> - int rs, int rt)
> + int acc, int rs, int rt)
> {
> const char *opn = "mul/div";
> TCGv t0, t1;
> - unsigned int acc;
>
> t0 = tcg_temp_new();
> t1 = tcg_temp_new();
> @@ -2663,6 +2655,9 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
> gen_load_gpr(t0, rs);
> gen_load_gpr(t1, rt);
>
> + if (acc != 0)
Missing braces, please read CODING_STYLE and check the patches with
checkpatch.pl.
> + check_dsp(ctx);
> +
> switch (opc) {
> case OPC_DIV:
> {
> @@ -2677,10 +2672,10 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> tcg_gen_or_tl(t2, t2, t3);
> tcg_gen_movi_tl(t3, 0);
> tcg_gen_movcond_tl(TCG_COND_NE, t1, t2, t3, t2, t1);
> - tcg_gen_div_tl(cpu_LO[0], t0, t1);
> - tcg_gen_rem_tl(cpu_HI[0], t0, t1);
> - tcg_gen_ext32s_tl(cpu_LO[0], cpu_LO[0]);
> - tcg_gen_ext32s_tl(cpu_HI[0], cpu_HI[0]);
> + tcg_gen_div_tl(cpu_LO[acc], t0, t1);
> + tcg_gen_rem_tl(cpu_HI[acc], t0, t1);
> + tcg_gen_ext32s_tl(cpu_LO[acc], cpu_LO[acc]);
> + tcg_gen_ext32s_tl(cpu_HI[acc], cpu_HI[acc]);
> tcg_temp_free(t3);
> tcg_temp_free(t2);
> }
> @@ -2693,10 +2688,10 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> tcg_gen_ext32u_tl(t0, t0);
> tcg_gen_ext32u_tl(t1, t1);
> tcg_gen_movcond_tl(TCG_COND_EQ, t1, t1, t2, t3, t1);
> - tcg_gen_divu_tl(cpu_LO[0], t0, t1);
> - tcg_gen_remu_tl(cpu_HI[0], t0, t1);
> - tcg_gen_ext32s_tl(cpu_LO[0], cpu_LO[0]);
> - tcg_gen_ext32s_tl(cpu_HI[0], cpu_HI[0]);
> + tcg_gen_divu_tl(cpu_LO[acc], t0, t1);
> + tcg_gen_remu_tl(cpu_HI[acc], t0, t1);
> + tcg_gen_ext32s_tl(cpu_LO[acc], cpu_LO[acc]);
> + tcg_gen_ext32s_tl(cpu_HI[acc], cpu_HI[acc]);
> tcg_temp_free(t3);
> tcg_temp_free(t2);
> }
> @@ -2706,10 +2701,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> {
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - if (acc != 0) {
> - check_dsp(ctx);
> - }
>
> tcg_gen_ext_tl_i64(t2, t0);
> tcg_gen_ext_tl_i64(t3, t1);
> @@ -2728,10 +2719,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> {
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - if (acc != 0) {
> - check_dsp(ctx);
> - }
>
> tcg_gen_ext32u_tl(t0, t0);
> tcg_gen_ext32u_tl(t1, t1);
> @@ -2760,8 +2747,8 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
> tcg_gen_or_tl(t2, t2, t3);
> tcg_gen_movi_tl(t3, 0);
> tcg_gen_movcond_tl(TCG_COND_NE, t1, t2, t3, t2, t1);
> - tcg_gen_div_tl(cpu_LO[0], t0, t1);
> - tcg_gen_rem_tl(cpu_HI[0], t0, t1);
> + tcg_gen_div_tl(cpu_LO[acc], t0, t1);
> + tcg_gen_rem_tl(cpu_HI[acc], t0, t1);
> tcg_temp_free(t3);
> tcg_temp_free(t2);
> }
> @@ -2772,8 +2759,8 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
> TCGv t2 = tcg_const_tl(0);
> TCGv t3 = tcg_const_tl(1);
> tcg_gen_movcond_tl(TCG_COND_EQ, t1, t1, t2, t3, t1);
> - tcg_gen_divu_i64(cpu_LO[0], t0, t1);
> - tcg_gen_remu_i64(cpu_HI[0], t0, t1);
> + tcg_gen_divu_i64(cpu_LO[acc], t0, t1);
> + tcg_gen_remu_i64(cpu_HI[acc], t0, t1);
> tcg_temp_free(t3);
> tcg_temp_free(t2);
> }
> @@ -2792,10 +2779,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> {
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - if (acc != 0) {
> - check_dsp(ctx);
> - }
>
> tcg_gen_ext_tl_i64(t2, t0);
> tcg_gen_ext_tl_i64(t3, t1);
> @@ -2816,10 +2799,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> {
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - if (acc != 0) {
> - check_dsp(ctx);
> - }
>
> tcg_gen_ext32u_tl(t0, t0);
> tcg_gen_ext32u_tl(t1, t1);
> @@ -2842,10 +2821,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> {
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - if (acc != 0) {
> - check_dsp(ctx);
> - }
>
> tcg_gen_ext_tl_i64(t2, t0);
> tcg_gen_ext_tl_i64(t3, t1);
> @@ -2866,10 +2841,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t
> opc,
> {
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
> - acc = ((ctx->opcode) >> 11) & 0x03;
> - if (acc != 0) {
> - check_dsp(ctx);
> - }
>
> tcg_gen_ext32u_tl(t0, t0);
> tcg_gen_ext32u_tl(t1, t1);
> @@ -10134,7 +10105,7 @@ static int decode_mips16_opc (CPUMIPSState *env,
> DisasContext *ctx,
> gen_logic(env, ctx, OPC_NOR, rx, ry, 0);
> break;
> case RR_MFHI:
> - gen_HILO(ctx, OPC_MFHI, rx);
> + gen_HILO(ctx, OPC_MFHI, 0, rx);
> break;
> case RR_CNVT:
> switch (cnvt_op) {
> @@ -10166,7 +10137,7 @@ static int decode_mips16_opc (CPUMIPSState *env,
> DisasContext *ctx,
> }
> break;
> case RR_MFLO:
> - gen_HILO(ctx, OPC_MFLO, rx);
> + gen_HILO(ctx, OPC_MFLO, 0, rx);
> break;
> #if defined (TARGET_MIPS64)
> case RR_DSRA:
> @@ -10187,33 +10158,33 @@ static int decode_mips16_opc (CPUMIPSState *env,
> DisasContext *ctx,
> break;
> #endif
> case RR_MULT:
> - gen_muldiv(ctx, OPC_MULT, rx, ry);
> + gen_muldiv(ctx, OPC_MULT, 0, rx, ry);
> break;
> case RR_MULTU:
> - gen_muldiv(ctx, OPC_MULTU, rx, ry);
> + gen_muldiv(ctx, OPC_MULTU, 0, rx, ry);
> break;
> case RR_DIV:
> - gen_muldiv(ctx, OPC_DIV, rx, ry);
> + gen_muldiv(ctx, OPC_DIV, 0, rx, ry);
> break;
> case RR_DIVU:
> - gen_muldiv(ctx, OPC_DIVU, rx, ry);
> + gen_muldiv(ctx, OPC_DIVU, 0, rx, ry);
> break;
> #if defined (TARGET_MIPS64)
> case RR_DMULT:
> check_mips_64(ctx);
> - gen_muldiv(ctx, OPC_DMULT, rx, ry);
> + gen_muldiv(ctx, OPC_DMULT, 0, rx, ry);
> break;
> case RR_DMULTU:
> check_mips_64(ctx);
> - gen_muldiv(ctx, OPC_DMULTU, rx, ry);
> + gen_muldiv(ctx, OPC_DMULTU, 0, rx, ry);
> break;
> case RR_DDIV:
> check_mips_64(ctx);
> - gen_muldiv(ctx, OPC_DDIV, rx, ry);
> + gen_muldiv(ctx, OPC_DDIV, 0, rx, ry);
> break;
> case RR_DDIVU:
> check_mips_64(ctx);
> - gen_muldiv(ctx, OPC_DDIVU, rx, ry);
> + gen_muldiv(ctx, OPC_DDIVU, 0, rx, ry);
> break;
> #endif
> default:
> @@ -10922,11 +10893,11 @@ static void gen_pool16c_insn (CPUMIPSState *env,
> DisasContext *ctx, int *is_bran
> break;
> case MFHI16 + 0:
> case MFHI16 + 1:
> - gen_HILO(ctx, OPC_MFHI, uMIPS_RS5(ctx->opcode));
> + gen_HILO(ctx, OPC_MFHI, 0, uMIPS_RS5(ctx->opcode));
> break;
> case MFLO16 + 0:
> case MFLO16 + 1:
> - gen_HILO(ctx, OPC_MFLO, uMIPS_RS5(ctx->opcode));
> + gen_HILO(ctx, OPC_MFLO, 0, uMIPS_RS5(ctx->opcode));
> break;
> case BREAK16:
> generate_exception(ctx, EXCP_BREAK);
> @@ -11124,30 +11095,33 @@ static void gen_pool32axf (CPUMIPSState *env,
> DisasContext *ctx, int rt, int rs,
> break;
> case MULT:
> mips32_op = OPC_MULT;
> - goto do_muldiv;
> + goto do_mul;
> case MULTU:
> mips32_op = OPC_MULTU;
> - goto do_muldiv;
> + goto do_mul;
> case DIV:
> mips32_op = OPC_DIV;
> - goto do_muldiv;
> + goto do_div;
> case DIVU:
> mips32_op = OPC_DIVU;
> - goto do_muldiv;
> + do_div:
> + check_insn(env, ctx, ISA_MIPS32);
> + gen_muldiv(ctx, mips32_op, 0, rs, rt);
> + break;
> case MADD:
> mips32_op = OPC_MADD;
> - goto do_muldiv;
> + goto do_mul;
> case MADDU:
> mips32_op = OPC_MADDU;
> - goto do_muldiv;
> + goto do_mul;
> case MSUB:
> mips32_op = OPC_MSUB;
> - goto do_muldiv;
> + goto do_mul;
> case MSUBU:
> mips32_op = OPC_MSUBU;
> - do_muldiv:
> + do_mul:
> check_insn(env, ctx, ISA_MIPS32);
> - gen_muldiv(ctx, mips32_op, rs, rt);
> + gen_muldiv(ctx, mips32_op, (ctx->opcode >> 14) & 3, rs, rt);
> break;
> default:
> goto pool32axf_invalid;
> @@ -11284,18 +11258,18 @@ static void gen_pool32axf (CPUMIPSState *env,
> DisasContext *ctx, int rt, int rs,
> }
> break;
> case 0x35:
> - switch (minor) {
> + switch (minor & 3) {
> case MFHI32:
> - gen_HILO(ctx, OPC_MFHI, rs);
> + gen_HILO(ctx, OPC_MFHI, minor >> 2, rs);
> break;
> case MFLO32:
> - gen_HILO(ctx, OPC_MFLO, rs);
> + gen_HILO(ctx, OPC_MFLO, minor >> 2, rs);
> break;
> case MTHI32:
> - gen_HILO(ctx, OPC_MTHI, rs);
> + gen_HILO(ctx, OPC_MTHI, minor >> 2, rs);
> break;
> case MTLO32:
> - gen_HILO(ctx, OPC_MTLO, rs);
> + gen_HILO(ctx, OPC_MTLO, minor >> 2, rs);
> break;
> default:
> goto pool32axf_invalid;
> @@ -14429,13 +14403,18 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
> case OPC_XOR:
> gen_logic(env, ctx, op1, rd, rs, rt);
> break;
> - case OPC_MULT ... OPC_DIVU:
> + case OPC_MULT:
> + case OPC_MULTU:
> if (sa) {
> check_insn(env, ctx, INSN_VR54XX);
> op1 = MASK_MUL_VR54XX(ctx->opcode);
> gen_mul_vr54xx(ctx, op1, rd, rs, rt);
> } else
> - gen_muldiv(ctx, op1, rs, rt);
> + gen_muldiv(ctx, op1, rd & 3, rs, rt);
> + break;
> + case OPC_DIV:
> + case OPC_DIVU:
> + gen_muldiv(ctx, op1, 0, rs, rt);
> break;
> case OPC_JR ... OPC_JALR:
> gen_compute_branch(ctx, op1, 4, rs, rd, sa);
> @@ -14447,11 +14426,11 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
> break;
> case OPC_MFHI: /* Move from HI/LO */
> case OPC_MFLO:
> - gen_HILO(ctx, op1, rd);
> + gen_HILO(ctx, op1, rs & 3, rd);
> break;
> case OPC_MTHI:
> case OPC_MTLO: /* Move to HI/LO */
> - gen_HILO(ctx, op1, rs);
> + gen_HILO(ctx, op1, rd & 3, rs);
> break;
> case OPC_PMON: /* Pmon entry point, also R4010 selsl */
> #ifdef MIPS_STRICT_STANDARD
> @@ -14571,7 +14550,7 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
> case OPC_DMULT ... OPC_DDIVU:
> check_insn(env, ctx, ISA_MIPS3);
> check_mips_64(ctx);
> - gen_muldiv(ctx, op1, rs, rt);
> + gen_muldiv(ctx, op1, 0, rs, rt);
> break;
> #endif
> default: /* Invalid */
> @@ -14586,7 +14565,7 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
> case OPC_MADD ... OPC_MADDU: /* Multiply and add/sub */
> case OPC_MSUB ... OPC_MSUBU:
> check_insn(env, ctx, ISA_MIPS32);
> - gen_muldiv(ctx, op1, rs, rt);
> + gen_muldiv(ctx, op1, rd & 3, rs, rt);
> break;
> case OPC_MUL:
> gen_arith(env, ctx, op1, rd, rs, rt);
> --
> 1.7.11.7
>
>