[Qemu-devel] [PULL 50/51] arm: translate.c: Fix smlald Instruction

[Peter Maydell]

From: Peter Crosthwaite <address@hidden>

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 <address@hidden>
Signed-off-by: Peter Crosthwaite <address@hidden>
Reviewed-by: Peter Maydell <address@hidden>
Message-id: address@hidden
Cc: address@hidden
Signed-off-by: Peter Maydell <address@hidden>
---
 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 0c08cc1..a4d920b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8430,27 +8430,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