Extend ArrayPop hydrogen support to COW arrays and popping from empty arrays.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 142c92171048e44b92b22998da9be366c4ae9cec..58b2d790adbaa6c5ccd6adfe145ff862cdd83700 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -7632,19 +7632,29 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
 
       Drop(expr->arguments()->length());
       HValue* result;
-      HValue* checked_object;
       HValue* reduced_length;
       HValue* receiver = Pop();
+
+      HValue* checked_object = AddCheckMap(receiver, receiver_map);
+      HValue* length = Add<HLoadNamedField>(
+          checked_object, HObjectAccess::ForArrayLength(elements_kind));
+
       { NoObservableSideEffectsScope scope(this);
-        checked_object = AddCheckMap(receiver, receiver_map);
+        IfBuilder length_checker(this);
+
+        length_checker.If<HCompareNumericAndBranch>(
+            length, graph()->GetConstant0(), Token::EQ);
+        length_checker.Then();
+
+        Push(graph()->GetConstantUndefined());
+
+        length_checker.Else();
         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());
+          elements = BuildCopyElementsOnWrite(checked_object, elements,
+                                              elements_kind, length);
         }
-        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);
@@ -7660,13 +7670,18 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
         }
         AddElementAccess(
             elements, reduced_length, hole, bounds_check, elements_kind, true);
+        Add<HStoreNamedField>(
+            checked_object, HObjectAccess::ForArrayLength(elements_kind),
+            reduced_length);

[mvstanton, 2014/01/29 10:16:09]

I just don't understand why storing the reduced length is done inside of a
NoObservableSideEffectsScope. Can you explain that? Is it that you have the
result object on the environment stack when the simulate is added below, and
that is sufficient?

+
+        Push(result);
+
+        length_checker.End();
       }
-      Add<HStoreNamedField>(
-          checked_object, HObjectAccess::ForArrayLength(elements_kind),
-          reduced_length);
-      if (!ast_context()->IsEffect()) Push(result);
+      result = ast_context()->IsEffect() ? Pop() : Top();
       Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
       if (!ast_context()->IsEffect()) Drop(1);
+
       ast_context()->ReturnValue(result);
       return true;
     }