Add hydrogen support for ArrayPop, and remove the handwritten call stubs.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 54e7ee82b93e747a1089cf6d4220a41696a38cc0..da112b4558d5e5f8d915060c7c3ab64eee5a100e 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -7614,6 +7614,53 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
         return true;
       }
       break;
+    case kArrayPop: {
+      if (!expr->IsMonomorphic() || expr->check_type() != RECEIVER_MAP_CHECK) {
+        return false;
+      }
+      if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;
+      ElementsKind elements_kind = receiver_map->elements_kind();
+      if (!IsFastElementsKind(elements_kind)) return false;
+      AddCheckConstantFunction(expr->holder(), receiver, receiver_map);
+
+      Drop(expr->arguments()->length());
+      HValue* result;
+      HValue* checked_object;
+      HValue* reduced_length;
+      HValue* receiver = Pop();
+      { NoObservableSideEffectsScope scope(this);
+        checked_object = AddCheckMap(receiver, receiver_map);
+        HValue* elements = AddLoadElements(checked_object);
+        // Ensure that we aren't popping from a copy-on-write array.
+        if (IsFastSmiOrObjectElementsKind(elements_kind)) {
+          Add<HCheckMaps>(
+              elements, isolate()->factory()->fixed_array_map(), top_info());
+        }
+        HValue* length = Add<HLoadNamedField>(
+            checked_object, HObjectAccess::ForArrayLength(elements_kind));
+        reduced_length = AddUncasted<HSub>(length, graph()->GetConstant1());
+        HValue* bounds_check = Add<HBoundsCheck>(
+            graph()->GetConstant0(), length);
+        result = AddElementAccess(elements, reduced_length, NULL,
+                                  bounds_check, elements_kind, false);

[mvstanton, 2014/01/20 12:45:39]

Can we make the boolean parameter to AddElementAccess be a LOAD/STORE enum? It's
a bit jarring because I expect to see some indication in the line that we are
doing a load here and a store below.

[Toon Verwaest, 2014/01/20 13:28:09]

Will do so.

On 2014/01/20 12:45:39, mvstanton wrote:

+        Factory* factory = isolate()->factory();
+        double nan_double = FixedDoubleArray::hole_nan_as_double();
+        HValue* hole = IsFastSmiOrObjectElementsKind(elements_kind)
+            ? Add<HConstant>(factory->the_hole_value())
+            : Add<HConstant>(nan_double);
+        if (IsFastSmiOrObjectElementsKind(elements_kind)) {
+          elements_kind = FAST_HOLEY_ELEMENTS;

[mvstanton, 2014/01/20 12:45:39]

This line confuses me a bit. A) Why wouldn't the elements_kind just change to be
the holey version of whatever it was? B) Why would double be excluded from this
operation? And if A) proves to be good enough, then you could eliminate the
change to elements kind in the case that it's already holey. 

If you have to do this for a technical reason so that that store works out
correctly, add a comment to explain.

[Toon Verwaest, 2014/01/20 13:28:09]

This is the standard way of storing holes into arrays. NaN-double is a double,
so it can directly be stored into a double array.

A hole (*the* hole), however, is a tagged value. If we'd try to store this into
a smi-array, there would be a tagged->smi conversion on the hole value; which
doesn't work. Hence we need to specifically allow this "tagged store" into a smi
array to let the hole go through.

As I said, this isn't necessary in case of double arrays, since double array
contains doubles and NaN in that case is just a special double.

On 2014/01/20 12:45:39, mvstanton wrote:

+        }
+        AddElementAccess(
+            elements, reduced_length, hole, bounds_check, elements_kind, true);
+      }
+      Add<HStoreNamedField>(
+          checked_object, HObjectAccess::ForArrayLength(elements_kind),
+          reduced_length);
+      Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
+      ast_context()->ReturnValue(result);
+      return true;
+    }
     default:
       // Not yet supported for inlining.
       break;