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

src/hydrogen.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 7614 matching lines @@
       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();
+
+      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);
         result = AddElementAccess(elements, reduced_length, NULL,
                                   bounds_check, elements_kind, false);
         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;
         }
         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;
     }
     case kArrayPush: {
       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;
@@ skipping 3414 matching lines @@
   if (ShouldProduceTraceOutput()) {
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 } }  // namespace v8::internal