Remove hand-written assembly ArrayPush stubs

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 96f4e7899b9df6211e7468e9047a402efa44c055..98619109d444d60215fba1d25bff33b7e59598de 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -2436,7 +2436,7 @@ HValue* HGraphBuilder::BuildNewElementsCapacity(HValue* old_capacity) {
   HValue* new_capacity = AddUncasted<HAdd>(half_old_capacity, old_capacity);
   new_capacity->ClearFlag(HValue::kCanOverflow);
 
-  HValue* min_growth = Add<HConstant>(16);
+  HValue* min_growth = Add<HConstant>(4);
 
   new_capacity = AddUncasted<HAdd>(new_capacity, min_growth);
   new_capacity->ClearFlag(HValue::kCanOverflow);
@@ -7660,9 +7660,10 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
     }
     case kArrayPush: {
       if (receiver_map.is_null()) return false;
-      if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;

[Toon Verwaest, 2014/04/11 13:06:57]

This is invalid, since .push is spec'ed as reading the length, writing the
element, updating the length. 
1) regular objects don't have a HObjectAccess::ForArrayLength(...) and
2) the length may be observable through accessors.
Additionally, I presume UncheckedMono... doesn't actually do anything to a
"length" field for non-JSArrays, so the fact that the length is updated by .push
is relevant.

Please add tests to this effect.

[danno, 2014/04/16 14:34:38]

Done.

       ElementsKind elements_kind = receiver_map->elements_kind();
       if (!IsFastElementsKind(elements_kind)) return false;
+      if (receiver_map->is_observed()) return false;
+      if (!receiver_map->is_extensible()) return false;

[Toon Verwaest, 2014/04/11 13:06:57]

I think is_extensible isn't necessary cause we guarantee we have dictionary
elements for those cases. We should definitely ASSERT though.

Both these conditions should also hold for ArrayPop. In fact, they are supposed
to hold for all BuildUncheckMonomorphicElementAccess users, so we should ASSERT
them in that method.

[danno, 2014/04/16 14:34:38]

I changed these into ASSERTs and added them to pop, too. Unfortuantely,
BuildUncheckMonomorphicElementAccess doesn't take a map so we can't use it as a
bottleneck.

 
       // If there may be elements accessors in the prototype chain, the fast
       // inlined version can't be used.
@@ -7675,31 +7676,29 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
       Handle<JSObject> prototype(JSObject::cast(receiver_map->prototype()));
       BuildCheckPrototypeMaps(prototype, Handle<JSObject>());
 
-      HValue* op_vals[] = {
-        context(),
-        // Receiver.
-        environment()->ExpressionStackAt(expr->arguments()->length())
-      };
-
       const int argc = expr->arguments()->length();
-      // Includes receiver.
-      PushArgumentsFromEnvironment(argc + 1);
+      if (argc != 1) return false;
 
-      CallInterfaceDescriptor* descriptor =
-          isolate()->call_descriptor(Isolate::CallHandler);
+      HValue* value_to_push = Pop();
+      HValue* array = Pop();
 
-      ArrayPushStub stub(receiver_map->elements_kind(), argc);
-      Handle<Code> code = stub.GetCode(isolate());
-      HConstant* code_value = Add<HConstant>(code);
+      HValue* length = Add<HLoadNamedField>(array, static_cast<HValue*>(NULL),
+          HObjectAccess::ForArrayLength(elements_kind));
 
-      ASSERT((sizeof(op_vals) / kPointerSize) ==
-             descriptor->environment_length());
+      {
+        NoObservableSideEffectsScope scope(this);
+
+        bool is_array = receiver_map->instance_type() == JS_ARRAY_TYPE;
+        BuildUncheckedMonomorphicElementAccess(array, length,
+                                               value_to_push, is_array,
+                                               elements_kind, STORE,
+                                               NEVER_RETURN_HOLE,
+                                               STORE_AND_GROW_NO_TRANSITION);
+      }
 
-      HInstruction* call = New<HCallWithDescriptor>(
-          code_value, argc + 1, descriptor,
-          Vector<HValue*>(op_vals, descriptor->environment_length()));
+      HInstruction* new_size = NewUncasted<HAdd>(length, Add<HConstant>(argc));
       Drop(1);  // Drop function.
-      ast_context()->ReturnInstruction(call, expr->id());
+      ast_context()->ReturnInstruction(new_size, expr->id());
       return true;
     }
     default: