From 83519ec87ad1ec8d5a9f116929fa6da79e57121e Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Fri, 28 Jun 2013 13:16:14 +0000 Subject: [PATCH] Hydrogen array constructor cleanup and improvements * Cleanup of LCallNewArray::PrintDataTo() method * Created HCallNewArray::PrintDataTo() * Created many more tests in array-constructor-feedback.js * Removed redundant instructions in GenerateRecordCallTarget * Bugfix in CreateArrayDispatchOneArgument: on a call to new Array(0), we'd like to set the type feedback cell to a packed elements kind, but we shouldn't do it if the cell contains the megamorphic sentinel. * When used from crankshaft, ArrayConstructorStubs can avoid verifying that the function being called is the array function from the current native context, relying instead on the fact that crankshaft issues an HCheckFunction to protect the constructor call. (this new minor key is used in LCodeGen::DoCallNewArray(), and influences code generation in CodeStubGraphBuilderBase::BuildArrayConstructor()). * Optimization: the array constructor specialized for FAST_SMI_ELEMENTS can save some instructions by looking up the correct map on the passed in constructor, rather than indexing into the array of cached maps per element kind. BUG= R=danno@chromium.org Review URL: https://codereview.chromium.org/17091002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15383 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 19 ++-- src/arm/lithium-arm.cc | 3 +- src/arm/lithium-codegen-arm.cc | 17 ++-- src/bootstrapper.cc | 5 + src/code-stubs-hydrogen.cc | 35 ++++--- src/code-stubs.h | 59 +++++++++--- src/hydrogen-instructions.cc | 7 ++ src/hydrogen-instructions.h | 2 + src/hydrogen.cc | 25 +++-- src/hydrogen.h | 3 +- src/ia32/code-stubs-ia32.cc | 21 +++-- src/ia32/lithium-codegen-ia32.cc | 17 ++-- src/ia32/lithium-ia32.cc | 3 +- src/x64/code-stubs-x64.cc | 19 ++-- src/x64/lithium-codegen-x64.cc | 17 ++-- src/x64/lithium-x64.cc | 3 +- test/mjsunit/allocation-site-info.js | 20 +++- test/mjsunit/array-constructor-feedback.js | 146 ++++++++++++++++++++++++++--- 18 files changed, 325 insertions(+), 96 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 9cdaa12..7659f76 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -4651,12 +4651,15 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { // Special handling of the Array() function, which caches not only the // monomorphic Array function but the initial ElementsKind with special // sentinels - Handle terminal_kind_sentinel = - TypeFeedbackCells::MonomorphicArraySentinel(masm->isolate(), - LAST_FAST_ELEMENTS_KIND); __ JumpIfNotSmi(r3, &miss); - __ cmp(r3, Operand(terminal_kind_sentinel)); - __ b(gt, &miss); + if (FLAG_debug_code) { + Handle terminal_kind_sentinel = + TypeFeedbackCells::MonomorphicArraySentinel(masm->isolate(), + LAST_FAST_ELEMENTS_KIND); + __ cmp(r3, Operand(terminal_kind_sentinel)); + __ Assert(le, "Array function sentinel is not an ElementsKind"); + } + // Make sure the function is the Array() function __ LoadArrayFunction(r3); __ cmp(r1, r3); @@ -7182,6 +7185,10 @@ static void CreateArrayDispatchOneArgument(MacroAssembler* masm) { __ cmp(r2, Operand(undefined_sentinel)); __ b(eq, &normal_sequence); + // The type cell may have gone megamorphic, don't overwrite if so + __ ldr(r5, FieldMemOperand(r2, kPointerSize)); + __ JumpIfNotSmi(r5, &normal_sequence); + // Save the resulting elements kind in type info __ SmiTag(r3); __ str(r3, FieldMemOperand(r2, kPointerSize)); @@ -7214,7 +7221,7 @@ static void ArrayConstructorStubAheadOfTimeHelper(Isolate* isolate) { T stub(kind); stub.GetCode(isolate)->set_is_pregenerated(true); if (AllocationSiteInfo::GetMode(kind) != DONT_TRACK_ALLOCATION_SITE) { - T stub1(kind, true); + T stub1(kind, CONTEXT_CHECK_REQUIRED, DISABLE_ALLOCATION_SITES); stub1.GetCode(isolate)->set_is_pregenerated(true); } } diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index 700617a..c28b641 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -326,8 +326,7 @@ void LCallNewArray::PrintDataTo(StringStream* stream) { constructor()->PrintTo(stream); stream->Add(" #%d / ", arity()); ASSERT(hydrogen()->property_cell()->value()->IsSmi()); - ElementsKind kind = static_cast( - Smi::cast(hydrogen()->property_cell()->value())->value()); + ElementsKind kind = hydrogen()->elements_kind(); stream->Add(" (%s) ", ElementsKindToString(kind)); } diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 0ad42db..8525095 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -4129,11 +4129,14 @@ void LCodeGen::DoCallNewArray(LCallNewArray* instr) { __ mov(r0, Operand(instr->arity())); __ mov(r2, Operand(instr->hydrogen()->property_cell())); ElementsKind kind = instr->hydrogen()->elements_kind(); - bool disable_allocation_sites = - (AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE); + AllocationSiteOverrideMode override_mode = + (AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE) + ? DISABLE_ALLOCATION_SITES + : DONT_OVERRIDE; + ContextCheckMode context_mode = CONTEXT_CHECK_NOT_REQUIRED; if (instr->arity() == 0) { - ArrayNoArgumentConstructorStub stub(kind, disable_allocation_sites); + ArrayNoArgumentConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); } else if (instr->arity() == 1) { Label done; @@ -4146,18 +4149,18 @@ void LCodeGen::DoCallNewArray(LCallNewArray* instr) { __ b(eq, &packed_case); ElementsKind holey_kind = GetHoleyElementsKind(kind); - ArraySingleArgumentConstructorStub stub(holey_kind, - disable_allocation_sites); + ArraySingleArgumentConstructorStub stub(holey_kind, context_mode, + override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); __ jmp(&done); __ bind(&packed_case); } - ArraySingleArgumentConstructorStub stub(kind, disable_allocation_sites); + ArraySingleArgumentConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); __ bind(&done); } else { - ArrayNArgumentsConstructorStub stub(kind, disable_allocation_sites); + ArrayNArgumentsConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); } } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 53dca03..c3d1c3a 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -866,6 +866,11 @@ bool Genesis::InitializeGlobal(Handle inner_global, array_function->shared()->set_length(1); Handle initial_map(array_function->initial_map()); + + // This assert protects an optimization in + // HGraphBuilder::JSArrayBuilder::EmitMapCode() + ASSERT(initial_map->elements_kind() == GetInitialFastElementsKind()); + Handle array_descriptors( factory->NewDescriptorArray(0, 1)); DescriptorArray::WhitenessWitness witness(*array_descriptors); diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index c055a7c..06c9039 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -106,7 +106,8 @@ class CodeStubGraphBuilderBase : public HGraphBuilder { }; HValue* BuildArrayConstructor(ElementsKind kind, - bool disable_allocation_sites, + ContextCheckMode context_mode, + AllocationSiteOverrideMode override_mode, ArgumentClass argument_class); HValue* BuildInternalArrayConstructor(ElementsKind kind, ArgumentClass argument_class); @@ -534,15 +535,19 @@ Handle TransitionElementsKindStub::GenerateCode() { } HValue* CodeStubGraphBuilderBase::BuildArrayConstructor( - ElementsKind kind, bool disable_allocation_sites, + ElementsKind kind, + ContextCheckMode context_mode, + AllocationSiteOverrideMode override_mode, ArgumentClass argument_class) { HValue* constructor = GetParameter(ArrayConstructorStubBase::kConstructor); - HValue* property_cell = GetParameter(ArrayConstructorStubBase::kPropertyCell); - HInstruction* array_function = BuildGetArrayFunction(context()); + if (context_mode == CONTEXT_CHECK_REQUIRED) { + HInstruction* array_function = BuildGetArrayFunction(context()); + ArrayContextChecker checker(this, constructor, array_function); + } - ArrayContextChecker(this, constructor, array_function); - JSArrayBuilder array_builder(this, kind, property_cell, - disable_allocation_sites); + HValue* property_cell = GetParameter(ArrayConstructorStubBase::kPropertyCell); + JSArrayBuilder array_builder(this, kind, property_cell, constructor, + override_mode); HValue* result = NULL; switch (argument_class) { case NONE: @@ -555,6 +560,7 @@ HValue* CodeStubGraphBuilderBase::BuildArrayConstructor( result = BuildArrayNArgumentsConstructor(&array_builder, kind); break; } + return result; } @@ -652,8 +658,9 @@ HValue* CodeStubGraphBuilderBase::BuildArrayNArgumentsConstructor( template <> HValue* CodeStubGraphBuilder::BuildCodeStub() { ElementsKind kind = casted_stub()->elements_kind(); - bool disable_allocation_sites = casted_stub()->disable_allocation_sites(); - return BuildArrayConstructor(kind, disable_allocation_sites, NONE); + ContextCheckMode context_mode = casted_stub()->context_mode(); + AllocationSiteOverrideMode override_mode = casted_stub()->override_mode(); + return BuildArrayConstructor(kind, context_mode, override_mode, NONE); } @@ -666,8 +673,9 @@ template <> HValue* CodeStubGraphBuilder:: BuildCodeStub() { ElementsKind kind = casted_stub()->elements_kind(); - bool disable_allocation_sites = casted_stub()->disable_allocation_sites(); - return BuildArrayConstructor(kind, disable_allocation_sites, SINGLE); + ContextCheckMode context_mode = casted_stub()->context_mode(); + AllocationSiteOverrideMode override_mode = casted_stub()->override_mode(); + return BuildArrayConstructor(kind, context_mode, override_mode, SINGLE); } @@ -679,8 +687,9 @@ Handle ArraySingleArgumentConstructorStub::GenerateCode() { template <> HValue* CodeStubGraphBuilder::BuildCodeStub() { ElementsKind kind = casted_stub()->elements_kind(); - bool disable_allocation_sites = casted_stub()->disable_allocation_sites(); - return BuildArrayConstructor(kind, disable_allocation_sites, MULTIPLE); + ContextCheckMode context_mode = casted_stub()->context_mode(); + AllocationSiteOverrideMode override_mode = casted_stub()->override_mode(); + return BuildArrayConstructor(kind, context_mode, override_mode, MULTIPLE); } diff --git a/src/code-stubs.h b/src/code-stubs.h index 6b6ba79..1340590 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1733,27 +1733,51 @@ class TransitionElementsKindStub : public HydrogenCodeStub { }; +enum ContextCheckMode { + CONTEXT_CHECK_REQUIRED, + CONTEXT_CHECK_NOT_REQUIRED, + LAST_CONTEXT_CHECK_MODE = CONTEXT_CHECK_NOT_REQUIRED +}; + + +enum AllocationSiteOverrideMode { + DONT_OVERRIDE, + DISABLE_ALLOCATION_SITES, + LAST_ALLOCATION_SITE_OVERRIDE_MODE = DISABLE_ALLOCATION_SITES +}; + + class ArrayConstructorStubBase : public HydrogenCodeStub { public: - ArrayConstructorStubBase(ElementsKind kind, bool disable_allocation_sites) { + ArrayConstructorStubBase(ElementsKind kind, ContextCheckMode context_mode, + AllocationSiteOverrideMode override_mode) { // It only makes sense to override local allocation site behavior // if there is a difference between the global allocation site policy // for an ElementsKind and the desired usage of the stub. - ASSERT(!disable_allocation_sites || + ASSERT(override_mode != DISABLE_ALLOCATION_SITES || AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE); bit_field_ = ElementsKindBits::encode(kind) | - DisableAllocationSitesBits::encode(disable_allocation_sites); + AllocationSiteOverrideModeBits::encode(override_mode) | + ContextCheckModeBits::encode(context_mode); } ElementsKind elements_kind() const { return ElementsKindBits::decode(bit_field_); } - bool disable_allocation_sites() const { - return DisableAllocationSitesBits::decode(bit_field_); + AllocationSiteOverrideMode override_mode() const { + return AllocationSiteOverrideModeBits::decode(bit_field_); + } + + ContextCheckMode context_mode() const { + return ContextCheckModeBits::decode(bit_field_); + } + + virtual bool IsPregenerated() { + // We only pre-generate stubs that verify correct context + return context_mode() == CONTEXT_CHECK_REQUIRED; } - virtual bool IsPregenerated() { return true; } static void GenerateStubsAheadOfTime(Isolate* isolate); static void InstallDescriptors(Isolate* isolate); @@ -1764,8 +1788,14 @@ class ArrayConstructorStubBase : public HydrogenCodeStub { private: int NotMissMinorKey() { return bit_field_; } + // Ensure data fits within available bits. + STATIC_ASSERT(LAST_ALLOCATION_SITE_OVERRIDE_MODE == 1); + STATIC_ASSERT(LAST_CONTEXT_CHECK_MODE == 1); + class ElementsKindBits: public BitField {}; - class DisableAllocationSitesBits: public BitField {}; + class AllocationSiteOverrideModeBits: public + BitField {}; // NOLINT + class ContextCheckModeBits: public BitField {}; uint32_t bit_field_; DISALLOW_COPY_AND_ASSIGN(ArrayConstructorStubBase); @@ -1776,8 +1806,9 @@ class ArrayNoArgumentConstructorStub : public ArrayConstructorStubBase { public: ArrayNoArgumentConstructorStub( ElementsKind kind, - bool disable_allocation_sites = false) - : ArrayConstructorStubBase(kind, disable_allocation_sites) { + ContextCheckMode context_mode = CONTEXT_CHECK_REQUIRED, + AllocationSiteOverrideMode override_mode = DONT_OVERRIDE) + : ArrayConstructorStubBase(kind, context_mode, override_mode) { } virtual Handle GenerateCode(); @@ -1797,8 +1828,9 @@ class ArraySingleArgumentConstructorStub : public ArrayConstructorStubBase { public: ArraySingleArgumentConstructorStub( ElementsKind kind, - bool disable_allocation_sites = false) - : ArrayConstructorStubBase(kind, disable_allocation_sites) { + ContextCheckMode context_mode = CONTEXT_CHECK_REQUIRED, + AllocationSiteOverrideMode override_mode = DONT_OVERRIDE) + : ArrayConstructorStubBase(kind, context_mode, override_mode) { } virtual Handle GenerateCode(); @@ -1818,8 +1850,9 @@ class ArrayNArgumentsConstructorStub : public ArrayConstructorStubBase { public: ArrayNArgumentsConstructorStub( ElementsKind kind, - bool disable_allocation_sites = false) - : ArrayConstructorStubBase(kind, disable_allocation_sites) { + ContextCheckMode context_mode = CONTEXT_CHECK_REQUIRED, + AllocationSiteOverrideMode override_mode = DONT_OVERRIDE) + : ArrayConstructorStubBase(kind, context_mode, override_mode) { } virtual Handle GenerateCode(); diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 6213583..98fa93f 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -1225,6 +1225,13 @@ void HCallKnownGlobal::PrintDataTo(StringStream* stream) { } +void HCallNewArray::PrintDataTo(StringStream* stream) { + stream->Add(ElementsKindToString(elements_kind())); + stream->Add(" "); + HBinaryCall::PrintDataTo(stream); +} + + void HCallRuntime::PrintDataTo(StringStream* stream) { stream->Add("%o ", *name()); stream->Add("#%d", argument_count()); diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 178867b..bb74687 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -2493,6 +2493,8 @@ class HCallNewArray: public HCallNew { elements_kind_(elements_kind), type_cell_(type_cell) {} + virtual void PrintDataTo(StringStream* stream); + Handle property_cell() const { return type_cell_; } diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 96d647e..d3b9aa8 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1714,14 +1714,15 @@ HInstruction* HGraphBuilder::BuildGetArrayFunction(HValue* context) { HGraphBuilder::JSArrayBuilder::JSArrayBuilder(HGraphBuilder* builder, - ElementsKind kind, - HValue* allocation_site_payload, - bool disable_allocation_sites) : + ElementsKind kind, + HValue* allocation_site_payload, + HValue* constructor_function, + AllocationSiteOverrideMode override_mode) : builder_(builder), kind_(kind), allocation_site_payload_(allocation_site_payload), - constructor_function_(NULL) { - mode_ = disable_allocation_sites + constructor_function_(constructor_function) { + mode_ = override_mode == DISABLE_ALLOCATION_SITES ? DONT_TRACK_ALLOCATION_SITE : AllocationSiteInfo::GetMode(kind); } @@ -1739,8 +1740,18 @@ HGraphBuilder::JSArrayBuilder::JSArrayBuilder(HGraphBuilder* builder, HValue* HGraphBuilder::JSArrayBuilder::EmitMapCode(HValue* context) { - HInstruction* native_context = builder()->BuildGetNativeContext(context); + if (kind_ == GetInitialFastElementsKind()) { + // No need for a context lookup if the kind_ matches the initial + // map, because we can just load the map in that case. + HObjectAccess access = HObjectAccess::ForPrototypeOrInitialMap(); + HInstruction* load = + builder()->BuildLoadNamedField(constructor_function_, + access, + Representation::Tagged()); + return builder()->AddInstruction(load); + } + HInstruction* native_context = builder()->BuildGetNativeContext(context); HInstruction* index = builder()->Add( static_cast(Context::JS_ARRAY_MAPS_INDEX)); @@ -1840,7 +1851,7 @@ HValue* HGraphBuilder::JSArrayBuilder::AllocateArray(HValue* size_in_bytes, // Fill in the fields: map, properties, length HValue* map; - if (constructor_function_ != NULL) { + if (allocation_site_payload_ == NULL) { map = EmitInternalMapCode(); } else { map = EmitMapCode(context); diff --git a/src/hydrogen.h b/src/hydrogen.h index 1086b24..7442b5f 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -1305,7 +1305,8 @@ class HGraphBuilder { JSArrayBuilder(HGraphBuilder* builder, ElementsKind kind, HValue* allocation_site_payload, - bool disable_allocation_sites); + HValue* constructor_function, + AllocationSiteOverrideMode override_mode); JSArrayBuilder(HGraphBuilder* builder, ElementsKind kind, diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 83b753b..7bb0f5d 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -4700,12 +4700,15 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { // Special handling of the Array() function, which caches not only the // monomorphic Array function but the initial ElementsKind with special // sentinels - Handle terminal_kind_sentinel = - TypeFeedbackCells::MonomorphicArraySentinel(isolate, - LAST_FAST_ELEMENTS_KIND); __ JumpIfNotSmi(ecx, &miss); - __ cmp(ecx, Immediate(terminal_kind_sentinel)); - __ j(above, &miss); + if (FLAG_debug_code) { + Handle terminal_kind_sentinel = + TypeFeedbackCells::MonomorphicArraySentinel(masm->isolate(), + LAST_FAST_ELEMENTS_KIND); + __ cmp(ecx, Immediate(terminal_kind_sentinel)); + __ Assert(less_equal, "Array function sentinel is not an ElementsKind"); + } + // Load the global or builtins object from the current context __ LoadGlobalContext(ecx); // Make sure the function is the Array() function @@ -7777,6 +7780,10 @@ static void CreateArrayDispatchOneArgument(MacroAssembler* masm) { __ cmp(ebx, Immediate(undefined_sentinel)); __ j(equal, &normal_sequence); + // The type cell may have gone megamorphic, don't overwrite if so + __ mov(ecx, FieldOperand(ebx, kPointerSize)); + __ JumpIfNotSmi(ecx, &normal_sequence); + // Save the resulting elements kind in type info __ SmiTag(edx); __ mov(FieldOperand(ebx, kPointerSize), edx); @@ -7806,10 +7813,10 @@ static void ArrayConstructorStubAheadOfTimeHelper(Isolate* isolate) { TERMINAL_FAST_ELEMENTS_KIND); for (int i = 0; i <= to_index; ++i) { ElementsKind kind = GetFastElementsKindFromSequenceIndex(i); - T stub(kind, false); + T stub(kind); stub.GetCode(isolate)->set_is_pregenerated(true); if (AllocationSiteInfo::GetMode(kind) != DONT_TRACK_ALLOCATION_SITE) { - T stub1(kind, true); + T stub1(kind, CONTEXT_CHECK_REQUIRED, DISABLE_ALLOCATION_SITES); stub1.GetCode(isolate)->set_is_pregenerated(true); } } diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 18c95cc..694ae13 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -4175,11 +4175,14 @@ void LCodeGen::DoCallNewArray(LCallNewArray* instr) { __ Set(eax, Immediate(instr->arity())); __ mov(ebx, instr->hydrogen()->property_cell()); ElementsKind kind = instr->hydrogen()->elements_kind(); - bool disable_allocation_sites = - (AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE); + AllocationSiteOverrideMode override_mode = + (AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE) + ? DISABLE_ALLOCATION_SITES + : DONT_OVERRIDE; + ContextCheckMode context_mode = CONTEXT_CHECK_NOT_REQUIRED; if (instr->arity() == 0) { - ArrayNoArgumentConstructorStub stub(kind, disable_allocation_sites); + ArrayNoArgumentConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); } else if (instr->arity() == 1) { Label done; @@ -4192,18 +4195,18 @@ void LCodeGen::DoCallNewArray(LCallNewArray* instr) { __ j(zero, &packed_case); ElementsKind holey_kind = GetHoleyElementsKind(kind); - ArraySingleArgumentConstructorStub stub(holey_kind, - disable_allocation_sites); + ArraySingleArgumentConstructorStub stub(holey_kind, context_mode, + override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); __ jmp(&done); __ bind(&packed_case); } - ArraySingleArgumentConstructorStub stub(kind, disable_allocation_sites); + ArraySingleArgumentConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); __ bind(&done); } else { - ArrayNArgumentsConstructorStub stub(kind, disable_allocation_sites); + ArrayNArgumentsConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); } } diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index e8ff766..04b8934 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -351,8 +351,7 @@ void LCallNewArray::PrintDataTo(StringStream* stream) { constructor()->PrintTo(stream); stream->Add(" #%d / ", arity()); ASSERT(hydrogen()->property_cell()->value()->IsSmi()); - ElementsKind kind = static_cast( - Smi::cast(hydrogen()->property_cell()->value())->value()); + ElementsKind kind = hydrogen()->elements_kind(); stream->Add(" (%s) ", ElementsKindToString(kind)); } diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 897f008..d8982d6 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -3738,12 +3738,15 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { // Special handling of the Array() function, which caches not only the // monomorphic Array function but the initial ElementsKind with special // sentinels - Handle terminal_kind_sentinel = - TypeFeedbackCells::MonomorphicArraySentinel(isolate, - LAST_FAST_ELEMENTS_KIND); __ JumpIfNotSmi(rcx, &miss); - __ Cmp(rcx, terminal_kind_sentinel); - __ j(above, &miss); + if (FLAG_debug_code) { + Handle terminal_kind_sentinel = + TypeFeedbackCells::MonomorphicArraySentinel(masm->isolate(), + LAST_FAST_ELEMENTS_KIND); + __ Cmp(rcx, terminal_kind_sentinel); + __ Assert(less_equal, "Array function sentinel is not an ElementsKind"); + } + // Make sure the function is the Array() function __ LoadArrayFunction(rcx); __ cmpq(rdi, rcx); @@ -6783,6 +6786,10 @@ static void CreateArrayDispatchOneArgument(MacroAssembler* masm) { __ Cmp(rbx, undefined_sentinel); __ j(equal, &normal_sequence); + // The type cell may have gone megamorphic, don't overwrite if so + __ movq(rcx, FieldOperand(rbx, kPointerSize)); + __ JumpIfNotSmi(rcx, &normal_sequence); + // Save the resulting elements kind in type info __ Integer32ToSmi(rdx, rdx); __ movq(FieldOperand(rbx, kPointerSize), rdx); @@ -6815,7 +6822,7 @@ static void ArrayConstructorStubAheadOfTimeHelper(Isolate* isolate) { T stub(kind); stub.GetCode(isolate)->set_is_pregenerated(true); if (AllocationSiteInfo::GetMode(kind) != DONT_TRACK_ALLOCATION_SITE) { - T stub1(kind, true); + T stub1(kind, CONTEXT_CHECK_REQUIRED, DISABLE_ALLOCATION_SITES); stub1.GetCode(isolate)->set_is_pregenerated(true); } } diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 0a2c3ce..f3045d5 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -3877,11 +3877,14 @@ void LCodeGen::DoCallNewArray(LCallNewArray* instr) { __ Set(rax, instr->arity()); __ Move(rbx, instr->hydrogen()->property_cell()); ElementsKind kind = instr->hydrogen()->elements_kind(); - bool disable_allocation_sites = - (AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE); + AllocationSiteOverrideMode override_mode = + (AllocationSiteInfo::GetMode(kind) == TRACK_ALLOCATION_SITE) + ? DISABLE_ALLOCATION_SITES + : DONT_OVERRIDE; + ContextCheckMode context_mode = CONTEXT_CHECK_NOT_REQUIRED; if (instr->arity() == 0) { - ArrayNoArgumentConstructorStub stub(kind, disable_allocation_sites); + ArrayNoArgumentConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); } else if (instr->arity() == 1) { Label done; @@ -3894,18 +3897,18 @@ void LCodeGen::DoCallNewArray(LCallNewArray* instr) { __ j(zero, &packed_case); ElementsKind holey_kind = GetHoleyElementsKind(kind); - ArraySingleArgumentConstructorStub stub(holey_kind, - disable_allocation_sites); + ArraySingleArgumentConstructorStub stub(holey_kind, context_mode, + override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); __ jmp(&done); __ bind(&packed_case); } - ArraySingleArgumentConstructorStub stub(kind, disable_allocation_sites); + ArraySingleArgumentConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); __ bind(&done); } else { - ArrayNArgumentsConstructorStub stub(kind, disable_allocation_sites); + ArrayNArgumentsConstructorStub stub(kind, context_mode, override_mode); CallCode(stub.GetCode(isolate()), RelocInfo::CONSTRUCT_CALL, instr); } } diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index a759f16..7a475a7 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -329,8 +329,7 @@ void LCallNewArray::PrintDataTo(StringStream* stream) { constructor()->PrintTo(stream); stream->Add(" #%d / ", arity()); ASSERT(hydrogen()->property_cell()->value()->IsSmi()); - ElementsKind kind = static_cast( - Smi::cast(hydrogen()->property_cell()->value())->value()); + ElementsKind kind = hydrogen()->elements_kind(); stream->Add(" (%s) ", ElementsKindToString(kind)); } diff --git a/test/mjsunit/allocation-site-info.js b/test/mjsunit/allocation-site-info.js index 542f874..86c28aa 100644 --- a/test/mjsunit/allocation-site-info.js +++ b/test/mjsunit/allocation-site-info.js @@ -89,7 +89,6 @@ function assertNotHoley(obj, name_opt) { } if (support_smi_only_arrays) { - obj = []; assertNotHoley(obj); assertKind(elements_kind.fast_smi_only, obj); @@ -305,9 +304,26 @@ if (support_smi_only_arrays) { var realmBArray = Realm.eval(realmB, "Array"); instanceof_check(Array); instanceof_check(realmBArray); - %OptimizeFunctionOnNextCall(instanceof_check); + %OptimizeFunctionOnNextCall(instanceof_check); + + // No de-opt will occur because HCallNewArray wasn't selected, on account of + // the call site not being monomorphic to Array. + instanceof_check(Array); + assertTrue(2 != %GetOptimizationStatus(instanceof_check)); + instanceof_check(realmBArray); + assertTrue(2 != %GetOptimizationStatus(instanceof_check)); + + // Try to optimize again, but first clear all type feedback, and allow it + // to be monomorphic on first call. Only after crankshafting do we introduce + // realmBArray. This should deopt the method. + %DeoptimizeFunction(instanceof_check); + %ClearFunctionTypeFeedback(instanceof_check); + instanceof_check(Array); + instanceof_check(Array); + %OptimizeFunctionOnNextCall(instanceof_check); instanceof_check(Array); assertTrue(2 != %GetOptimizationStatus(instanceof_check)); + instanceof_check(realmBArray); assertTrue(1 != %GetOptimizationStatus(instanceof_check)); } diff --git a/test/mjsunit/array-constructor-feedback.js b/test/mjsunit/array-constructor-feedback.js index 302239b..e29e769 100644 --- a/test/mjsunit/array-constructor-feedback.js +++ b/test/mjsunit/array-constructor-feedback.js @@ -81,19 +81,137 @@ function assertKind(expected, obj, name_opt) { } if (support_smi_only_arrays) { - function bar0(t) { - return new t(); - } - a = bar0(Array); - a[0] = 3.5; - b = bar0(Array); - assertKind(elements_kind.fast_double, b); - %OptimizeFunctionOnNextCall(bar0); - b = bar0(Array); - assertKind(elements_kind.fast_double, b); - assertTrue(2 != %GetOptimizationStatus(bar0)); - // bar0 should deopt - b = bar0(Object); - assertTrue(1 != %GetOptimizationStatus(bar0)); + // Test: If a call site goes megamorphic, it loses the ability to + // use allocation site feedback. + (function() { + function bar(t, len) { + return new t(len); + } + + a = bar(Array, 10); + a[0] = 3.5; + b = bar(Array, 1); + assertKind(elements_kind.fast_double, b); + c = bar(Object, 3); + b = bar(Array, 10); + assertKind(elements_kind.fast_smi_only, b); + b[0] = 3.5; + c = bar(Array, 10); + assertKind(elements_kind.fast_smi_only, c); + })(); + + + // Test: ensure that crankshafted array constructor sites are deopted + // if another function is used. + (function() { + function bar0(t) { + return new t(); + } + a = bar0(Array); + a[0] = 3.5; + b = bar0(Array); + assertKind(elements_kind.fast_double, b); + %OptimizeFunctionOnNextCall(bar0); + b = bar0(Array); + assertKind(elements_kind.fast_double, b); + assertTrue(2 != %GetOptimizationStatus(bar0)); + // bar0 should deopt + b = bar0(Object); + assertTrue(1 != %GetOptimizationStatus(bar0)); + // When it's re-optimized, we should call through the full stub + bar0(Array); + %OptimizeFunctionOnNextCall(bar0); + b = bar0(Array); + // We also lost our ability to record kind feedback, as the site + // is megamorphic now. + assertKind(elements_kind.fast_smi_only, b); + assertTrue(2 != %GetOptimizationStatus(bar0)); + b[0] = 3.5; + c = bar0(Array); + assertKind(elements_kind.fast_smi_only, c); + })(); + + + // Test: Ensure that bailouts from the stub don't deopt a crankshafted + // method with a call to that stub. + (function() { + function bar(len) { + return new Array(len); + } + a = bar(10); + a[0] = "a string"; + a = bar(10); + assertKind(elements_kind.fast, a); + %OptimizeFunctionOnNextCall(bar); + a = bar(10); + assertKind(elements_kind.fast, a); + assertTrue(2 != %GetOptimizationStatus(bar)); + // The stub bails out, but the method call should be fine. + a = bar(100000); + assertTrue(2 != %GetOptimizationStatus(bar)); + assertKind(elements_kind.dictionary, a); + + // If the argument isn't a smi, it bails out as well + a = bar("oops"); + assertTrue(2 != %GetOptimizationStatus(bar)); + assertKind(elements_kind.fast, a); + + function barn(one, two, three) { + return new Array(one, two, three); + } + + barn(1, 2, 3); + barn(1, 2, 3); + %OptimizeFunctionOnNextCall(barn); + barn(1, 2, 3); + assertTrue(2 != %GetOptimizationStatus(barn)); + a = barn(1, "oops", 3); + // The stub should bail out but the method should remain optimized. + assertKind(elements_kind.fast, a); + assertTrue(2 != %GetOptimizationStatus(barn)); + })(); + + + // Test: When a method with array constructor is crankshafted, the type + // feedback for elements kind is baked in. Verify that transitions don't + // change it anymore + (function() { + function bar() { + return new Array(); + } + a = bar(); + bar(); + %OptimizeFunctionOnNextCall(bar); + b = bar(); + // This only makes sense to test if we allow crankshafting + if (4 != %GetOptimizationStatus(bar)) { + assertTrue(2 != %GetOptimizationStatus(bar)); + %DebugPrint(3); + b[0] = 3.5; + c = bar(); + assertKind(elements_kind.fast_smi_only, c); + assertTrue(2 != %GetOptimizationStatus(bar)); + } + })(); + + + // Test: create arrays in two contexts, verifying that the correct + // map for Array in that context will be used. + (function() { + function bar() { return new Array(); } + bar(); + bar(); + %OptimizeFunctionOnNextCall(bar); + a = bar(); + assertTrue(a instanceof Array); + + var contextB = Realm.create(); + Realm.eval(contextB, "function bar2() { return new Array(); };"); + Realm.eval(contextB, "bar2(); bar2();"); + Realm.eval(contextB, "%OptimizeFunctionOnNextCall(bar2);"); + Realm.eval(contextB, "bar2();"); + assertFalse(Realm.eval(contextB, "bar2();") instanceof Array); + assertTrue(Realm.eval(contextB, "bar2() instanceof Array")); + })(); } -- 2.7.4