From 0fb8a7de8e8013362922d802db7eda5f9bf37766 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Wed, 16 Apr 2014 20:20:52 -0700 Subject: [PATCH] arm: translate.c: Fix smlald Instruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Andreas Färber --- 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);