MSP430: sim: Fix incorrect simulation of unsigned widening multiply

Operand sizes used for simulation of MSP430 hardware multiply
operations are not aligned with the sizes used on the target, resulting
in the simulator storing signed operands with too much precision.

Additionally, simulation of unsigned multiplication is missing explicit
casts to prevent any implicit sign extension.

gcc.c-torture/execute/pr91450-1.c uses unsigned widening multiplication
of 32-bit operands -4 and 2, to produce a 64-bit result:
0xffff fffc * 0x2 = 0x1 ffff fff8

If -4 is stored in 64-bit precision, then the multiplication is
essentially signed and the result is -8 in 64-bit precision
(0xffff ffff ffff fffc), which is not correct.

sim/msp430/ChangeLog:

	* msp430-sim.c (put_op): For unsigned multiplication, explicitly cast
	operands to the unsigned type before multiplying.
	* msp430-sim.h (struct msp430_cpu_state): Fix types used to store hwmult
	operands.

sim/testsuite/sim/msp430/ChangeLog:

	* mpyull_hwmult.s: New test.

diff --git a/sim/msp430/ChangeLog b/sim/msp430/ChangeLog
index 0f27982068c8a15c620d08757225914f98db3ec2..41da2a734bd05cad8deb08bd8c12bf2fc5a60f08 100644 (file)
--- a/sim/msp430/ChangeLog
+++ b/sim/msp430/ChangeLog
@@ -1,3 +1,10 @@
+2020-08-05  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
+
+       * msp430-sim.c (put_op): For unsigned multiplication, explicitly cast
+       operands to the unsigned type before multiplying.
+       * msp430-sim.h (struct msp430_cpu_state): Fix types used to store hwmult
+       operands.
+
 2020-01-22  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
 
        * msp430-sim.c (msp430_step_once): Ignore the carry flag when executing

diff --git a/sim/msp430/msp430-sim.c b/sim/msp430/msp430-sim.c
index e21c8cf6a649703a6a46b80849fc4bf689f9d73a..a330c6caf5dba7dd747426b31c16ef0ff9ea8b30 100644 (file)
--- a/sim/msp430/msp430-sim.c
+++ b/sim/msp430/msp430-sim.c
@@ -566,8 +566,13 @@ put_op (SIM_DESC sd, MSP430_Opcode_Decoded *opc, int n, int val)
              switch (HWMULT (sd, hwmult_type))
                {
                case UNSIGN_32:
-                 HWMULT (sd, hwmult_result) = HWMULT (sd, hwmult_op1) * HWMULT (sd, hwmult_op2);
-                 HWMULT (sd, hwmult_signed_result) = (signed) HWMULT (sd, hwmult_result);
+                 a = HWMULT (sd, hwmult_op1);
+                 b = HWMULT (sd, hwmult_op2);
+                 /* For unsigned 32-bit multiplication of 16-bit operands, an
+                    explicit cast is required to prevent any implicit
+                    sign-extension.  */
+                 HWMULT (sd, hwmult_result) = (unsigned32) a * (unsigned32) b;
+                 HWMULT (sd, hwmult_signed_result) = a * b;
                  HWMULT (sd, hwmult_accumulator) = HWMULT (sd, hwmult_signed_accumulator) = 0;
                  break;
 
@@ -575,13 +580,16 @@ put_op (SIM_DESC sd, MSP430_Opcode_Decoded *opc, int n, int val)
                  a = sign_ext (HWMULT (sd, hwmult_op1), 16);
                  b = sign_ext (HWMULT (sd, hwmult_op2), 16);
                  HWMULT (sd, hwmult_signed_result) = a * b;
-                 HWMULT (sd, hwmult_result) = (unsigned) HWMULT (sd, hwmult_signed_result);
+                 HWMULT (sd, hwmult_result) = (unsigned32) a * (unsigned32) b;
                  HWMULT (sd, hwmult_accumulator) = HWMULT (sd, hwmult_signed_accumulator) = 0;
                  break;
 
                case UNSIGN_MAC_32:
-                 HWMULT (sd, hwmult_accumulator) += HWMULT (sd, hwmult_op1) * HWMULT (sd, hwmult_op2);
-                 HWMULT (sd, hwmult_signed_accumulator) += HWMULT (sd, hwmult_op1) * HWMULT (sd, hwmult_op2);
+                 a = HWMULT (sd, hwmult_op1);
+                 b = HWMULT (sd, hwmult_op2);
+                 HWMULT (sd, hwmult_accumulator)
+                   += (unsigned32) a * (unsigned32) b;
+                 HWMULT (sd, hwmult_signed_accumulator) += a * b;
                  HWMULT (sd, hwmult_result) = HWMULT (sd, hwmult_accumulator);
                  HWMULT (sd, hwmult_signed_result) = HWMULT (sd, hwmult_signed_accumulator);
                  break;
@@ -589,7 +597,8 @@ put_op (SIM_DESC sd, MSP430_Opcode_Decoded *opc, int n, int val)
                case SIGN_MAC_32:
                  a = sign_ext (HWMULT (sd, hwmult_op1), 16);
                  b = sign_ext (HWMULT (sd, hwmult_op2), 16);
-                 HWMULT (sd, hwmult_accumulator) += a * b;
+                 HWMULT (sd, hwmult_accumulator)
+                   += (unsigned32) a * (unsigned32) b;
                  HWMULT (sd, hwmult_signed_accumulator) += a * b;
                  HWMULT (sd, hwmult_result) = HWMULT (sd, hwmult_accumulator);
                  HWMULT (sd, hwmult_signed_result) = HWMULT (sd, hwmult_signed_accumulator);
@@ -648,10 +657,13 @@ put_op (SIM_DESC sd, MSP430_Opcode_Decoded *opc, int n, int val)
              switch (HWMULT (sd, hw32mult_type))
                {
                case UNSIGN_64:
-                 HWMULT (sd, hw32mult_result) = HWMULT (sd, hw32mult_op1) * HWMULT (sd, hw32mult_op2);
+                 HWMULT (sd, hw32mult_result)
+                   = (unsigned64) HWMULT (sd, hw32mult_op1)
+                   * (unsigned64) HWMULT (sd, hw32mult_op2);
                  break;
                case SIGN_64:
-                 HWMULT (sd, hw32mult_result) = sign_ext (HWMULT (sd, hw32mult_op1), 32)
+                 HWMULT (sd, hw32mult_result)
+                   = sign_ext (HWMULT (sd, hw32mult_op1), 32)
                    * sign_ext (HWMULT (sd, hw32mult_op2), 32);
                  break;
                }

diff --git a/sim/msp430/msp430-sim.h b/sim/msp430/msp430-sim.h
index ad83e5b6ae6806f23ffd0b5f037e34c221fd990b..7c486c2f3502fa9d572e8a07349d96e844c17d78 100644 (file)
--- a/sim/msp430/msp430-sim.h
+++ b/sim/msp430/msp430-sim.h
@@ -31,16 +31,16 @@ struct msp430_cpu_state
   int cio_buffer;
 
   hwmult_type  hwmult_type;
-  unsigned32   hwmult_op1;
-  unsigned32   hwmult_op2;
+  unsigned16   hwmult_op1;
+  unsigned16   hwmult_op2;
   unsigned32   hwmult_result;
   signed32     hwmult_signed_result;
   unsigned32   hwmult_accumulator;
   signed32     hwmult_signed_accumulator;
 
   hw32mult_type  hw32mult_type;
-  unsigned64     hw32mult_op1;
-  unsigned64     hw32mult_op2;
+  unsigned32     hw32mult_op1;
+  unsigned32     hw32mult_op2;
   unsigned64     hw32mult_result;
 };
 

diff --git a/sim/testsuite/sim/msp430/ChangeLog b/sim/testsuite/sim/msp430/ChangeLog
index 7dc370f0a41c37ded41f643a49eff727e95216a3..cd6b1952adcbd6d3987d5cb1edf4abb25b9d2f8d 100644 (file)
--- a/sim/testsuite/sim/msp430/ChangeLog
+++ b/sim/testsuite/sim/msp430/ChangeLog
@@ -1,3 +1,7 @@
+2020-08-05  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
+
+       * mpyull_hwmult.s: New test.
+
 2020-01-22  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
 
        * rrux.s: New test.

diff --git a/sim/testsuite/sim/msp430/mpyull_hwmult.s b/sim/testsuite/sim/msp430/mpyull_hwmult.s
new file mode 100644 (file)
index 0000000..911fa11
--- /dev/null
+++ b/sim/testsuite/sim/msp430/mpyull_hwmult.s
@@ -0,0 +1,55 @@
+# Test that unsigned widening multiplication of 32-bit operands to produce a
+# 64-bit result is simulated correctly, when using 32-bit or F5series hardware
+# multiply functionality.
+# 0xffff fffc * 0x2 = 0x1 ffff fff8
+# mach: msp430
+
+# 32-bit hwmult register addresses
+.set MPY32L,   0x0140
+.set MPY32H,   0x0142
+.set OP2L,     0x0150
+.set OP2H,     0x0152
+.set RES0,     0x0154
+.set RES1,     0x0156
+.set RES2,     0x0158
+.set RES3,     0x015A
+
+# F5series hwmult register addresses
+.set MPY32L_F5,                0x04D0
+.set MPY32H_F5,                0x04D2
+.set OP2L_F5,          0x04E0
+.set OP2H_F5,          0x04E2
+.set RES0_F5,          0x04E4
+.set RES1_F5,          0x04E6
+.set RES2_F5,          0x04E8
+.set RES3_F5,          0x04EA
+
+.include "testutils.inc"
+
+       start
+
+       ; Test 32bit hwmult
+       MOV.W   #2, &MPY32L             ; Load operand 1 Low into multiplier
+       MOV.W   #0, &MPY32H             ; Load operand 1 High into multiplier
+       MOV.W   #-4, &OP2L              ; Load operand 2 Low into multiplier
+       MOV.W   #-1, &OP2H              ; Load operand 2 High, trigger MPY
+
+       CMP.W   #-8, &RES0      { JNE   .L5
+       CMP.W   #-1, &RES1      { JNE   .L5
+       CMP.W   #1, &RES2       { JNE   .L5
+       CMP.W   #0, &RES3       { JNE   .L5
+
+       ; Test f5series hwmult
+       MOV.W   #2, &MPY32L_F5
+       MOV.W   #0, &MPY32H_F5
+       MOV.W   #-4, &OP2L_F5
+       MOV.W   #-1, &OP2H_F5
+
+       CMP.W   #-8, &RES0_F5   { JNE   .L5
+       CMP.W   #-1, &RES1_F5   { JNE   .L5
+       CMP.W   #1, &RES2_F5    { JNE   .L5
+       CMP.W   #0, &RES3_F5    { JEQ   .L6
+.L5:
+       fail
+.L6:
+       pass