From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35966) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9ZiE-00032S-PX for qemu-devel@nongnu.org; Thu, 06 Sep 2012 06:53:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T9ZiA-0006yB-2Q for qemu-devel@nongnu.org; Thu, 06 Sep 2012 06:53:30 -0400 Received: from hall.aurel32.net ([88.191.126.93]:59548) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9Zi9-0006xz-PU for qemu-devel@nongnu.org; Thu, 06 Sep 2012 06:53:25 -0400 Date: Thu, 6 Sep 2012 11:11:17 +0200 From: Aurelien Jarno Message-ID: <20120906091117.GA22010@ohm.aurel32.net> References: <1346135785-12119-1-git-send-email-proljc@gmail.com> <1346135785-12119-4-git-send-email-proljc@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <1346135785-12119-4-git-send-email-proljc@gmail.com> Subject: Re: [Qemu-devel] [PATCH v7 03/14] target-mips-ase-dsp: Use correct acc value to index cpu_HI/cpu_LO rather than using a fix number List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jia Liu Cc: qemu-devel@nongnu.org On Tue, Aug 28, 2012 at 02:36:14PM +0800, Jia Liu wrote: > Use correct acc value to index cpu_HI/cpu_LO rather than using a fix number. > > Signed-off-by: Jia Liu > --- > target-mips/translate.c | 134 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 107 insertions(+), 27 deletions(-) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 6000183..e1ea9c1 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -5,6 +5,7 @@ > * Copyright (c) 2006 Marius Groeger (FPU operations) > * Copyright (c) 2006 Thiemo Seufer (MIPS32R2 support) > * Copyright (c) 2009 CodeSourcery (MIPS16 and microMIPS support) > + * Copyright (c) 2012 Jia Liu & Dongxue Zhang (MIPS ASE DSP support) > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -1990,33 +1991,87 @@ static void gen_shift (CPUMIPSState *env, DisasContext *ctx, uint32_t opc, > static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg) > { > const char *opn = "hilo"; > + unsigned int acc; > > if (reg == 0 && (opc == OPC_MFHI || opc == OPC_MFLO)) { > /* Treat as NOP. */ > MIPS_DEBUG("NOP"); > 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); > + } > + > switch (opc) { > case OPC_MFHI: > - tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[0]); > +#if defined(TARGET_MIPS64) > + if (acc == 0) { > + tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[acc]); > + } else { > + tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[acc]); > + } > +#else > + tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[acc]); > +#endif This kind of code can be written: #if defined(TARGET_MIPS64) if (acc != 0) { tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[acc]); } else #endif { tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[acc]); } > opn = "mfhi"; > break; > case OPC_MFLO: > - tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[0]); > +#if defined(TARGET_MIPS64) > + if (acc == 0) { > + tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[acc]); > + } else { > + tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_LO[acc]); > + } > +#else > + tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[acc]); > +#endif > opn = "mflo"; > break; > case OPC_MTHI: > - if (reg != 0) > - tcg_gen_mov_tl(cpu_HI[0], cpu_gpr[reg]); > - else > - tcg_gen_movi_tl(cpu_HI[0], 0); > +#if defined(TARGET_MIPS64) > + if (reg != 0) { > + if (acc == 0) { > + tcg_gen_mov_tl(cpu_HI[acc], cpu_gpr[reg]); > + } else { > + tcg_gen_ext32s_tl(cpu_HI[acc], cpu_gpr[reg]); > + } > + } else { > + tcg_gen_movi_tl(cpu_HI[acc], 0); > + } > +#else > + if (reg != 0) { > + tcg_gen_mov_tl(cpu_HI[acc], cpu_gpr[reg]); > + } else { > + tcg_gen_movi_tl(cpu_HI[acc], 0); > + } > +#endif > opn = "mthi"; > break; > case OPC_MTLO: > - if (reg != 0) > - tcg_gen_mov_tl(cpu_LO[0], cpu_gpr[reg]); > - else > - tcg_gen_movi_tl(cpu_LO[0], 0); > +#if defined(TARGET_MIPS64) > + if (reg != 0) { > + if (acc == 0) { > + tcg_gen_mov_tl(cpu_LO[acc], cpu_gpr[reg]); > + } else { > + tcg_gen_ext32s_tl(cpu_LO[acc], cpu_gpr[reg]); > + } > + } else { > + tcg_gen_movi_tl(cpu_LO[acc], 0); > + } > +#else > + if (reg != 0) { > + tcg_gen_mov_tl(cpu_LO[acc], cpu_gpr[reg]); > + } else { > + tcg_gen_movi_tl(cpu_LO[acc], 0); > + } > +#endif > opn = "mtlo"; > break; > } > @@ -2029,6 +2084,7 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc, > { > const char *opn = "mul/div"; > TCGv t0, t1; > + unsigned int acc; > > switch (opc) { > case OPC_DIV: > @@ -2091,6 +2147,10 @@ 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); > @@ -2100,8 +2160,8 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc, > tcg_gen_shri_i64(t2, t2, 32); > tcg_gen_trunc_i64_tl(t1, t2); > tcg_temp_free_i64(t2); > - tcg_gen_ext32s_tl(cpu_LO[0], t0); > - tcg_gen_ext32s_tl(cpu_HI[0], t1); > + tcg_gen_ext32s_tl(cpu_LO[acc], t0); > + tcg_gen_ext32s_tl(cpu_HI[acc], t1); > } > opn = "mult"; > break; > @@ -2109,6 +2169,10 @@ 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); > @@ -2120,8 +2184,8 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc, > tcg_gen_shri_i64(t2, t2, 32); > tcg_gen_trunc_i64_tl(t1, t2); > tcg_temp_free_i64(t2); > - tcg_gen_ext32s_tl(cpu_LO[0], t0); > - tcg_gen_ext32s_tl(cpu_HI[0], t1); > + tcg_gen_ext32s_tl(cpu_LO[acc], t0); > + tcg_gen_ext32s_tl(cpu_HI[acc], t1); > } > opn = "multu"; > break; > @@ -2168,41 +2232,49 @@ 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); > tcg_gen_mul_i64(t2, t2, t3); > - tcg_gen_concat_tl_i64(t3, cpu_LO[0], cpu_HI[0]); > + tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]); > tcg_gen_add_i64(t2, t2, t3); > tcg_temp_free_i64(t3); > tcg_gen_trunc_i64_tl(t0, t2); > tcg_gen_shri_i64(t2, t2, 32); > tcg_gen_trunc_i64_tl(t1, t2); > tcg_temp_free_i64(t2); > - tcg_gen_ext32s_tl(cpu_LO[0], t0); > - tcg_gen_ext32s_tl(cpu_HI[0], t1); > + tcg_gen_ext32s_tl(cpu_LO[acc], t0); > + tcg_gen_ext32s_tl(cpu_HI[acc], t1); > } > opn = "madd"; > break; > case OPC_MADDU: > - { > + { > 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); > tcg_gen_extu_tl_i64(t2, t0); > tcg_gen_extu_tl_i64(t3, t1); > tcg_gen_mul_i64(t2, t2, t3); > - tcg_gen_concat_tl_i64(t3, cpu_LO[0], cpu_HI[0]); > + tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]); > tcg_gen_add_i64(t2, t2, t3); > tcg_temp_free_i64(t3); > tcg_gen_trunc_i64_tl(t0, t2); > tcg_gen_shri_i64(t2, t2, 32); > tcg_gen_trunc_i64_tl(t1, t2); > tcg_temp_free_i64(t2); > - tcg_gen_ext32s_tl(cpu_LO[0], t0); > - tcg_gen_ext32s_tl(cpu_HI[0], t1); > + tcg_gen_ext32s_tl(cpu_LO[acc], t0); > + tcg_gen_ext32s_tl(cpu_HI[acc], t1); > } > opn = "maddu"; > break; > @@ -2210,19 +2282,23 @@ 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); > tcg_gen_mul_i64(t2, t2, t3); > - tcg_gen_concat_tl_i64(t3, cpu_LO[0], cpu_HI[0]); > + tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]); > tcg_gen_sub_i64(t2, t3, t2); > tcg_temp_free_i64(t3); > tcg_gen_trunc_i64_tl(t0, t2); > tcg_gen_shri_i64(t2, t2, 32); > tcg_gen_trunc_i64_tl(t1, t2); > tcg_temp_free_i64(t2); > - tcg_gen_ext32s_tl(cpu_LO[0], t0); > - tcg_gen_ext32s_tl(cpu_HI[0], t1); > + tcg_gen_ext32s_tl(cpu_LO[acc], t0); > + tcg_gen_ext32s_tl(cpu_HI[acc], t1); > } > opn = "msub"; > break; > @@ -2230,21 +2306,25 @@ 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); > tcg_gen_extu_tl_i64(t2, t0); > tcg_gen_extu_tl_i64(t3, t1); > tcg_gen_mul_i64(t2, t2, t3); > - tcg_gen_concat_tl_i64(t3, cpu_LO[0], cpu_HI[0]); > + tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]); > tcg_gen_sub_i64(t2, t3, t2); > tcg_temp_free_i64(t3); > tcg_gen_trunc_i64_tl(t0, t2); > tcg_gen_shri_i64(t2, t2, 32); > tcg_gen_trunc_i64_tl(t1, t2); > tcg_temp_free_i64(t2); > - tcg_gen_ext32s_tl(cpu_LO[0], t0); > - tcg_gen_ext32s_tl(cpu_HI[0], t1); > + tcg_gen_ext32s_tl(cpu_LO[acc], t0); > + tcg_gen_ext32s_tl(cpu_HI[acc], t1); > } > opn = "msubu"; > break; Except the minor nitpick above, this patch looks fine. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net