From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrHBd-00010T-0G for qemu-devel@nongnu.org; Thu, 12 Dec 2013 20:05:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VrHBX-0006Cp-3d for qemu-devel@nongnu.org; Thu, 12 Dec 2013 20:05:00 -0500 Received: from mail-pb0-x231.google.com ([2607:f8b0:400e:c01::231]:57312) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrHBW-0006Cj-O2 for qemu-devel@nongnu.org; Thu, 12 Dec 2013 20:04:55 -0500 Received: by mail-pb0-f49.google.com with SMTP id jt11so1501795pbb.8 for ; Thu, 12 Dec 2013 17:04:53 -0800 (PST) Date: Fri, 13 Dec 2013 11:04:17 +1000 From: "Edgar E. Iglesias" Message-ID: <20131213010417.GA2161@edvb> References: <1386718821-2562-1-git-send-email-rth@twiddle.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1386718821-2562-1-git-send-email-rth@twiddle.net> Subject: Re: [Qemu-devel] [PATCH] target-microblaze: Use the new qemu_ld/st opcodes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org On Tue, Dec 10, 2013 at 03:40:21PM -0800, Richard Henderson wrote: > The ability of the new opcodes to byte-swap the memory operation > simplifies the code in and around dec_load and dec_store significantly. I've tested and applied this, thanks. Edgar > > Cc: Edgar E. Iglesias > Signed-off-by: Richard Henderson > --- > target-microblaze/translate.c | 139 +++++++++++------------------------------- > 1 file changed, 35 insertions(+), 104 deletions(-) > > diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c > index 9edcb67..270138c 100644 > --- a/target-microblaze/translate.c > +++ b/target-microblaze/translate.c > @@ -864,26 +864,6 @@ static void dec_imm(DisasContext *dc) > dc->clear_imm = 0; > } > > -static inline void gen_load(DisasContext *dc, TCGv dst, TCGv addr, > - unsigned int size, bool exclusive) > -{ > - int mem_index = cpu_mmu_index(dc->env); > - > - if (size == 1) { > - tcg_gen_qemu_ld8u(dst, addr, mem_index); > - } else if (size == 2) { > - tcg_gen_qemu_ld16u(dst, addr, mem_index); > - } else if (size == 4) { > - tcg_gen_qemu_ld32u(dst, addr, mem_index); > - } else > - cpu_abort(dc->env, "Incorrect load size %d\n", size); > - > - if (exclusive) { > - tcg_gen_mov_tl(env_res_addr, addr); > - tcg_gen_mov_tl(env_res_val, dst); > - } > -} > - > static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) > { > unsigned int extimm = dc->tb_flags & IMM_FLAG; > @@ -935,35 +915,22 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) > return t; > } > > -static inline void dec_byteswap(DisasContext *dc, TCGv dst, TCGv src, int size) > -{ > - if (size == 4) { > - tcg_gen_bswap32_tl(dst, src); > - } else if (size == 2) { > - TCGv t = tcg_temp_new(); > - > - /* bswap16 assumes the high bits are zero. */ > - tcg_gen_andi_tl(t, src, 0xffff); > - tcg_gen_bswap16_tl(dst, t); > - tcg_temp_free(t); > - } else { > - /* Ignore. > - cpu_abort(dc->env, "Invalid ldst byteswap size %d\n", size); > - */ > - } > -} > - > static void dec_load(DisasContext *dc) > { > - TCGv t, *addr; > + TCGv t, v, *addr; > unsigned int size, rev = 0, ex = 0; > + TCGMemOp mop; > > - size = 1 << (dc->opcode & 3); > - > + mop = dc->opcode & 3; > + size = 1 << mop; > if (!dc->type_b) { > rev = (dc->ir >> 9) & 1; > ex = (dc->ir >> 10) & 1; > } > + mop |= MO_TE; > + if (rev) { > + mop ^= MO_BSWAP; > + } > > if (size > 4 && (dc->tb_flags & MSR_EE_FLAG) > && (dc->env->pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)) { > @@ -1044,40 +1011,30 @@ static void dec_load(DisasContext *dc) > sync_jmpstate(dc); > > /* Verify alignment if needed. */ > - if ((dc->env->pvr.regs[2] & PVR2_UNALIGNED_EXC_MASK) && size > 1) { > - TCGv v = tcg_temp_new(); > - > - /* > - * Microblaze gives MMU faults priority over faults due to > - * unaligned addresses. That's why we speculatively do the load > - * into v. If the load succeeds, we verify alignment of the > - * address and if that succeeds we write into the destination reg. > - */ > - gen_load(dc, v, *addr, size, ex); > + /* > + * Microblaze gives MMU faults priority over faults due to > + * unaligned addresses. That's why we speculatively do the load > + * into v. If the load succeeds, we verify alignment of the > + * address and if that succeeds we write into the destination reg. > + */ > + v = tcg_temp_new(); > + tcg_gen_qemu_ld_tl(v, *addr, cpu_mmu_index(dc->env), mop); > > + if ((dc->env->pvr.regs[2] & PVR2_UNALIGNED_EXC_MASK) && size > 1) { > tcg_gen_movi_tl(cpu_SR[SR_PC], dc->pc); > gen_helper_memalign(cpu_env, *addr, tcg_const_tl(dc->rd), > tcg_const_tl(0), tcg_const_tl(size - 1)); > - if (dc->rd) { > - if (rev) { > - dec_byteswap(dc, cpu_R[dc->rd], v, size); > - } else { > - tcg_gen_mov_tl(cpu_R[dc->rd], v); > - } > - } > - tcg_temp_free(v); > - } else { > - if (dc->rd) { > - gen_load(dc, cpu_R[dc->rd], *addr, size, ex); > - if (rev) { > - dec_byteswap(dc, cpu_R[dc->rd], cpu_R[dc->rd], size); > - } > - } else { > - /* We are loading into r0, no need to reverse. */ > - gen_load(dc, env_imm, *addr, size, ex); > - } > } > > + if (ex) { > + tcg_gen_mov_tl(env_res_addr, *addr); > + tcg_gen_mov_tl(env_res_val, v); > + } > + if (dc->rd) { > + tcg_gen_mov_tl(cpu_R[dc->rd], v); > + } > + tcg_temp_free(v); > + > if (ex) { /* lwx */ > /* no support for for AXI exclusive so always clear C */ > write_carryi(dc, 0); > @@ -1087,32 +1044,23 @@ static void dec_load(DisasContext *dc) > tcg_temp_free(t); > } > > -static void gen_store(DisasContext *dc, TCGv addr, TCGv val, > - unsigned int size) > -{ > - int mem_index = cpu_mmu_index(dc->env); > - > - if (size == 1) > - tcg_gen_qemu_st8(val, addr, mem_index); > - else if (size == 2) { > - tcg_gen_qemu_st16(val, addr, mem_index); > - } else if (size == 4) { > - tcg_gen_qemu_st32(val, addr, mem_index); > - } else > - cpu_abort(dc->env, "Incorrect store size %d\n", size); > -} > - > static void dec_store(DisasContext *dc) > { > TCGv t, *addr, swx_addr; > int swx_skip = 0; > unsigned int size, rev = 0, ex = 0; > + TCGMemOp mop; > > - size = 1 << (dc->opcode & 3); > + mop = dc->opcode & 3; > + size = 1 << mop; > if (!dc->type_b) { > rev = (dc->ir >> 9) & 1; > ex = (dc->ir >> 10) & 1; > } > + mop |= MO_TE; > + if (rev) { > + mop ^= MO_BSWAP; > + } > > if (size > 4 && (dc->tb_flags & MSR_EE_FLAG) > && (dc->env->pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)) { > @@ -1148,7 +1096,7 @@ static void dec_store(DisasContext *dc) > this compare and the following write to be atomic. For user > emulation we need to add atomicity between threads. */ > tval = tcg_temp_new(); > - gen_load(dc, tval, swx_addr, 4, false); > + tcg_gen_qemu_ld_tl(tval, swx_addr, cpu_mmu_index(dc->env), MO_TEUL); > tcg_gen_brcond_tl(TCG_COND_NE, env_res_val, tval, swx_skip); > write_carryi(dc, 0); > tcg_temp_free(tval); > @@ -1197,25 +1145,8 @@ static void dec_store(DisasContext *dc) > cpu_abort(dc->env, "Invalid reverse size\n"); > break; > } > - > - if (size != 1) { > - TCGv bs_data = tcg_temp_new(); > - dec_byteswap(dc, bs_data, cpu_R[dc->rd], size); > - gen_store(dc, *addr, bs_data, size); > - tcg_temp_free(bs_data); > - } else { > - gen_store(dc, *addr, cpu_R[dc->rd], size); > - } > - } else { > - if (rev) { > - TCGv bs_data = tcg_temp_new(); > - dec_byteswap(dc, bs_data, cpu_R[dc->rd], size); > - gen_store(dc, *addr, bs_data, size); > - tcg_temp_free(bs_data); > - } else { > - gen_store(dc, *addr, cpu_R[dc->rd], size); > - } > } > + tcg_gen_qemu_st_tl(cpu_R[dc->rd], *addr, cpu_mmu_index(dc->env), mop); > > /* Verify alignment if needed. */ > if ((dc->env->pvr.regs[2] & PVR2_UNALIGNED_EXC_MASK) && size > 1) { > -- > 1.8.1.4 >