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

src/runtime/runtime-simd.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/runtime/runtime-utils.h"
 
 #include "src/arguments.h"
 #include "src/base/macros.h"
 #include "src/conversions.h"
 #include "src/factory.h"
@@ skipping 150 matching lines @@
   HandleScope scope(isolate);
   DCHECK(args.length() == 1);
   return isolate->heap()->ToBoolean(args[0]->IsSimd128Value());
 }
 
 
 //-------------------------------------------------------------------
 
 // 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;                                                    \
   if (args[index]->Is##Type()) {                                        \
     name = args.at<Type>(index);                                        \
   } else {                                                              \
     THROW_NEW_ERROR_RETURN_FAILURE(                                     \
         isolate, NewTypeError(MessageTemplate::kInvalidSimdOperation)); \
   }
 
@@ skipping 26 matching lines @@
   bool lanes[kLaneCount];                                                 \
   for (int i = 0; i < kLaneCount; i++) {                                  \
     lanes[i] = a->get_lane(i) op b->get_lane(i);                          \
   }                                                                       \
   Handle<bool_type> result = isolate->factory()->New##bool_type(lanes);
 
 //-------------------------------------------------------------------
 
 // 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) \
   name = args[index]->BooleanValue();
 
 #define SIMD_ALL_TYPES(FUNCTION)                              \
   FUNCTION(Float32x4, float, 4, NewNumber, GET_NUMERIC_ARG)   \
   FUNCTION(Int32x4, int32_t, 4, NewNumber, GET_NUMERIC_ARG)   \
   FUNCTION(Uint32x4, uint32_t, 4, NewNumber, GET_NUMERIC_ARG) \
   FUNCTION(Bool32x4, bool, 4, ToBoolean, GET_BOOLEAN_ARG)     \
@@ skipping 156 matching lines @@
 #define SIMD_INT_TYPES(FUNCTION)    \
   FUNCTION(Int32x4, int32_t, 32, 4) \
   FUNCTION(Int16x8, int16_t, 16, 8) \
   FUNCTION(Int8x16, int8_t, 8, 16)
 
 #define SIMD_UINT_TYPES(FUNCTION)     \
   FUNCTION(Uint32x4, uint32_t, 32, 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) \
   RUNTIME_FUNCTION(Runtime_##type##ShiftLeftByScalar) {           \
     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] = 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)      \
   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);                               \
-    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; \
       lanes[i] = static_cast<lane_type>(shifted);                      \
     }                                                                  \
     Handle<type> result = isolate->factory()->New##type(lanes);        \
     return *result;                                                    \
   }
 
 SIMD_INT_TYPES(SIMD_LSL_FUNCTION)
@@ skipping 320 matching lines @@
   FUNCTION(Uint8x16, uint8_t, 16, Int8x16, int8_t)
 
 #define SIMD_FROM_FUNCTION(type, lane_type, lane_count, from_type, from_ctype) \
   RUNTIME_FUNCTION(Runtime_##type##From##from_type) {                          \
     static const int kLaneCount = lane_count;                                  \
     HandleScope scope(isolate);                                                \
     DCHECK(args.length() == 1);                                                \
     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);                \
     return *result;                                                            \
   }
 
 SIMD_FROM_TYPES(SIMD_FROM_FUNCTION)
 
 #define SIMD_FROM_BITS_TYPES(FUNCTION)       \
   FUNCTION(Float32x4, float, 4, Int32x4)     \
@@ skipping 56 matching lines @@
 
 //-------------------------------------------------------------------
 
 // Load and Store functions.
 
 #define SIMD_LOADN_STOREN_TYPES(FUNCTION) \
   FUNCTION(Float32x4, float, 4)           \
   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
 
 #define SIMD_LOAD(type, lane_type, lane_count, count, result)          \
   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()) +    \
       tarray_offset;                                                   \
   lane_type lanes[kLaneCount] = {0};                                   \
   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()) +    \
       tarray_offset;                                                   \
   lane_type lanes[kLaneCount];                                         \
   for (int i = 0; i < kLaneCount; i++) {                               \
     lanes[i] = a->get_lane(i);                                         \
   }                                                                    \
   memcpy(tarray_base + index * bpe, lanes, bytes);
 
-
 #define SIMD_LOAD_FUNCTION(type, lane_type, lane_count)         \
   RUNTIME_FUNCTION(Runtime_##type##Load) {                      \
     HandleScope scope(isolate);                                 \
     SIMD_LOAD(type, lane_type, lane_count, lane_count, result); \
     return *result;                                             \
   }
 
 
 #define SIMD_LOAD1_FUNCTION(type, lane_type, lane_count) \
   RUNTIME_FUNCTION(Runtime_##type##Load1) {              \
@@ skipping 57 matching lines @@
 SIMD_LOADN_STOREN_TYPES(SIMD_LOAD3_FUNCTION)
 SIMD_NUMERIC_TYPES(SIMD_STORE_FUNCTION)
 SIMD_LOADN_STOREN_TYPES(SIMD_STORE1_FUNCTION)
 SIMD_LOADN_STOREN_TYPES(SIMD_STORE2_FUNCTION)
 SIMD_LOADN_STOREN_TYPES(SIMD_STORE3_FUNCTION)
 
 //-------------------------------------------------------------------
 
 }  // namespace internal
 }  // namespace v8