Becuase of cross-context calls, hydrogen-based Array constructor needs to ensure

the array constructor pointer passed in matches that of the current context.

BUG=
R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/14846017

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14581 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/code-stubs-arm.cc

@@ -138,9 +138,10 @@ static void InitializeArrayConstructorDescriptor(
     int constant_stack_parameter_count) {
   // register state
   // r0 -- number of arguments
+  // r1 -- function
   // r2 -- type info cell with elements kind
-  static Register registers[] = { r2 };
-  descriptor->register_param_count_ = 1;
+  static Register registers[] = { r1, r2 };
+  descriptor->register_param_count_ = 2;
   if (constant_stack_parameter_count != 0) {
     // stack param count needs (constructor pointer, and single argument)
     descriptor->stack_parameter_count_ = &r0;
@@ -7361,14 +7362,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
     // Get the elements kind and case on that.
     __ cmp(r2, Operand(undefined_sentinel));
     __ b(eq, &no_info);
-    __ ldr(r3, FieldMemOperand(r2, kPointerSize));
-
-    // There is no info if the call site went megamorphic either
-    // TODO(mvstanton): Really? I thought if it was the array function that
-    // the cell wouldn't get stamped as megamorphic.
-    __ cmp(r3,
-           Operand(TypeFeedbackCells::MegamorphicSentinel(masm->isolate())));
-    __ b(eq, &no_info);
+    __ ldr(r3, FieldMemOperand(r2, JSGlobalPropertyCell::kValueOffset));
+    __ JumpIfNotSmi(r3, &no_info);
     __ SmiUntag(r3);
     __ jmp(&switch_ready);
     __ bind(&no_info);

src/builtins.cc

@@ -194,55 +194,58 @@ BUILTIN(EmptyFunction) {
 }
 
 
-#define CONVERT_ARG_STUB_CALLER_ARGS(name)                           \
-  Arguments* name = reinterpret_cast<Arguments*>(args[0]);
-
-
 RUNTIME_FUNCTION(MaybeObject*, ArrayConstructor_StubFailure) {
-  CONVERT_ARG_STUB_CALLER_ARGS(caller_args);
-  ASSERT(args.length() == 2);
-  Handle<Object> type_info = args.at<Object>(1);
+  // If we get 2 arguments then they are the stub parameters (constructor, type
+  // info).  If we get 3, then the first one is a pointer to the arguments
+  // passed by the caller.
+  Arguments empty_args(0, NULL);
+  bool no_caller_args = args.length() == 2;
+  ASSERT(no_caller_args || args.length() == 3);
+  int parameters_start = no_caller_args ? 0 : 1;
+  Arguments* caller_args = no_caller_args
+      ? &empty_args
+      : reinterpret_cast<Arguments*>(args[0]);
+  Handle<JSFunction> constructor = args.at<JSFunction>(parameters_start);
+  Handle<Object> type_info = args.at<Object>(parameters_start + 1);
 
-  JSArray* array = NULL;
   bool holey = false;
   if (caller_args->length() == 1 && (*caller_args)[0]->IsSmi()) {
     int value = Smi::cast((*caller_args)[0])->value();
     holey = (value > 0 && value < JSObject::kInitialMaxFastElementArray);
   }
 
+  JSArray* array;
   MaybeObject* maybe_array;
-  if (*type_info != isolate->heap()->undefined_value()) {
+  if (*type_info != isolate->heap()->undefined_value() &&
+      JSGlobalPropertyCell::cast(*type_info)->value()->IsSmi()) {
     JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(*type_info);
-    if (cell->value()->IsSmi()) {
-      Smi* smi = Smi::cast(cell->value());
-      ElementsKind to_kind = static_cast<ElementsKind>(smi->value());
-      if (holey && !IsFastHoleyElementsKind(to_kind)) {
-        to_kind = GetHoleyElementsKind(to_kind);
-        // Update the allocation site info to reflect the advice alteration.
-        cell->set_value(Smi::FromInt(to_kind));
-      }
+    Smi* smi = Smi::cast(cell->value());
+    ElementsKind to_kind = static_cast<ElementsKind>(smi->value());
+    if (holey && !IsFastHoleyElementsKind(to_kind)) {
+      to_kind = GetHoleyElementsKind(to_kind);
+      // Update the allocation site info to reflect the advice alteration.
+      cell->set_value(Smi::FromInt(to_kind));
+    }
 
-      AllocationSiteMode mode = AllocationSiteInfo::GetMode(to_kind);
-      if (mode == TRACK_ALLOCATION_SITE) {
-        maybe_array = isolate->heap()->AllocateEmptyJSArrayWithAllocationSite(
-            to_kind, type_info);
-      } else {
-        maybe_array = isolate->heap()->AllocateEmptyJSArray(to_kind);
-      }
-      if (!maybe_array->To(&array)) return maybe_array;
+    maybe_array = isolate->heap()->AllocateJSObjectWithAllocationSite(
+        *constructor, type_info);
+    if (!maybe_array->To(&array)) return maybe_array;
+  } else {
+    ElementsKind kind = constructor->initial_map()->elements_kind();
+    ASSERT(kind == GetInitialFastElementsKind());
+    maybe_array = isolate->heap()->AllocateJSObject(*constructor);
+    if (!maybe_array->To(&array)) return maybe_array;
+    // We might need to transition to holey
+    if (holey) {
+      kind = GetHoleyElementsKind(kind);
+      maybe_array = array->TransitionElementsKind(kind);
+      if (maybe_array->IsFailure()) return maybe_array;
     }
   }
 
-  ElementsKind kind = GetInitialFastElementsKind();
-  if (holey) {
-    kind = GetHoleyElementsKind(kind);
-  }
-
-  if (array == NULL) {
-    maybe_array = isolate->heap()->AllocateEmptyJSArray(kind);
-    if (!maybe_array->To(&array)) return maybe_array;
-  }
-
+  maybe_array = isolate->heap()->AllocateJSArrayStorage(array, 0, 0,
+      DONT_INITIALIZE_ARRAY_ELEMENTS);
+  if (maybe_array->IsFailure()) return maybe_array;
   maybe_array = ArrayConstructInitializeElements(array, caller_args);
   if (maybe_array->IsFailure()) return maybe_array;
   return array;

src/code-stubs-hydrogen.cc

@@ -82,6 +82,24 @@ class CodeStubGraphBuilderBase : public HGraphBuilder {
   HContext* context() { return context_; }
   Isolate* isolate() { return info_.isolate(); }
 
+  class ArrayContextChecker {
+   public:
+    ArrayContextChecker(HGraphBuilder* builder, HValue* constructor,
+                        HValue* array_function)
+        : checker_(builder) {
+      checker_.If<HCompareObjectEqAndBranch, HValue*>(constructor,
+                                                      array_function);
+      checker_.Then();
+    }
+
+    ~ArrayContextChecker() {
+      checker_.ElseDeopt();
+      checker_.End();
+    }
+   private:
+    IfBuilder checker_;
+  };
+
  private:
   SmartArrayPointer<HParameter*> parameters_;
   HValue* arguments_length_;
@@ -524,6 +542,10 @@ HValue* CodeStubGraphBuilder<ArrayNoArgumentConstructorStub>::BuildCodeStub() {
   //  -- Parameter 1 : type info cell
   //  -- Parameter 0 : constructor
   // -----------------------------------
+  HInstruction* array_function = BuildGetArrayFunction(context());
+  ArrayContextChecker(this,
+                      GetParameter(ArrayConstructorStubBase::kConstructor),
+                      array_function);
   // Get the right map
   // Should be a constant
   JSArrayBuilder array_builder(
@@ -543,6 +565,10 @@ Handle<Code> ArrayNoArgumentConstructorStub::GenerateCode() {
 template <>
 HValue* CodeStubGraphBuilder<ArraySingleArgumentConstructorStub>::
     BuildCodeStub() {
+  HInstruction* array_function = BuildGetArrayFunction(context());
+  ArrayContextChecker(this,
+                      GetParameter(ArrayConstructorStubBase::kConstructor),
+                      array_function);
   // Smi check and range check on the input arg.
   HValue* constant_one = graph()->GetConstant1();
   HValue* constant_zero = graph()->GetConstant0();
@@ -596,6 +622,10 @@ Handle<Code> ArraySingleArgumentConstructorStub::GenerateCode() {
 
 template <>
 HValue* CodeStubGraphBuilder<ArrayNArgumentsConstructorStub>::BuildCodeStub() {
+  HInstruction* array_function = BuildGetArrayFunction(context());
+  ArrayContextChecker(this,
+                      GetParameter(ArrayConstructorStubBase::kConstructor),
+                      array_function);
   ElementsKind kind = casted_stub()->elements_kind();
   HValue* length = GetArgumentsLength();
 

src/code-stubs.h

@@ -1652,7 +1652,8 @@ class ArrayConstructorStubBase : public HydrogenCodeStub {
   static void InstallDescriptors(Isolate* isolate);
 
   // Parameters accessed via CodeStubGraphBuilder::GetParameter()
-  static const int kPropertyCell = 0;
+  static const int kConstructor = 0;
+  static const int kPropertyCell = 1;
 
  private:
   int NotMissMinorKey() { return bit_field_; }

src/flag-definitions.h

@@ -255,7 +255,7 @@ DEFINE_bool(unreachable_code_elimination, false,
             "eliminate unreachable code (hidden behind soft deopts)")
 DEFINE_bool(track_allocation_sites, true,
             "Use allocation site info to reduce transitions")
-DEFINE_bool(optimize_constructed_arrays, false,
+DEFINE_bool(optimize_constructed_arrays, true,
             "Use allocation site info on constructed arrays")
 DEFINE_bool(trace_osr, false, "trace on-stack replacement")
 DEFINE_int(stress_runs, 0, "number of stress runs")

src/heap.cc

@@ -4340,8 +4340,7 @@ MaybeObject* Heap::AllocateJSObjectWithAllocationSite(JSFunction* constructor,
   ElementsKind to_kind = static_cast<ElementsKind>(smi->value());
   AllocationSiteMode mode = TRACK_ALLOCATION_SITE;
   if (to_kind != initial_map->elements_kind()) {
-    MaybeObject* maybe_new_map = constructor->GetElementsTransitionMap(
-        isolate(), to_kind);
+    MaybeObject* maybe_new_map = initial_map->AsElementsKind(to_kind);
     if (!maybe_new_map->To(&initial_map)) return maybe_new_map;
     // Possibly alter the mode, since we found an updated elements kind
     // in the type info cell.

src/hydrogen.cc

@@ -1863,6 +1863,26 @@ HValue* HGraphBuilder::BuildCreateAllocationSiteInfo(HValue* previous_object,
 }
 
 
+HInstruction* HGraphBuilder::BuildGetNativeContext(HValue* context) {
+  HInstruction* global_object = AddInstruction(new(zone())
+                                               HGlobalObject(context));
+  HInstruction* native_context = AddInstruction(new(zone())
+      HLoadNamedField(global_object, true, Representation::Tagged(),
+                      GlobalObject::kNativeContextOffset));
+  return native_context;
+}
+
+
+HInstruction* HGraphBuilder::BuildGetArrayFunction(HValue* context) {
+  HInstruction* native_context = BuildGetNativeContext(context);
+  int offset = Context::kHeaderSize +
+      kPointerSize * Context::ARRAY_FUNCTION_INDEX;
+  HInstruction* array_function = AddInstruction(new(zone())
+      HLoadNamedField(native_context, true, Representation::Tagged(), offset));
+  return array_function;
+}
+
+
 HGraphBuilder::JSArrayBuilder::JSArrayBuilder(HGraphBuilder* builder,
                                               ElementsKind kind,
                                               HValue* allocation_site_payload,
@@ -1879,12 +1899,7 @@ HGraphBuilder::JSArrayBuilder::JSArrayBuilder(HGraphBuilder* builder,
 
 
 HValue* HGraphBuilder::JSArrayBuilder::EmitMapCode(HValue* context) {
-  // Get the global context, the native context, the map array
-  HInstruction* global_object = AddInstruction(new(zone())
-      HGlobalObject(context));
-  HInstruction* native_context = AddInstruction(new(zone())
-      HLoadNamedField(global_object, true, Representation::Tagged(),
-                      GlobalObject::kNativeContextOffset));
+  HInstruction* native_context = builder()->BuildGetNativeContext(context);
   int offset = Context::kHeaderSize +
       kPointerSize * Context::JS_ARRAY_MAPS_INDEX;
   HInstruction* map_array = AddInstruction(new(zone())
@@ -9661,16 +9676,13 @@ void HOptimizedGraphBuilder::VisitCallNew(CallNew* expr) {
     CHECK_ALIVE(VisitArgumentList(expr->arguments()));
     HCallNew* call;
     if (use_call_new_array) {
-      AddInstruction(new(zone()) HCheckFunction(constructor,
-          Handle<JSFunction>(isolate()->global_context()->array_function())));
-      Handle<Object> feedback = oracle()->GetInfo(expr->CallNewFeedbackId());
-      ASSERT(feedback->IsSmi());
-
       // TODO(mvstanton): It would be better to use the already created global
       // property cell that is shared by full code gen. That way, any transition
       // information that happened after crankshaft won't be lost.  The right
       // way to do that is to begin passing the cell to the type feedback oracle
       // instead of just the value in the cell. Do this in a follow-up checkin.
+      Handle<Object> feedback = oracle()->GetInfo(expr->CallNewFeedbackId());
+      ASSERT(feedback->IsSmi());
       Handle<JSGlobalPropertyCell> cell =
           isolate()->factory()->NewJSGlobalPropertyCell(feedback);
 

src/hydrogen.h

@@ -1324,6 +1324,9 @@ class HGraphBuilder {
                                         int previous_object_size,
                                         HValue* payload);
 
+  HInstruction* BuildGetNativeContext(HValue* context);
+  HInstruction* BuildGetArrayFunction(HValue* context);
+
  private:
   HGraphBuilder();
   CompilationInfo* info_;

src/ia32/code-stubs-ia32.cc

@@ -130,9 +130,10 @@ static void InitializeArrayConstructorDescriptor(
     int constant_stack_parameter_count) {
   // register state
   // eax -- number of arguments
+  // edi -- function
   // ebx -- type info cell with elements kind
-  static Register registers[] = { ebx };
-  descriptor->register_param_count_ = 1;
+  static Register registers[] = { edi, ebx };
+  descriptor->register_param_count_ = 2;
 
   if (constant_stack_parameter_count != 0) {
     // stack param count needs (constructor pointer, and single argument)
@@ -7921,15 +7922,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
     // Get the elements kind and case on that.
     __ cmp(ebx, Immediate(undefined_sentinel));
     __ j(equal, &no_info);
-    __ mov(edx, FieldOperand(ebx, kPointerSize));
-
-    // There is no info if the call site went megamorphic either
-
-    // TODO(mvstanton): Really? I thought if it was the array function that
-    // the cell wouldn't get stamped as megamorphic.
-    __ cmp(edx, Immediate(TypeFeedbackCells::MegamorphicSentinel(
-        masm->isolate())));
-    __ j(equal, &no_info);
+    __ mov(edx, FieldOperand(ebx, JSGlobalPropertyCell::kValueOffset));
+    __ JumpIfNotSmi(edx, &no_info);
     __ SmiUntag(edx);
     __ jmp(&switch_ready);
     __ bind(&no_info);

src/x64/code-stubs-x64.cc

@@ -125,9 +125,10 @@ static void InitializeArrayConstructorDescriptor(
     int constant_stack_parameter_count) {
   // register state
   // rax -- number of arguments
+  // rdi -- function
   // rbx -- type info cell with elements kind
-  static Register registers[] = { rbx };
-  descriptor->register_param_count_ = 1;
+  static Register registers[] = { rdi, rbx };
+  descriptor->register_param_count_ = 2;
   if (constant_stack_parameter_count != 0) {
     // stack param count needs (constructor pointer, and single argument)
     descriptor->stack_parameter_count_ = &rax;
@@ -6948,14 +6949,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
     // Get the elements kind and case on that.
     __ Cmp(rbx, undefined_sentinel);
     __ j(equal, &no_info);
-    __ movq(rdx, FieldOperand(rbx, kPointerSize));
-
-    // There is no info if the call site went megamorphic either
-
-    // TODO(mvstanton): Really? I thought if it was the array function that
-    // the cell wouldn't get stamped as megamorphic.
-    __ Cmp(rdx, TypeFeedbackCells::MegamorphicSentinel(masm->isolate()));
-    __ j(equal, &no_info);
+    __ movq(rdx, FieldOperand(rbx, JSGlobalPropertyCell::kValueOffset));
+    __ JumpIfNotSmi(rdx, &no_info);
     __ SmiToInteger32(rdx, rdx);
     __ jmp(&switch_ready);
     __ bind(&no_info);

test/mjsunit/allocation-site-info.js

@@ -41,7 +41,7 @@
 
 // support_smi_only_arrays = %HasFastSmiElements(new Array(1,2,3,4,5,6,7,8));
 support_smi_only_arrays = true;
-optimize_constructed_arrays = false;
+optimize_constructed_arrays = true;
 
 if (support_smi_only_arrays) {
   print("Tests include smi-only arrays.");
@@ -284,5 +284,25 @@ if (support_smi_only_arrays) {
     assertKind(elements_kind.fast, obj);
     obj = newarraycase_list_smiobj(2);
     assertKind(elements_kind.fast, obj);
+
+    // Verify that cross context calls work
+    var realmA = Realm.current();
+    var realmB = Realm.create();
+    assertEquals(0, realmA);
+    assertEquals(1, realmB);
+
+    function instanceof_check(type) {
+      assertTrue(new type() instanceof type);
+      assertTrue(new type(5) instanceof type);
+      assertTrue(new type(1,2,3) instanceof type);
+    }
+
+    var realmBArray = Realm.eval(realmB, "Array");
+    instanceof_check(Array);
+    instanceof_check(realmBArray);
+    %OptimizeFunctionOnNextCall(instanceof_check);
+    instanceof_check(Array);
+    instanceof_check(realmBArray);
+    assertTrue(2 != %GetOptimizationStatus(instanceof_check));
   }
 }