From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39195) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4Z48-0003AW-Ci for qemu-devel@nongnu.org; Tue, 08 Jul 2014 13:20:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4Z3u-0007Qa-IC for qemu-devel@nongnu.org; Tue, 08 Jul 2014 13:20:28 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:43558) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4Z3u-0007QF-FG for qemu-devel@nongnu.org; Tue, 08 Jul 2014 13:20:14 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 8 Jul 2014 13:20:14 -0400 From: Michael Roth Date: Tue, 8 Jul 2014 12:16:52 -0500 Message-Id: <1404839947-1086-22-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1404839947-1086-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1404839947-1086-1-git-send-email-mdroth@linux.vnet.ibm.com> Subject: [Qemu-devel] [PATCH 021/156] arm: translate.c: Fix smlald Instruction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org From: Peter Crosthwaite 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 Message-id: 2cddb6f5a15be4ab8d2160f3499d128ae93d304d.1397704570.git.peter.crosthwaite@xilinx.com Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell (cherry picked from commit 33bbd75a7c3321432fe40a8cbacd64619c56138c) Signed-off-by: Michael Roth --- 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 5f003e7..e0c3eaa 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7732,27 +7732,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); -- 1.9.1