From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq7OG-00072s-Id for qemu-devel@nongnu.org; Mon, 09 Dec 2013 15:25:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vq7O7-0005NC-Uq for qemu-devel@nongnu.org; Mon, 09 Dec 2013 15:25:16 -0500 Received: from mail-pb0-x22f.google.com ([2607:f8b0:400e:c01::22f]:59925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq7O7-0005Md-Hm for qemu-devel@nongnu.org; Mon, 09 Dec 2013 15:25:07 -0500 Received: by mail-pb0-f47.google.com with SMTP id um1so6067089pbc.20 for ; Mon, 09 Dec 2013 12:25:06 -0800 (PST) Sender: Richard Henderson Message-ID: <52A6271E.6050704@twiddle.net> Date: Mon, 09 Dec 2013 12:25:02 -0800 From: Richard Henderson MIME-Version: 1.0 References: <1386612744-1013-1-git-send-email-peter.maydell@linaro.org> <1386612744-1013-3-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1386612744-1013-3-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: patches@linaro.org, Michael Matz , Claudio Fontana , Dirk Mueller , Will Newton , Laurent Desnogues , =?UTF-8?B?QWxleCBCZW5uw6ll?= , kvmarm@lists.cs.columbia.edu, Christoffer Dall On 12/09/2013 10:12 AM, Peter Maydell wrote: > +static void do_gpr_ld(DisasContext *s, TCGv_i64 dest, TCGv_i64 tcg_addr, > + int size, bool is_signed, bool extend) > +{ > + switch (size) { > + case 0: > + if (is_signed) { > + tcg_gen_qemu_ld8s(dest, tcg_addr, get_mem_index(s)); > + } else { > + tcg_gen_qemu_ld8u(dest, tcg_addr, get_mem_index(s)); > + } > + break; > + case 1: > + if (is_signed) { > + tcg_gen_qemu_ld16s(dest, tcg_addr, get_mem_index(s)); > + } else { > + tcg_gen_qemu_ld16u(dest, tcg_addr, get_mem_index(s)); > + } > + break; > + case 2: > + if (is_signed) { > + tcg_gen_qemu_ld32s(dest, tcg_addr, get_mem_index(s)); > + } else { > + tcg_gen_qemu_ld32u(dest, tcg_addr, get_mem_index(s)); > + } > + break; > + case 3: > + tcg_gen_qemu_ld64(dest, tcg_addr, get_mem_index(s)); > + break; Likewise, combine all of this with tcg_gen_qemu_ld_i64. > + if (extend && is_signed) { > + g_assert(size < 3); > + tcg_gen_ext32u_i64(dest, dest); > + } Is it worth noticing in size==2 && !extend that is_signed can be forced false to avoid the extra extension. > +static void handle_ldp(DisasContext *s, uint32_t insn) > +{ > + int rt = extract32(insn, 0, 5); > + int rn = extract32(insn, 5, 5); > + int rt2 = extract32(insn, 10, 5); > + int64_t offset = sextract32(insn, 15, 7); > + int idx = extract32(insn, 23, 3); > + bool is_signed = false; > + bool is_vector = extract32(insn, 26, 1); > + int opc = extract32(insn, 30, 2); > + int size; > + bool postindex = true; > + bool wback = false; > + > + TCGv_i64 tcg_rt = cpu_reg(s, rt); > + TCGv_i64 tcg_rt2 = cpu_reg(s, rt2); > + TCGv_i64 tcg_addr; > + > + if (opc == 3) { > + unallocated_encoding(s); > + return; > + } > + if (is_vector) { > + size = 2 + opc; > + } else { > + is_signed = opc & 1; > + size = 2 + extract32(opc, 1, 1); > + } > + > + switch (idx) { > + case 1: /* post-index */ > + postindex = true; > + wback = true; > + break; > + case 2: /* signed offset, rn not updated */ > + postindex = false; > + break; > + case 3: /* STP (pre-index) */ > + postindex = false; > + wback = true; > + break; > + default: /* Failed decoder tree? */ > + unallocated_encoding(s); > + break; > + } > + > + offset <<= size; > + > + if (rn == 31) { > + gen_check_sp_alignment(s); > + } > + > + tcg_addr = tcg_temp_new_i64(); > + tcg_gen_mov_i64(tcg_addr, cpu_reg_sp(s, rn)); > + > + if (!postindex) { > + tcg_gen_addi_i64(tcg_addr, tcg_addr, offset); > + } > + > + if (is_vector) { > + do_fp_ld(s, rt, tcg_addr, size); > + } else { > + do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false); > + } > + tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size); > + if (is_vector) { > + do_fp_ld(s, rt2, tcg_addr, size); > + } else { > + do_gpr_ld(s, tcg_rt2, tcg_addr, size, is_signed, false); > + } > + > + if (wback) { > + if (postindex) { > + tcg_gen_addi_i64(tcg_addr, tcg_addr, offset - (1 << size)); > + } else { > + tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size); > + } > + tcg_gen_mov_i64(cpu_reg_sp(s, rn), tcg_addr); > + } > + tcg_temp_free_i64(tcg_addr); > +} There's so much overlap with STP, I think these ought not be separate functions. Just merge the middle bit where the actual load/store happens. r~