From patchwork Sat Jul 22 00:35:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emilio Cota X-Patchwork-Id: 9857925 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CB950600CA for ; Sat, 22 Jul 2017 00:37:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BBDE728655 for ; Sat, 22 Jul 2017 00:37:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B084328675; Sat, 22 Jul 2017 00:37:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E96F028655 for ; Sat, 22 Jul 2017 00:37:21 +0000 (UTC) Received: from localhost ([::1]:45280 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dYiPF-0004XI-U1 for patchwork-qemu-devel@patchwork.kernel.org; Fri, 21 Jul 2017 20:36:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dYiOO-0004W7-CK for qemu-devel@nongnu.org; Fri, 21 Jul 2017 20:35:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dYiOL-0000qj-8U for qemu-devel@nongnu.org; Fri, 21 Jul 2017 20:35:36 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:60371) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dYiOL-0000pn-23 for qemu-devel@nongnu.org; Fri, 21 Jul 2017 20:35:33 -0400 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 46A3E207DF; Fri, 21 Jul 2017 20:35:32 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute4.internal (MEProxy); Fri, 21 Jul 2017 20:35:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=braap.org; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=uCeJrlpQNOjJniXeRkHqfLTyeOlEDWws1+JZUF 8ne0k=; b=jGTf0p6ly5rMS42Siu2FnNzMOp/2fZ/ZAv346+/ppXGzWoROr+F6Wx 3dYPkzhp6a1YV2jTn1DZrmMrXGv0Hm6ZwbCc2SBz5T4Y3TY3Ij99hc1tn4dX3ptk gR8Q1PUJrKmEuW2ocG5sqUXn9xLRJC51ofVDKwErTEVCRqT5bcMOA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=uCeJrlpQNOjJniXeRk HqfLTyeOlEDWws1+JZUF8ne0k=; b=ULMK21Py4Ea6xrSsuXSTVWOtpc1yTUkFBL I6ovJEtejQcfQykLPvXE8nqFVNURR3FArV9kVamNcQAEthX1wI/ZDN7RqnyoH5id a4AxbHUtOBGwqQcHVfmoWrc8+uV9PS0Zfu8BeTB0lkDjCx+EQL6g+f+jTCsOWI6G WzbxReR6hDXr7+85efXRuhBAINFdj1onTb523heu6oMO+CqRtQ807+49TyycUGYW tkNO01ppOXEtX2+s16JAhL4WIZTqhWeb/N3leo/kopRjR+oeohMcq70eHQm8/1Nh T+fI/JccrctxOuN9H+aeuBfYb6jf9AY7ysc0e4W+72vE+1H0CBaQ== X-ME-Sender: X-Sasl-enc: /2K0hYwgOcFeDaD2PtV4u7U+lQWfS7a0PMR0PyceGRRz 1500683732 Received: from localhost (flamenco.cs.columbia.edu [128.59.20.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 079A9240AF; Fri, 21 Jul 2017 20:35:32 -0400 (EDT) Date: Fri, 21 Jul 2017 20:35:31 -0400 From: "Emilio G. Cota" To: Richard Henderson Message-ID: <20170722003531.GB9909@flamenco> References: <20170715094243.28371-1-rth@twiddle.net> <20170715094243.28371-34-rth@twiddle.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170715094243.28371-34-rth@twiddle.net> User-Agent: Mutt/1.5.24 (2015-08-30) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.111.4.25 Subject: Re: [Qemu-devel] [PATCH v14 33/34] target/arm: Split out thumb_tr_translate_insn X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pbonzini@redhat.com, crosthwaite.peter@gmail.com, alex.bennee@linaro.org, qemu-devel@nongnu.org, vilanova@ac.upc.edu Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jul 14, 2017 at 23:42:42 -1000, Richard Henderson wrote: > We need not check for ARM vs Thumb state in order to dispatch > disassembly of every instruction. > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 134 +++++++++++++++++++++++++++++++------------------ > 1 file changed, 86 insertions(+), 48 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index ebe1c1a..d7c3c10 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c (snip) > +static void arm_post_translate_insn(CPUARMState *env, DisasContext *dc) > +{ > + /* Translation stops when a conditional branch is encountered. > + * Otherwise the subsequent code could get translated several times. > + * Also stop translation when a page boundary is reached. This > + * ensures prefetch aborts occur at the right place. > + * > + * We want to stop the TB if the next insn starts in a new page, > + * or if it spans between this page and the next. This means that > + * if we're looking at the last halfword in the page we need to > + * see if it's a 16-bit Thumb insn (which will fit in this TB) > + * or a 32-bit Thumb insn (which won't). > + * This is to avoid generating a silly TB with a single 16-bit insn > + * in it at the end of this page (which would execute correctly > + * but isn't very efficient). > + */ > + if (dc->base.is_jmp == DISAS_NEXT > + && (dc->pc >= dc->next_page_start > + || (dc->pc >= dc->next_page_start - 3 > + && insn_crosses_page(env, dc)))) { > + dc->base.is_jmp = DISAS_TOO_MANY; > } > > if (dc->condjmp && !dc->base.is_jmp) { > gen_set_label(dc->condlabel); > dc->condjmp = 0; > } > + dc->base.pc_next = dc->pc; > + translator_loop_temp_check(&dc->base); > +} > > - if (dc->base.is_jmp == DISAS_NEXT) { > - /* Translation stops when a conditional branch is encountered. > - * Otherwise the subsequent code could get translated several times. > - * Also stop translation when a page boundary is reached. This > - * ensures prefetch aborts occur at the right place. */ > - > - if (dc->pc >= dc->next_page_start || > - (dc->pc >= dc->next_page_start - 3 && > - insn_crosses_page(env, dc))) { > - /* We want to stop the TB if the next insn starts in a new page, > - * or if it spans between this page and the next. This means that > - * if we're looking at the last halfword in the page we need to > - * see if it's a 16-bit Thumb insn (which will fit in this TB) > - * or a 32-bit Thumb insn (which won't). > - * This is to avoid generating a silly TB with a single 16-bit insn > - * in it at the end of this page (which would execute correctly > - * but isn't very efficient). > - */ > - dc->base.is_jmp = DISAS_TOO_MANY; Note that we've moved the above hunk ("if (dc->base.is_jmp == DISAS_NEXT)") above the "if (dc->condjmp && !dc->base.is_jmp)" one; so when we now set is_jmp to DISAS_TOO_MANY, dc->condjmp might be wrongly cleared by the second hunk. My review missed this, but luckily TCG asserts caught it while booting debian-arm: [ OK ] Started Increase datagram queue length. systemd[1]: Started Increase datagram queue length. [ OK ] Mounted POSIX Message Queue File System. systemd[1]: Mounted POSIX Message Queue File System. qemu-system-arm: /data/src/qemu/tcg/tcg-op.c:2584: tcg_gen_goto_tb: Assertion `(tcg_ctx.goto_tb_issue_mask & (1 << idx)) == 0' failed. Aborted (core dumped) Keeping the original order fixes it. Emilio --8<-- --- target/arm/translate.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 77fe6a9..9cbace5 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -11979,6 +11979,11 @@ static bool arm_pre_translate_insn(DisasContext *dc) static void arm_post_translate_insn(CPUARMState *env, DisasContext *dc) { + if (dc->condjmp && !dc->base.is_jmp) { + gen_set_label(dc->condlabel); + dc->condjmp = 0; + } + /* Translation stops when a conditional branch is encountered. * Otherwise the subsequent code could get translated several times. * Also stop translation when a page boundary is reached. This @@ -12000,10 +12005,6 @@ static void arm_post_translate_insn(CPUARMState *env, DisasContext *dc) dc->base.is_jmp = DISAS_TOO_MANY; } - if (dc->condjmp && !dc->base.is_jmp) { - gen_set_label(dc->condlabel); - dc->condjmp = 0; - } dc->base.pc_next = dc->pc; translator_loop_temp_check(&dc->base); }