[simdjs] Implement error raising semantics as per spec for integers.

src/runtime/runtime-simd.cc

Index: src/runtime/runtime-simd.cc
diff --git a/src/runtime/runtime-simd.cc b/src/runtime/runtime-simd.cc
index 9e5614242a656d882f31856c84519eb12d715d8b..33ee19e0f0d7a12e8f372e57ffef7a6eeb5d7a2d 100644
--- a/src/runtime/runtime-simd.cc
+++ b/src/runtime/runtime-simd.cc
@@ -168,9 +168,17 @@ RUNTIME_FUNCTION(Runtime_IsSimdValue) {
 
 // Utility macros.
 
-#define CONVERT_SIMD_LANE_ARG_CHECKED(name, index, lanes) \
-  CONVERT_INT32_ARG_CHECKED(name, index);                 \
-  RUNTIME_ASSERT(name >= 0 && name < lanes);
+#define CONVERT_SIMD_LANE_ARG_CHECKED(name, index, lanes)                  \
+  Handle<Object> name_object = args.at<Object>(index);                     \
+  if (!name_object->IsNumber() || !IsInt32Double(name_object->Number())) { \

[bbudge, 2016/05/18 09:31:13]

IsInt32Double rejects -0. Reading the spec, it looks like the conversion allows
-0 (SameValueZero considers +0 and -0 equivalent)

http://tc39.github.io/ecmascript_simd/#simd-to-lane

Is the spec up to date?

[gdeepti, 2016/05/19 12:11:56]

The Spec specifies ToNumber conversion, but that has not been implemented in the
polyfill yet. Added that in my first patch but had to revert as tryjobs fail
because we run tests against the polyfill. Just refactoring  here to get rid of
runtime asserts. Will update once the polyfill is in sync with spec or Test262
version is rolled forward and we can use that instead. Added a TODO.

+    THROW_NEW_ERROR_RETURN_FAILURE(                                        \
+        isolate, NewTypeError(MessageTemplate::kInvalidSimdOperation));    \

[bbudge, 2016/05/18 09:31:13]

It seems like kInvalidSimdIndex would be a more informative error.

[gdeepti, 2016/05/19 12:11:56]

Done.

+  }                                                                        \
+  uint32_t name = name_object->Number();                                   \
+  if (name < 0 || name >= lanes) {                                         \
+    THROW_NEW_ERROR_RETURN_FAILURE(                                        \
+        isolate, NewRangeError(MessageTemplate::kInvalidSimdIndex));       \
+  }
 
 #define CONVERT_SIMD_ARG_HANDLE_THROW(Type, name, index)                \
   Handle<Type> name;                                                    \
@@ -217,8 +225,10 @@ RUNTIME_FUNCTION(Runtime_IsSimdValue) {
 
 // Common functions.
 
-#define GET_NUMERIC_ARG(lane_type, name, index) \
-  CONVERT_NUMBER_ARG_HANDLE_CHECKED(a, index);  \
+#define GET_NUMERIC_ARG(lane_type, name, index)              \
+  Handle<Object> a;                                          \
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(                        \
+      isolate, a, Object::ToNumber(args.at<Object>(index))); \
   name = ConvertNumber<lane_type>(a->Number());
 
 #define GET_BOOLEAN_ARG(lane_type, name, index) \
@@ -395,10 +405,14 @@ SIMD_MAXNUM_FUNCTION(Float32x4, float, 4)
   FUNCTION(Uint16x8, uint16_t, 16, 8) \
   FUNCTION(Uint8x16, uint8_t, 8, 16)
 
-#define CONVERT_SHIFT_ARG_CHECKED(name, index)         \
-  RUNTIME_ASSERT(args[index]->IsNumber());             \
-  int32_t signed_shift = 0;                            \
-  RUNTIME_ASSERT(args[index]->ToInt32(&signed_shift)); \
+#define CONVERT_SHIFT_ARG_CHECKED(name, index)                             \
+  Handle<Object> name_object = args.at<Object>(index);                     \
+  if (!name_object->IsNumber() || !IsInt32Double(name_object->Number())) { \

[bbudge, 2016/05/18 09:31:13]

From the spec, the conversion for shift counts is ToUint32:

https://tc39.github.io/ecma262/#sec-touint32

[gdeepti, 2016/05/19 12:11:56]

Not sure if I'm understanding this correctly, the conversion for shift counts is
ToUint32 below, so the only thing required is to remove the IsInt32Double check
as only IsNumber is required?

+    THROW_NEW_ERROR_RETURN_FAILURE(                                        \
+        isolate, NewTypeError(MessageTemplate::kInvalidSimdOperation));    \
+  }                                                                        \
+  int32_t signed_shift = 0;                                                \
+  RUNTIME_ASSERT(args[index]->ToInt32(&signed_shift));                     \
   uint32_t name = bit_cast<uint32_t>(signed_shift);
 
 #define SIMD_LSL_FUNCTION(type, lane_type, lane_bits, lane_count) \
@@ -409,31 +423,29 @@ SIMD_MAXNUM_FUNCTION(Float32x4, float, 4)
     CONVERT_SIMD_ARG_HANDLE_THROW(type, a, 0);                    \
     CONVERT_SHIFT_ARG_CHECKED(shift, 1);                          \
     lane_type lanes[kLaneCount] = {0};                            \
-    if (shift < lane_bits) {                                      \
-      for (int i = 0; i < kLaneCount; i++) {                      \
-        lanes[i] = a->get_lane(i) << shift;                       \
-      }                                                           \
+    shift &= lane_bits - 1;                                       \
+    for (int i = 0; i < kLaneCount; i++) {                        \
+      lanes[i] = a->get_lane(i) << shift;                         \
     }                                                             \
     Handle<type> result = isolate->factory()->New##type(lanes);   \
     return *result;                                               \
   }
 
-#define SIMD_LSR_FUNCTION(type, lane_type, lane_bits, lane_count) \
-  RUNTIME_FUNCTION(Runtime_##type##ShiftRightByScalar) {          \
-    static const int kLaneCount = lane_count;                     \
-    HandleScope scope(isolate);                                   \
-    DCHECK(args.length() == 2);                                   \
-    CONVERT_SIMD_ARG_HANDLE_THROW(type, a, 0);                    \
-    CONVERT_SHIFT_ARG_CHECKED(shift, 1);                          \
-    lane_type lanes[kLaneCount] = {0};                            \
-    if (shift < lane_bits) {                                      \
-      for (int i = 0; i < kLaneCount; i++) {                      \
-        lanes[i] = static_cast<lane_type>(                        \
-            bit_cast<lane_type>(a->get_lane(i)) >> shift);        \
-      }                                                           \
-    }                                                             \
-    Handle<type> result = isolate->factory()->New##type(lanes);   \
-    return *result;                                               \
+#define SIMD_LSR_FUNCTION(type, lane_type, lane_bits, lane_count)              \
+  RUNTIME_FUNCTION(Runtime_##type##ShiftRightByScalar) {                       \
+    static const int kLaneCount = lane_count;                                  \
+    HandleScope scope(isolate);                                                \
+    DCHECK(args.length() == 2);                                                \
+    CONVERT_SIMD_ARG_HANDLE_THROW(type, a, 0);                                 \
+    CONVERT_SHIFT_ARG_CHECKED(shift, 1);                                       \
+    lane_type lanes[kLaneCount] = {0};                                         \
+    shift &= lane_bits - 1;                                                    \
+    for (int i = 0; i < kLaneCount; i++) {                                     \
+      lanes[i] = static_cast<lane_type>(bit_cast<lane_type>(a->get_lane(i)) >> \
+                                        shift);                                \
+    }                                                                          \
+    Handle<type> result = isolate->factory()->New##type(lanes);                \
+    return *result;                                                            \
   }
 
 #define SIMD_ASR_FUNCTION(type, lane_type, lane_bits, lane_count)      \
@@ -443,7 +455,7 @@ SIMD_MAXNUM_FUNCTION(Float32x4, float, 4)
     DCHECK(args.length() == 2);                                        \
     CONVERT_SIMD_ARG_HANDLE_THROW(type, a, 0);                         \
     CONVERT_SHIFT_ARG_CHECKED(shift, 1);                               \
-    if (shift >= lane_bits) shift = lane_bits - 1;                     \
+    shift &= lane_bits - 1;                                            \
     lane_type lanes[kLaneCount];                                       \
     for (int i = 0; i < kLaneCount; i++) {                             \
       int64_t shifted = static_cast<int64_t>(a->get_lane(i)) >> shift; \
@@ -784,9 +796,11 @@ SIMD_SIGNED_TYPES(SIMD_NEG_FUNCTION)
     CONVERT_SIMD_ARG_HANDLE_THROW(from_type, a, 0);                            \
     lane_type lanes[kLaneCount];                                               \
     for (int i = 0; i < kLaneCount; i++) {                                     \
-      from_ctype a_value = a->get_lane(i);                                     \
-      if (a_value != a_value) a_value = 0;                                     \
-      RUNTIME_ASSERT(CanCast<lane_type>(a_value));                             \
+      from_ctype a_value = std::trunc(a->get_lane(i));                         \
+      if (a_value != a_value || !CanCast<lane_type>(a_value)) {                \
+        THROW_NEW_ERROR_RETURN_FAILURE(                                        \
+            isolate, NewRangeError(MessageTemplate::kInvalidSimdIndex));       \

[bbudge, 2016/05/18 09:31:13]

Invalid SIMD index doesn't seem right here since these are lane values.

[gdeepti, 2016/05/19 12:11:56]

Done.

+      }                                                                        \
       lanes[i] = static_cast<lane_type>(a_value);                              \
     }                                                                          \
     Handle<type> result = isolate->factory()->New##type(lanes);                \
@@ -863,6 +877,15 @@ SIMD_FROM_BITS_TYPES(SIMD_FROM_BITS_FUNCTION)
   FUNCTION(Int32x4, int32_t, 4)           \
   FUNCTION(Uint32x4, uint32_t, 4)
 
+#define SIMD_COERCE_INDEX(name, i)                                          \
+  Handle<Object> name_object;                                               \
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, name_object,                  \
+                                     Object::ToNumber(args.at<Object>(i))); \
+  if (name_object->Number() != std::floor(name_object->Number())) {         \

[bbudge, 2016/05/18 09:31:13]

The spec says the conversion is ToLength.

http://tc39.github.io/ecmascript_simd/#simd-load-from-tarray

[gdeepti, 2016/05/19 12:11:56]

Added  index ≠ ToLength(index) check as per spec.

+    THROW_NEW_ERROR_RETURN_FAILURE(                                         \
+        isolate, NewRangeError(MessageTemplate::kInvalidSimdIndex));        \
+  }                                                                         \
+  int32_t name = name_object->Number();
 
 // Common Load and Store Functions
 
@@ -870,11 +893,14 @@ SIMD_FROM_BITS_TYPES(SIMD_FROM_BITS_FUNCTION)
   static const int kLaneCount = lane_count;                            \
   DCHECK(args.length() == 2);                                          \
   CONVERT_SIMD_ARG_HANDLE_THROW(JSTypedArray, tarray, 0);              \
-  CONVERT_INT32_ARG_CHECKED(index, 1)                                  \
+  SIMD_COERCE_INDEX(index, 1);                                         \
   size_t bpe = tarray->element_size();                                 \
   uint32_t bytes = count * sizeof(lane_type);                          \
   size_t byte_length = NumberToSize(isolate, tarray->byte_length());   \
-  RUNTIME_ASSERT(index >= 0 && index * bpe + bytes <= byte_length);    \
+  if (index < 0 || index * bpe + bytes > byte_length) {                \
+    THROW_NEW_ERROR_RETURN_FAILURE(                                    \
+        isolate, NewRangeError(MessageTemplate::kInvalidSimdIndex));   \
+  }                                                                    \
   size_t tarray_offset = NumberToSize(isolate, tarray->byte_offset()); \
   uint8_t* tarray_base =                                               \
       static_cast<uint8_t*>(tarray->GetBuffer()->backing_store()) +    \
@@ -883,17 +909,19 @@ SIMD_FROM_BITS_TYPES(SIMD_FROM_BITS_FUNCTION)
   memcpy(lanes, tarray_base + index * bpe, bytes);                     \
   Handle<type> result = isolate->factory()->New##type(lanes);
 
-
 #define SIMD_STORE(type, lane_type, lane_count, count, a)              \
   static const int kLaneCount = lane_count;                            \
   DCHECK(args.length() == 3);                                          \
   CONVERT_SIMD_ARG_HANDLE_THROW(JSTypedArray, tarray, 0);              \
   CONVERT_SIMD_ARG_HANDLE_THROW(type, a, 2);                           \
-  CONVERT_INT32_ARG_CHECKED(index, 1)                                  \
+  SIMD_COERCE_INDEX(index, 1);                                         \
   size_t bpe = tarray->element_size();                                 \
   uint32_t bytes = count * sizeof(lane_type);                          \
   size_t byte_length = NumberToSize(isolate, tarray->byte_length());   \
-  RUNTIME_ASSERT(index >= 0 && index * bpe + bytes <= byte_length);    \
+  if (index < 0 || byte_length < index * bpe + bytes) {                \
+    THROW_NEW_ERROR_RETURN_FAILURE(                                    \
+        isolate, NewRangeError(MessageTemplate::kInvalidSimdIndex));   \
+  }                                                                    \
   size_t tarray_offset = NumberToSize(isolate, tarray->byte_offset()); \
   uint8_t* tarray_base =                                               \
       static_cast<uint8_t*>(tarray->GetBuffer()->backing_store()) +    \
@@ -904,7 +932,6 @@ SIMD_FROM_BITS_TYPES(SIMD_FROM_BITS_FUNCTION)
   }                                                                    \
   memcpy(tarray_base + index * bpe, lanes, bytes);
 
-
 #define SIMD_LOAD_FUNCTION(type, lane_type, lane_count)         \
   RUNTIME_FUNCTION(Runtime_##type##Load) {                      \
     HandleScope scope(isolate);                                 \