From patchwork Thu Apr 17 03:20:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Crosthwaite X-Patchwork-Id: 339770 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 ozlabs.org (Postfix) with ESMTPS id CE5FB1400AC for ; Thu, 17 Apr 2014 13:21:52 +1000 (EST) Received: from localhost ([::1]:57638 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wacta-0006Zg-Mj for incoming@patchwork.ozlabs.org; Wed, 16 Apr 2014 23:21:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52637) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WactG-0006IE-IW for qemu-devel@nongnu.org; Wed, 16 Apr 2014 23:21:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WactB-0003Vd-5r for qemu-devel@nongnu.org; Wed, 16 Apr 2014 23:21:30 -0400 Received: from mail-qg0-f42.google.com ([209.85.192.42]:56114) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WactB-0003VX-16 for qemu-devel@nongnu.org; Wed, 16 Apr 2014 23:21:25 -0400 Received: by mail-qg0-f42.google.com with SMTP id q107so12450759qgd.29 for ; Wed, 16 Apr 2014 20:21:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=607zMRjxrqKlrlymY1gEgb+KNWhay0F2/JjYOLuqGu4=; b=kWLf8lenWamFSradf89gn4yZ91KKBPwPK0MjILdx1tgyRUq1/ryEZldmzxLFuZsfOc hwbmqiwqEBBSxM5ZBMIA8HD1vbUCvMUqC+d9HvQ8ufg/QowEDNUf1RWfZ3k7kGc3bo66 LP72UFMx+yy13S8e3mvbAQqVj2WifiP1LO39cX0A1qpnKHsBPuT3O1ZF7UGKyQFr606J 77iBza+74r6AO9JhcDGMP9HD8Dz2Vn0PQ6JgkU4T37cp29IIcJpKjDjtPZSKw6DrvCzZ VNem4my+pEXiPiAw3OmzOcfhpHNmqea3DEa+AYiWp/akxNbB2MHRF3jX3DL1d3m0D8rg uRMg== X-Gm-Message-State: ALoCoQm7VEWoTzsKVQsD8zkiWREAyWRbnQaw9iJBz8BspsjJhIuh2K1AZIjJwMRBHdHjwrwPCJMt X-Received: by 10.229.216.72 with SMTP id hh8mr7657173qcb.9.1397704884454; Wed, 16 Apr 2014 20:21:24 -0700 (PDT) Received: from localhost ([149.199.62.254]) by mx.google.com with ESMTPSA id e69sm31164456qgf.17.2014.04.16.20.21.23 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 16 Apr 2014 20:21:23 -0700 (PDT) From: Peter Crosthwaite To: qemu-devel@nongnu.org Date: Wed, 16 Apr 2014 20:20:52 -0700 Message-Id: <2cddb6f5a15be4ab8d2160f3499d128ae93d304d.1397704570.git.peter.crosthwaite@xilinx.com> X-Mailer: git-send-email 1.9.2.1.g06c4abd X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.192.42 Cc: laurent.desnogues@gmail.com, peter.maydell@linaro.org, christina.smith@xilinx.com Subject: [Qemu-devel] [PATCH target-arm v2 1/1] arm: translate.c: Fix smlald Instruction X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The smlald (and probably smlsld) instruction was doing incorrect sign extensions of the operands amongst 64bit result calculation. The instruction psuedo-code is: operand2 = if m_swap then ROR(R[m],16) else R[m]; product1 = SInt(R[n]<15:0>) * SInt(operand2<15:0>); product2 = SInt(R[n]<31:16>) * SInt(operand2<31:16>); result = product1 + product2 + SInt(R[dHi]:R[dLo]); R[dHi] = result<63:32>; R[dLo] = result<31:0>; The result calculation should be done in 64 bit arithmetic, and hence product1 and product2 should be sign extended to 64b before calculation. The current implementation was adding product1 and product2 together then sign-extending the intermediate result leading to false negatives. E.G. if product1 = product2 = 0x4000000, their sum = 0x80000000, which will be incorrectly interpreted as -ve on sign extension. We fix by doing the 64b extensions on both product1 and product2 before any addition/subtraction happens. We also fix where we were possibly incorrectly setting the Q saturation flag for SMLSLD, which the ARM ARM specifically says is not set. Reported-by: Christina Smith Signed-off-by: Peter Crosthwaite Reviewed-by: Peter Maydell --- changed since v1 (PMM review): Added commit msg note about Q flag correction Localised tmp64_2 declaration Fixed "else" comment location target-arm/translate.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 56e3b4b..0335f10 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -8328,27 +8328,39 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) if (insn & (1 << 5)) gen_swap_half(tmp2); gen_smul_dual(tmp, tmp2); - if (insn & (1 << 6)) { - /* This subtraction cannot overflow. */ - tcg_gen_sub_i32(tmp, tmp, tmp2); - } else { - /* This addition cannot overflow 32 bits; - * however it may overflow considered as a signed - * operation, in which case we must set the Q flag. - */ - gen_helper_add_setq(tmp, cpu_env, tmp, tmp2); - } - tcg_temp_free_i32(tmp2); if (insn & (1 << 22)) { /* smlald, smlsld */ + TCGv_i64 tmp64_2; + tmp64 = tcg_temp_new_i64(); + tmp64_2 = tcg_temp_new_i64(); tcg_gen_ext_i32_i64(tmp64, tmp); + tcg_gen_ext_i32_i64(tmp64_2, tmp2); tcg_temp_free_i32(tmp); + tcg_temp_free_i32(tmp2); + if (insn & (1 << 6)) { + tcg_gen_sub_i64(tmp64, tmp64, tmp64_2); + } else { + tcg_gen_add_i64(tmp64, tmp64, tmp64_2); + } + tcg_temp_free_i64(tmp64_2); gen_addq(s, tmp64, rd, rn); gen_storeq_reg(s, rd, rn, tmp64); tcg_temp_free_i64(tmp64); } else { /* smuad, smusd, smlad, smlsd */ + if (insn & (1 << 6)) { + /* This subtraction cannot overflow. */ + tcg_gen_sub_i32(tmp, tmp, tmp2); + } else { + /* This addition cannot overflow 32 bits; + * however it may overflow considered as a + * signed operation, in which case we must set + * the Q flag. + */ + gen_helper_add_setq(tmp, cpu_env, tmp, tmp2); + } + tcg_temp_free_i32(tmp2); if (rd != 15) { tmp2 = load_reg(s, rd);