Migrate %TypedArray%.prototype.fill to C++

src/elements.cc

Index: src/elements.cc
diff --git a/src/elements.cc b/src/elements.cc
index 7b7f6893d84b557224acb1232b7798eb39cbb461..7e8418c07dd1752c6f80ef5070e4d4490ed9b092 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -470,6 +470,28 @@ static void SortIndices(
   }
 }
 
+static Object* FillNumberSlowPath(Isolate* isolate, Handle<JSTypedArray> array,
+                               Handle<Object> obj_value,
+                               uint32_t start, uint32_t end) {
+  const char* kMethodName = "%TypedArray%.prototype.fill";
+  Handle<Object> cast_value;
+  ElementsAccessor* elements = array->GetElementsAccessor();
+
+  for (uint32_t k = start; k < end; ++k) {
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, cast_value, Object::ToNumber(obj_value));
+    // IntegerIndexedElementSet step 5
+    if (V8_UNLIKELY(array->WasNeutered())) {

[Camillo Bruni, 2017/03/13 08:12:58]

Can you add a "TODO(caitp,cbruni): throw on neutered array" and just "return
*array"
This is a recent spec change that hasn't been implemented yet.

[rongjie, 2017/03/13 08:43:01]

Done.

+      THROW_NEW_ERROR_RETURN_FAILURE(
+          isolate, NewTypeError(MessageTemplate::kDetachedOperation,
+                                isolate->factory()->NewStringFromAsciiChecked(
+                                kMethodName)));
+    }
+    elements->Set(array, k, *cast_value);
+  }
+  return *array;
+}
+
 static Maybe<bool> IncludesValueSlowPath(Isolate* isolate,
                                          Handle<JSObject> receiver,
                                          Handle<Object> value,
@@ -1191,6 +1213,17 @@ class ElementsAccessorBase : public ElementsAccessor {
     return Subclass::GetCapacityImpl(holder, backing_store);
   }
 
+  static Object* FillImpl(Isolate* isolate, Handle<JSObject> receiver,
+                       Handle<Object> obj_value, uint32_t start, uint32_t end) {
+    UNREACHABLE();
+    return *receiver;
+  }
+
+  Object* Fill(Isolate* isolate, Handle<JSObject> receiver,
+            Handle<Object> obj_value, uint32_t start, uint32_t end) {
+    return Subclass::FillImpl(isolate, receiver, obj_value, start, end);
+  }
+
   static Maybe<bool> IncludesValueImpl(Isolate* isolate,
                                        Handle<JSObject> receiver,
                                        Handle<Object> value,
@@ -2815,6 +2848,33 @@ class TypedElementsAccessor
     return Just(true);
   }
 
+  static Object* FillImpl(Isolate* isolate, Handle<JSObject> receiver,
+                       Handle<Object> obj_value, uint32_t start, uint32_t end) {
+    Handle<JSTypedArray> array = Handle<JSTypedArray>::cast(receiver);
+    if (obj_value->IsNumber()) {

[Camillo Bruni, 2017/03/13 08:12:58]

nit: I prefer early returns and non-indented code :P
if (!obj_value->IsNumber) {
  return FillNumberSlowPath(isolate, array, obj_value, start, end):
} 

[rongjie, 2017/03/13 08:43:01]

Done.

+      double value = 0.0;

[Camillo Bruni, 2017/03/13 08:12:58]

ctype value = 0; 
This way you can avoid an unnecessary double cast in the Smi case for integer
arrays, hope this works :)

[rongjie, 2017/03/13 08:43:02]

Done.

+
+      if (obj_value->IsSmi()) {
+        value = Smi::cast(*obj_value)->value();

[Camillo Bruni, 2017/03/13 08:12:58]

value = static_cast<ctype>(Smi::cast(...));

[rongjie, 2017/03/13 08:43:02]

Done.

+      } else {
+        DCHECK(obj_value->IsHeapNumber());
+        value = HeapNumber::cast(*obj_value)->value();

[Camillo Bruni, 2017/03/13 08:12:58]

same here..

[rongjie, 2017/03/13 08:43:01]

Done.

+      }
+      if (array->type() == kExternalUint8ClampedArray) {
+        value = std::min<double>(std::max<double>(0, value), 255);
+      }
+
+      Handle<FixedTypedArrayBase> elements(
+          FixedTypedArrayBase::cast(array->elements()));
+      ctype* data = static_cast<ctype*>(elements->DataPtr());
+      ctype cast_value = static_cast<ctype>(value);
+      std::fill(data + start, data + end, cast_value);

[Camillo Bruni, 2017/03/13 08:12:58]

Please add DCHECKS here as well for the range parameters.

[rongjie, 2017/03/13 08:43:01]

To ensure array->length_value() is not executed in release build, I fit it in
DCHECK.

+      return *array;
+    } else {
+      return FillNumberSlowPath(isolate, array, obj_value, start, end);
+    }
+  }
+
   static Maybe<bool> IncludesValueImpl(Isolate* isolate,
                                        Handle<JSObject> receiver,
                                        Handle<Object> value,