From patchwork Fri Jan 29 19:19:20 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 44019 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 2B3C0B7CC1 for ; Sat, 30 Jan 2010 06:31:06 +1100 (EST) Received: from localhost ([127.0.0.1]:54743 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NawTK-0000WL-NA for incoming@patchwork.ozlabs.org; Fri, 29 Jan 2010 14:25:38 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NawNI-00033L-M1 for qemu-devel@nongnu.org; Fri, 29 Jan 2010 14:19:24 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NawNH-00032d-9d for qemu-devel@nongnu.org; Fri, 29 Jan 2010 14:19:23 -0500 Received: from [199.232.76.173] (port=54874 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NawNH-00032T-2z for qemu-devel@nongnu.org; Fri, 29 Jan 2010 14:19:23 -0500 Received: from are.twiddle.net ([75.149.56.221]:58276) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NawNG-0007jK-24 for qemu-devel@nongnu.org; Fri, 29 Jan 2010 14:19:22 -0500 Received: from stone.twiddle.home (stone.twiddle.home [172.31.0.16]) by are.twiddle.net (Postfix) with ESMTPSA id 58AFC31; Fri, 29 Jan 2010 11:19:20 -0800 (PST) Message-ID: <4B6334B8.9040002@twiddle.net> Date: Fri, 29 Jan 2010 11:19:20 -0800 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7) Gecko/20100120 Fedora/3.0.1-1.fc12 Thunderbird/3.0.1 MIME-Version: 1.0 To: identifier scorpio Subject: Re: [Qemu-devel] [PATCH] Porting TCG to alpha platform References: <242393.28161.qm@web15901.mail.cnb.yahoo.com> <4B5E4311.10707@twiddle.net> In-Reply-To: <4B5E4311.10707@twiddle.net> X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org > + } else if (cond == TCG_COND_EQ || cond == TCG_COND_NE) { > + tcg_out_mem_long(s, INSN_LDA, TMP_REG1, arg1, -arg2); > + opc = (cond == TCG_COND_EQ ? INSN_BEQ : INSN_BNE); Bug here. What was intended was to add "arg1 = TMP_REG1". But since the constraints use "I" for uint8_t input constants, we might as well remove this hunk entirely. Also, let's future-proof this routine against changes to the layout of the TCGCond enumeration. r~ commit 9d787576342c193f13e2531953fc81442458de7e Author: Richard Henderson Date: Fri Jan 29 11:14:20 2010 -0800 tcg-alpha: Fix EQ/NE with a constant. There was start of code to handle EQ and NE with arbitrary constants, but it wasn't completed. Remove the half-done code and add a comment for future enhancements. Also, don't rely on the current layout of TCGCond; instead encode the need for inversion of the compare insn result by means of a low bit set in the cmp_opc table. Reduce the element size of cmp_opc. diff --git a/tcg/alpha/tcg-target.c b/tcg/alpha/tcg-target.c index 5b7dd25..18ab2c8 100644 --- a/tcg/alpha/tcg-target.c +++ b/tcg/alpha/tcg-target.c @@ -540,9 +540,11 @@ static void tcg_out_br(TCGContext *s, int opc, int ra, int label_index) tcg_out_fmt_br(s, opc, ra, value); } -static void tcg_out_brcond(TCGContext *s, int cond, TCGArg arg1, +static void tcg_out_brcond(TCGContext *s, TCGCond cond, TCGArg arg1, TCGArg arg2, int const_arg2, int label_index) { + /* Note that unsigned comparisons are not present here, which means + that their entries will contain zeros. */ static const int br_opc[10] = { [TCG_COND_EQ] = INSN_BEQ, [TCG_COND_NE] = INSN_BNE, @@ -552,38 +554,56 @@ static void tcg_out_brcond(TCGContext *s, int cond, TCGArg arg1, [TCG_COND_GT] = INSN_BGT }; - static const uint64_t cmp_opc[10] = { + /* The low bit of these entries indicates that the result of + the comparison must be inverted. This bit should not be + output with the rest of the instruction. */ + static const int cmp_opc[10] = { [TCG_COND_EQ] = INSN_CMPEQ, - [TCG_COND_NE] = INSN_CMPEQ, + [TCG_COND_NE] = INSN_CMPEQ | 1, [TCG_COND_LT] = INSN_CMPLT, - [TCG_COND_GE] = INSN_CMPLT, + [TCG_COND_GE] = INSN_CMPLT | 1, [TCG_COND_LE] = INSN_CMPLE, - [TCG_COND_GT] = INSN_CMPLE, + [TCG_COND_GT] = INSN_CMPLE | 1, [TCG_COND_LTU] = INSN_CMPULT, - [TCG_COND_GEU] = INSN_CMPULT, + [TCG_COND_GEU] = INSN_CMPULT | 1, [TCG_COND_LEU] = INSN_CMPULE, - [TCG_COND_GTU] = INSN_CMPULE + [TCG_COND_GTU] = INSN_CMPULE | 1 }; int opc = 0; - if (const_arg2) { - if (arg2 == 0) { - opc = br_opc[cond]; - } else if (cond == TCG_COND_EQ || cond == TCG_COND_NE) { - tcg_out_mem_long(s, INSN_LDA, TMP_REG1, arg1, -arg2); - opc = (cond == TCG_COND_EQ ? INSN_BEQ : INSN_BNE); - } + /* Possible improvements: + (1) Notice arg1 == $31 and !const_arg2. In this case, swap the + two operands and swap the sense of the comparison to allow the + use of the direct branches. + + (2) Allow arbitrary constants. We can, on occasion, generate one + less instruction if we compute + TMP = ARG1 - CONST + instead of + TMP = ARG1 cmp TMP2 + where TMP2 is the constant loaded into a register by generic code. + Note that for 64-bit operands this works only for EQ and NE. For + 32-bit operands, we would need to either limit this to signed + comparisons or properly zero-extend unsigned inputs. The payoff + here isn't great though; much less than(1). */ + + /* Notice signed comparisons vs zero. These are handled by the + branch instructions directly. */ + if (const_arg2 && arg2 == 0) { + opc = br_opc[cond]; } + /* Otherwise, generate a comparison into a temporary. */ if (opc == 0) { - opc = cmp_opc[cond]; + opc = cmp_opc[cond] & ~1; if (const_arg2) { tcg_out_fmt_opi(s, opc, arg1, arg2, TMP_REG1); } else { tcg_out_fmt_opr(s, opc, arg1, arg2, TMP_REG1); } - opc = (cond & 1) ? INSN_BEQ : INSN_BNE; + + opc = (cmp_opc[cond] & 1 ? INSN_BEQ : INSN_BNE); arg1 = TMP_REG1; }