From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32953) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4Sd9-0000Gm-PW for qemu-devel@nongnu.org; Mon, 15 Jun 2015 07:32:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4Sd5-00052S-9W for qemu-devel@nongnu.org; Mon, 15 Jun 2015 07:32:43 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:41433) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4Sd5-00051J-1T for qemu-devel@nongnu.org; Mon, 15 Jun 2015 07:32:39 -0400 Message-ID: <557EB7CC.4090507@imgtec.com> Date: Mon, 15 Jun 2015 12:32:28 +0100 From: Leon Alrae MIME-Version: 1.0 References: <1434117743-53520-1-git-send-email-yongbok.kim@imgtec.com> <1434117743-53520-4-git-send-email-yongbok.kim@imgtec.com> In-Reply-To: <1434117743-53520-4-git-send-email-yongbok.kim@imgtec.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/13] target-mips: refactor {D}LSA, {D}ALIGN, {D}BITSWAP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yongbok Kim , qemu-devel@nongnu.org Cc: aurelien@aurel32.net On 12/06/2015 15:02, Yongbok Kim wrote: > Refactor those instructions in order to reuse them for microMIPS32 > Release 6. > > Signed-off-by: Yongbok Kim > --- > target-mips/translate.c | 164 +++++++++++++++++++++++++++++----------------- > 1 files changed, 103 insertions(+), 61 deletions(-) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index b8c7164..2244630 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -4831,6 +4831,102 @@ static void gen_bshfl (DisasContext *ctx, uint32_t op2, int rt, int rd) > tcg_temp_free(t0); > } > > +static void gen_lsa(DisasContext *ctx, int opc, int rd, int rs, int rt, > + int imm2) Indentation is unusual. > +{ > + TCGv t0; > + TCGv t1; > + if (rd == 0) { > + /* Treat as NOP. */ > + return; > + } > + t0 = tcg_temp_new(); > + t1 = tcg_temp_new(); > + gen_load_gpr(t0, rs); > + gen_load_gpr(t1, rt); > + tcg_gen_shli_tl(t0, t0, imm2 + 1); > + switch (opc) { > + case OPC_LSA: > + tcg_gen_add_tl(t0, t0, t1); > + tcg_gen_ext32s_tl(cpu_gpr[rd], t0); > + break; > +#if defined(TARGET_MIPS64) > + case OPC_DLSA: > + tcg_gen_add_tl(cpu_gpr[rd], t0, t1); > + break; > +#endif The only difference between LSA and DLSA is that LSA sign extends the value from bit 31. Wouldn't it be better to replace this switch with: tcg_gen_add_tl(cpu_gpr[rd], t0, t1); if (opc == OPC_LSA) { tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]); } > + } > + tcg_temp_free(t1); > + tcg_temp_free(t0); > + > + return; > +} > + > +static void gen_align(DisasContext *ctx, int opc, int rd, int rs, int rt, > + int bp) > +{ > + TCGv t0; > + if (rd == 0) { > + /* Treat as NOP. */ > + return; > + } > + t0 = tcg_temp_new(); > + gen_load_gpr(t0, rt); > + if (bp == 0) { > + tcg_gen_mov_tl(cpu_gpr[rd], t0); > + } else { > + TCGv t1 = tcg_temp_new(); > + gen_load_gpr(t1, rs); > + switch (opc) { > + case OPC_ALIGN: > + { > + TCGv_i64 t2 = tcg_temp_new_i64(); > + tcg_gen_concat_tl_i64(t2, t1, t0); > + tcg_gen_shri_i64(t2, t2, 8 * (4 - bp)); > +#if defined(TARGET_MIPS64) > + tcg_gen_ext32s_i64(cpu_gpr[rd], t2); > +#else > + tcg_gen_trunc_i64_i32(cpu_gpr[rd], t2); > +#endif This can be replaced with gen_move_low32(). > + tcg_temp_free_i64(t2); > + } > + break; > +#if defined(TARGET_MIPS64) > + case OPC_DALIGN: > + tcg_gen_shli_tl(t0, t0, 8 * bp); > + tcg_gen_shri_tl(t1, t1, 8 * (8 - bp)); > + tcg_gen_or_tl(cpu_gpr[rd], t1, t0); > + break; > +#endif > + } > + tcg_temp_free(t1); > + } > + > + tcg_temp_free(t0); > +} > + > +static void gen_bitswap(DisasContext *ctx, int opc, int rd, int rt) > +{ > + TCGv t0; > + if (rd == 0) { > + /* Treat as NOP. */ > + return; > + } > + t0 = tcg_temp_new(); > + gen_load_gpr(t0, rt); > + switch (opc) { > + case OPC_BITSWAP: > + gen_helper_bitswap(cpu_gpr[rd], t0); > + break; > +#if defined(TARGET_MIPS64) > + case OPC_DBITSWAP: > + gen_helper_dbitswap(cpu_gpr[rd], t0); > + break; > +#endif > + } > + tcg_temp_free(t0); > +} > + > #ifndef CONFIG_USER_ONLY > /* CP0 (MMU and control) */ > static inline void gen_mfc0_load32 (TCGv arg, target_ulong off) > @@ -16191,18 +16287,7 @@ static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx) > op1 = MASK_SPECIAL(ctx->opcode); > switch (op1) { > case OPC_LSA: > - if (rd != 0) { > - int imm2 = extract32(ctx->opcode, 6, 3); > - TCGv t0 = tcg_temp_new(); > - TCGv t1 = tcg_temp_new(); > - gen_load_gpr(t0, rs); > - gen_load_gpr(t1, rt); > - tcg_gen_shli_tl(t0, t0, imm2 + 1); > - tcg_gen_add_tl(t0, t0, t1); > - tcg_gen_ext32s_tl(cpu_gpr[rd], t0); > - tcg_temp_free(t1); > - tcg_temp_free(t0); > - } > + gen_lsa(ctx, op1, rd, rs, rt, extract32(ctx->opcode, 6, 2)); Thanks for fixing the bug, length is indeed 2. It worked because additional bit we read here was always 0. > break; > case OPC_MULT ... OPC_DIVU: > op2 = MASK_R6_MULDIV(ctx->opcode); > @@ -16247,17 +16332,7 @@ static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx) > #if defined(TARGET_MIPS64) > case OPC_DLSA: > check_mips_64(ctx); > - if (rd != 0) { > - int imm2 = extract32(ctx->opcode, 6, 3); > - TCGv t0 = tcg_temp_new(); > - TCGv t1 = tcg_temp_new(); > - gen_load_gpr(t0, rs); > - gen_load_gpr(t1, rt); > - tcg_gen_shli_tl(t0, t0, imm2 + 1); > - tcg_gen_add_tl(cpu_gpr[rd], t0, t1); > - tcg_temp_free(t1); > - tcg_temp_free(t0); > - } > + gen_lsa(ctx, op1, rd, rs, rt, extract32(ctx->opcode, 6, 2)); > break; > case R6_OPC_DCLO: > case R6_OPC_DCLZ: > @@ -16682,35 +16757,15 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx) > /* Treat as NOP. */ > break; > } > - TCGv t0 = tcg_temp_new(); > - gen_load_gpr(t0, rt); > - > op2 = MASK_BSHFL(ctx->opcode); > switch (op2) { > case OPC_ALIGN ... OPC_ALIGN_END: > - sa &= 3; > - if (sa == 0) { > - tcg_gen_mov_tl(cpu_gpr[rd], t0); > - } else { > - TCGv t1 = tcg_temp_new(); > - TCGv_i64 t2 = tcg_temp_new_i64(); > - gen_load_gpr(t1, rs); > - tcg_gen_concat_tl_i64(t2, t1, t0); > - tcg_gen_shri_i64(t2, t2, 8 * (4 - sa)); > -#if defined(TARGET_MIPS64) > - tcg_gen_ext32s_i64(cpu_gpr[rd], t2); > -#else > - tcg_gen_trunc_i64_i32(cpu_gpr[rd], t2); > -#endif > - tcg_temp_free_i64(t2); > - tcg_temp_free(t1); > - } > + gen_align(ctx, OPC_ALIGN, rd, rs, rt, sa & 3); > break; > case OPC_BITSWAP: > - gen_helper_bitswap(cpu_gpr[rd], t0); > + gen_bitswap(ctx, op2, rd, rt); > break; > } > - tcg_temp_free(t0); > } > break; > #if defined(TARGET_MIPS64) > @@ -16727,29 +16782,16 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx) > /* Treat as NOP. */ > break; > } > - TCGv t0 = tcg_temp_new(); > - gen_load_gpr(t0, rt); > - > op2 = MASK_DBSHFL(ctx->opcode); > switch (op2) { > case OPC_DALIGN ... OPC_DALIGN_END: > - sa &= 7; > - if (sa == 0) { > - tcg_gen_mov_tl(cpu_gpr[rd], t0); > - } else { > - TCGv t1 = tcg_temp_new(); > - gen_load_gpr(t1, rs); > - tcg_gen_shli_tl(t0, t0, 8 * sa); > - tcg_gen_shri_tl(t1, t1, 8 * (8 - sa)); > - tcg_gen_or_tl(cpu_gpr[rd], t1, t0); > - tcg_temp_free(t1); > - } > + gen_align(ctx, OPC_DALIGN, rd, rs, rt, sa & 7); > break; > case OPC_DBITSWAP: > - gen_helper_dbitswap(cpu_gpr[rd], t0); > + gen_bitswap(ctx, op2, rd, rt); > break; > } > - tcg_temp_free(t0); > + > } > break; > #endif >