Fix q15 saturating mul

Forgot to saturate the result in both test generation code and interpreter. This fixes both cases.
ngzhian · Feb 25, 2021 · 770ce64 · 770ce64
1 parent b4fde03
commit 770ce64
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 15 deletions.
diff --git a/interpreter/exec/int.ml b/interpreter/exec/int.ml
@@ -296,7 +296,7 @@ struct
     assert (Rep.bitwidth < 32);
     let x64 = Rep.to_int64 x in
     let y64 = Rep.to_int64 y in
-    Rep.of_int64 Int64.((shift_right (add (mul x64 y64) 0x4000L) 15))
+    saturate_s (Rep.of_int64 Int64.((shift_right (add (mul x64 y64) 0x4000L) 15)))
 
   let to_int_s = Rep.to_int
   let to_int_u i = Rep.to_int i land (Rep.to_int Rep.max_int lsl 1) lor 1

diff --git a/test/core/simd/meta/simd_integer_op.py b/test/core/simd/meta/simd_integer_op.py
@@ -68,10 +68,7 @@ def _saturate(self, operand1: int, operand2: int, lane: LaneValue) -> int:
             if self.op.startswith('sub'):
                 value = operand1 - operand2
 
-            if value > lane.max:
-                return lane.max
-            if value < lane.min:
-                return lane.min
+            return lane.sat_s(value)
 
         if self.op.endswith('sat_u'):
             if operand1 < 0:
@@ -83,10 +80,7 @@ def _saturate(self, operand1: int, operand2: int, lane: LaneValue) -> int:
             if self.op.startswith('sub'):
                 value = operand1 - operand2
 
-            if value > lane.mask:
-                return lane.mask
-            if value < 0:
-                return 0
+            return lane.sat_u(value)
 
         return value
 
@@ -122,10 +116,10 @@ def unary_op(self, operand, lane):
             return str(bin(result % lane.mod).count('1'))
         elif self.op == 'sat_s':
             # Don't call get_valid_value, it will truncate results.
-            return max(lane.min, min(v, lane.max))
+            return lane.sat_s(v)
         elif self.op == 'sat_u':
             # Don't call get_valid_value, it will truncate results.
-            return max(0, min(v, lane.mask))
+            return lane.sat_u(v)
         else:
             raise Exception('Unknown unary operation')
 
@@ -181,8 +175,7 @@ def binary_op(self, operand1, operand2, src_lane, dst_lane=None):
             # This should be before 'sat' case.
             i1 = ArithmeticOp.get_valid_value(v1, src_lane)
             i2 = ArithmeticOp.get_valid_value(v2, src_lane)
-            result = (i1 * i2 + 0x4000) >> 15
-            return ArithmeticOp.get_valid_value(result, src_lane)
+            return src_lane.sat_s((i1 * i2 + 0x4000) >> 15)
         elif 'sat' in self.op:
             value = self._saturate(v1, v2, src_lane)
             if self.op.endswith('_u'):

diff --git a/test/core/simd/meta/simd_lane_value.py b/test/core/simd/meta/simd_lane_value.py
@@ -29,4 +29,10 @@ def mod(self):
 
     @property
     def quarter(self):
-        return pow(2, self.lane_width - 2)
+        return pow(2, self.lane_width - 2)
+
+    def sat_s(self, v):
+        return max(self.min, min(v, self.max))
+
+    def sat_u(self, v):
+        return max(0, min(v, self.mask))
diff --git a/test/core/simd/simd_i16x8_q15mulr_sat_s.wast b/test/core/simd/simd_i16x8_q15mulr_sat_s.wast
@@ -63,7 +63,7 @@
                                              (v128.const i16x8 32766 32766 32766 32766 32766 32766 32766 32766))
 (assert_return (invoke "i16x8.q15mulr_sat_s" (v128.const i16x8 -32768 -32768 -32768 -32768 -32768 -32768 -32768 -32768)
                                              (v128.const i16x8 -32768 -32768 -32768 -32768 -32768 -32768 -32768 -32768))
-                                             (v128.const i16x8 -32768 -32768 -32768 -32768 -32768 -32768 -32768 -32768))
+                                             (v128.const i16x8 32767 32767 32767 32767 32767 32767 32767 32767))
 (assert_return (invoke "i16x8.q15mulr_sat_s" (v128.const i16x8 -32768 -32768 -32768 -32768 -32768 -32768 -32768 -32768)
                                              (v128.const i16x8 -32767 -32767 -32767 -32767 -32767 -32767 -32767 -32767))
                                              (v128.const i16x8 32767 32767 32767 32767 32767 32767 32767 32767))