On 9/26/18 11:59 PM, Philippe Mathieu-Daudé wrote: > On 9/25/18 2:20 PM, Philippe Mathieu-Daudé wrote: >> Hi Fredrik, >> >> On 9/15/18 11:25 AM, Fredrik Noring wrote: >>> Signed-off-by: Fredrik Noring <nor...@nocrew.org> >>> --- >>> target/mips/translate.c | 53 >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 53 insertions(+) >>> >>> diff --git a/target/mips/translate.c b/target/mips/translate.c >>> index ab16cdb911..fb571e278e 100644 >>> --- a/target/mips/translate.c >>> +++ b/target/mips/translate.c >>> @@ -3768,6 +3768,57 @@ static void gen_muldiv(DisasContext *ctx, uint32_t >>> opc, >>> tcg_temp_free(t1); >>> } >>> >>> +static void gen_mul_r5900(DisasContext *ctx, uint32_t opc, >>> + int acc, int rd, int rs, int rt) >>> +{ >>> + TCGv t0 = tcg_temp_new(); >>> + TCGv t1 = tcg_temp_new(); >>> + >>> + gen_load_gpr(t0, rs); >>> + gen_load_gpr(t1, rt); >>> + >>> + switch (opc) { >>> + case OPC_MULT: >>> + { >>> + TCGv_i32 t2 = tcg_temp_new_i32(); >>> + TCGv_i32 t3 = tcg_temp_new_i32(); >>> + tcg_gen_trunc_tl_i32(t2, t0); >>> + tcg_gen_trunc_tl_i32(t3, t1); >>> + tcg_gen_muls2_i32(t2, t3, t2, t3); >>> + if (rd) >>> + tcg_gen_ext_i32_tl(cpu_gpr[rd], t2); >>> + tcg_gen_ext_i32_tl(cpu_LO[acc], t2); >>> + tcg_gen_ext_i32_tl(cpu_HI[acc], t3); >>> + tcg_temp_free_i32(t2); >>> + tcg_temp_free_i32(t3); >>> + } >>> + break; >>> + case OPC_MULTU: >>> + { >>> + TCGv_i32 t2 = tcg_temp_new_i32(); >>> + TCGv_i32 t3 = tcg_temp_new_i32(); >>> + tcg_gen_trunc_tl_i32(t2, t0); >>> + tcg_gen_trunc_tl_i32(t3, t1); >>> + tcg_gen_mulu2_i32(t2, t3, t2, t3); >>> + if (rd) >>> + tcg_gen_ext_i32_tl(cpu_gpr[rd], t2); >>> + tcg_gen_ext_i32_tl(cpu_LO[acc], t2); >>> + tcg_gen_ext_i32_tl(cpu_HI[acc], t3); >>> + tcg_temp_free_i32(t2); >>> + tcg_temp_free_i32(t3); >>> + } >>> + break; >>> + default: >>> + MIPS_INVAL("mul R5900"); >>> + generate_exception_end(ctx, EXCP_RI); >>> + goto out; >>> + } >>> + >>> + out: >>> + tcg_temp_free(t0); >>> + tcg_temp_free(t1); >>> +} >> >> Why not simply modify gen_muldiv? > > Testing my own patch eh... > > I missed the check_dsp() call: > > if (acc != 0 && unlikely(!(ctx->insn_flags & INSN_R5900))) { > check_dsp(ctx); > } > > Anyway if we plan to also add MADD/MADDU, using another function might > be cleaner. > >> >> -- >8 -- >> @@ -3630,29 +3630,35 @@ static void gen_muldiv(DisasContext *ctx, >> uint32_t opc, >> break; >> case OPC_MULT: >> { >> TCGv_i32 t2 = tcg_temp_new_i32(); >> TCGv_i32 t3 = tcg_temp_new_i32(); >> tcg_gen_trunc_tl_i32(t2, t0); >> tcg_gen_trunc_tl_i32(t3, t1); >> tcg_gen_muls2_i32(t2, t3, t2, t3); >> + if (ctx->insn_flags & INSN_R5900 && rd) { >> + tcg_gen_ext_i32_tl(cpu_gpr[rd], t2); >> + } >> tcg_gen_ext_i32_tl(cpu_LO[acc], t2); >> tcg_gen_ext_i32_tl(cpu_HI[acc], t3); >> tcg_temp_free_i32(t2); >> tcg_temp_free_i32(t3); >> } >> break; >> case OPC_MULTU: >> { >> TCGv_i32 t2 = tcg_temp_new_i32(); >> TCGv_i32 t3 = tcg_temp_new_i32(); >> tcg_gen_trunc_tl_i32(t2, t0); >> tcg_gen_trunc_tl_i32(t3, t1); >> tcg_gen_mulu2_i32(t2, t3, t2, t3); >> + if (ctx->insn_flags & INSN_R5900 && rd) { >> + tcg_gen_ext_i32_tl(cpu_LO[acc], t2); >> + } >> tcg_gen_ext_i32_tl(cpu_LO[acc], t2); >> tcg_gen_ext_i32_tl(cpu_HI[acc], t3); >> tcg_temp_free_i32(t2); >> tcg_temp_free_i32(t3); >> } >> break; >> --- >> >>> + >>> static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc, >>> int rd, int rs, int rt) >>> { >>> @@ -22378,6 +22429,8 @@ static void decode_opc_special_legacy(CPUMIPSState >>> *env, DisasContext *ctx) >>> check_insn(ctx, INSN_VR54XX); >>> op1 = MASK_MUL_VR54XX(ctx->opcode); >>> gen_mul_vr54xx(ctx, op1, rd, rs, rt); >>> + } else if (ctx->insn_flags & INSN_R5900) { >>> + gen_mul_r5900(ctx, op1, 0, rd, rs, rt);
And I now notice you take all the $rd bits, so my patch proposal should also require: } else if (ctx->insn_flags & INSN_R5900) { gen_muldiv(ctx, op1, 0, rd, rs, rt); Which a this point makes it pointless. I'll restart reviewing your patch :) >>> } else { >>> gen_muldiv(ctx, op1, rd & 3, rs, rt); >>> } >>>