From 6bb134051506d5a8cdd4643eda42e54269107767 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Mon, 22 Sep 2014 13:23:27 +0000 Subject: [PATCH] Make KeyedLoads from a sloppy arguments array use a handler. Before, a custom stub was installed. R=verwaest@chromium.org Review URL: https://codereview.chromium.org/546683003 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24120 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/builtins.cc | 5 -- src/builtins.h | 1 - src/code-stubs-hydrogen.cc | 121 ++++++++++++++++++++++++++++++++++++++++++++- src/code-stubs.cc | 4 +- src/code-stubs.h | 15 ++++++ src/elements-kind.h | 5 ++ src/ic/arm/ic-arm.cc | 26 ---------- src/ic/arm64/ic-arm64.cc | 29 ----------- src/ic/handler-compiler.cc | 4 +- src/ic/ia32/ic-ia32.cc | 26 ---------- src/ic/ic-compiler.cc | 2 + src/ic/ic.cc | 6 +-- src/ic/ic.h | 4 -- src/ic/mips/ic-mips.cc | 26 ---------- src/ic/x64/ic-x64.cc | 25 ---------- 15 files changed, 147 insertions(+), 152 deletions(-) diff --git a/src/builtins.cc b/src/builtins.cc index 4a393cb..d0c19e5 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -1289,11 +1289,6 @@ static void Generate_KeyedLoadIC_PreMonomorphic(MacroAssembler* masm) { } -static void Generate_KeyedLoadIC_SloppyArguments(MacroAssembler* masm) { - KeyedLoadIC::GenerateSloppyArguments(masm); -} - - static void Generate_StoreIC_Miss(MacroAssembler* masm) { StoreIC::GenerateMiss(masm); } diff --git a/src/builtins.h b/src/builtins.h index f9409da..c1ed91d 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -89,7 +89,6 @@ enum BuiltinExtraArguments { kNoExtraICState) \ V(KeyedLoadIC_Generic, KEYED_LOAD_IC, GENERIC, kNoExtraICState) \ V(KeyedLoadIC_String, KEYED_LOAD_IC, MEGAMORPHIC, kNoExtraICState) \ - V(KeyedLoadIC_SloppyArguments, KEYED_LOAD_IC, MONOMORPHIC, kNoExtraICState) \ \ V(StoreIC_Setter_ForDeopt, STORE_IC, MONOMORPHIC, StoreIC::kStrictModeState) \ \ diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 3a9688a..1c43049 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -71,6 +71,8 @@ class CodeStubGraphBuilderBase : public HGraphBuilder { MULTIPLE }; + HValue* UnmappedCase(HValue* elements, HValue* key); + HValue* BuildArrayConstructor(ElementsKind kind, AllocationSiteOverrideMode override_mode, ArgumentClass argument_class); @@ -600,6 +602,122 @@ HValue* CodeStubGraphBuilder::BuildCodeStub() { Handle LoadConstantStub::GenerateCode() { return DoGenerateCode(this); } +HValue* CodeStubGraphBuilderBase::UnmappedCase(HValue* elements, HValue* key) { + HValue* result; + HInstruction* backing_store = Add( + elements, graph()->GetConstant1(), static_cast(NULL), + FAST_ELEMENTS, ALLOW_RETURN_HOLE); + Add(backing_store, isolate()->factory()->fixed_array_map()); + HValue* backing_store_length = + Add(backing_store, static_cast(NULL), + HObjectAccess::ForFixedArrayLength()); + IfBuilder in_unmapped_range(this); + in_unmapped_range.If(key, backing_store_length, + Token::LT); + in_unmapped_range.Then(); + { + result = Add(backing_store, key, static_cast(NULL), + FAST_HOLEY_ELEMENTS, NEVER_RETURN_HOLE); + } + in_unmapped_range.ElseDeopt("Outside of range"); + in_unmapped_range.End(); + return result; +} + + +template <> +HValue* CodeStubGraphBuilder::BuildCodeStub() { + HValue* receiver = GetParameter(LoadDescriptor::kReceiverIndex); + HValue* key = GetParameter(LoadDescriptor::kNameIndex); + + // Mapped arguments are actual arguments. Unmapped arguments are values added + // to the arguments object after it was created for the call. Mapped arguments + // are stored in the context at indexes given by elements[key + 2]. Unmapped + // arguments are stored as regular indexed properties in the arguments array, + // held at elements[1]. See NewSloppyArguments() in runtime.cc for a detailed + // look at argument object construction. + // + // The sloppy arguments elements array has a special format: + // + // 0: context + // 1: unmapped arguments array + // 2: mapped_index0, + // 3: mapped_index1, + // ... + // + // length is 2 + min(number_of_actual_arguments, number_of_formal_arguments). + // If key + 2 >= elements.length then attempt to look in the unmapped + // arguments array (given by elements[1]) and return the value at key, missing + // to the runtime if the unmapped arguments array is not a fixed array or if + // key >= unmapped_arguments_array.length. + // + // Otherwise, t = elements[key + 2]. If t is the hole, then look up the value + // in the unmapped arguments array, as described above. Otherwise, t is a Smi + // index into the context array given at elements[0]. Return the value at + // context[t]. + + key = AddUncasted(key, Representation::Smi()); + IfBuilder positive_smi(this); + positive_smi.If(key, graph()->GetConstant0(), + Token::LT); + positive_smi.ThenDeopt("key is negative"); + positive_smi.End(); + + HValue* constant_two = Add(2); + HValue* elements = AddLoadElements(receiver, static_cast(NULL)); + HValue* elements_length = + Add(elements, static_cast(NULL), + HObjectAccess::ForFixedArrayLength()); + HValue* adjusted_length = AddUncasted(elements_length, constant_two); + IfBuilder in_range(this); + in_range.If(key, adjusted_length, Token::LT); + in_range.Then(); + { + HValue* index = AddUncasted(key, constant_two); + HInstruction* mapped_index = + Add(elements, index, static_cast(NULL), + FAST_HOLEY_ELEMENTS, ALLOW_RETURN_HOLE); + + IfBuilder is_valid(this); + is_valid.IfNot(mapped_index, + graph()->GetConstantHole()); + is_valid.Then(); + { + // TODO(mvstanton): I'd like to assert from this point, that if the + // mapped_index is not the hole that it is indeed, a smi. An unnecessary + // smi check is being emitted. + HValue* the_context = + Add(elements, graph()->GetConstant0(), + static_cast(NULL), FAST_ELEMENTS); + DCHECK(Context::kHeaderSize == FixedArray::kHeaderSize); + HValue* result = + Add(the_context, mapped_index, static_cast(NULL), + FAST_ELEMENTS, ALLOW_RETURN_HOLE); + environment()->Push(result); + } + is_valid.Else(); + { + HValue* result = UnmappedCase(elements, key); + environment()->Push(result); + } + is_valid.End(); + } + in_range.Else(); + { + HValue* result = UnmappedCase(elements, key); + environment()->Push(result); + } + in_range.End(); + + return environment()->Pop(); +} + + +Handle KeyedLoadSloppyArgumentsStub::GenerateCode() { + return DoGenerateCode(this); +} + + void CodeStubGraphBuilderBase::BuildStoreNamedField( HValue* object, HValue* value, FieldIndex index, Representation representation) { @@ -1092,7 +1210,6 @@ Handle ToBooleanStub::GenerateCode() { template <> HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { StoreGlobalStub* stub = casted_stub(); - Handle hole(isolate()->heap()->the_hole_value(), isolate()); Handle placeholer_value(Smi::FromInt(0), isolate()); Handle placeholder_cell = isolate()->factory()->NewPropertyCell(placeholer_value); @@ -1124,7 +1241,7 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { // property has been deleted and that the store must be handled by the // runtime. IfBuilder builder(this); - HValue* hole_value = Add(hole); + HValue* hole_value = graph()->GetConstantHole(); builder.If(cell_contents, hole_value); builder.Then(); builder.Deopt("Unexpected cell contents in global store"); diff --git a/src/code-stubs.cc b/src/code-stubs.cc index 96460c5..5c9e1a2 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -586,12 +586,14 @@ void KeyedLoadGenericStub::InitializeDescriptor( void HandlerStub::InitializeDescriptor(CodeStubDescriptor* descriptor) { if (kind() == Code::STORE_IC) { descriptor->Initialize(FUNCTION_ADDR(StoreIC_MissFromStubFailure)); + } else if (kind() == Code::KEYED_LOAD_IC) { + descriptor->Initialize(FUNCTION_ADDR(KeyedLoadIC_MissFromStubFailure)); } } CallInterfaceDescriptor HandlerStub::GetCallInterfaceDescriptor() { - if (kind() == Code::LOAD_IC) { + if (kind() == Code::LOAD_IC || kind() == Code::KEYED_LOAD_IC) { return LoadDescriptor(isolate()); } else { DCHECK_EQ(Code::STORE_IC, kind()); diff --git a/src/code-stubs.h b/src/code-stubs.h index f9016f1..3b31399 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -82,6 +82,7 @@ namespace internal { /* IC Handler stubs */ \ V(LoadConstant) \ V(LoadField) \ + V(KeyedLoadSloppyArguments) \ V(StoreField) \ V(StoreGlobal) \ V(StringLength) @@ -914,6 +915,20 @@ class LoadFieldStub: public HandlerStub { }; +class KeyedLoadSloppyArgumentsStub : public HandlerStub { + public: + explicit KeyedLoadSloppyArgumentsStub(Isolate* isolate) + : HandlerStub(isolate) {} + + protected: + virtual Code::Kind kind() const { return Code::KEYED_LOAD_IC; } + virtual Code::StubType GetStubType() { return Code::FAST; } + + private: + DEFINE_HANDLER_CODE_STUB(KeyedLoadSloppyArguments, HandlerStub); +}; + + class LoadConstantStub : public HandlerStub { public: LoadConstantStub(Isolate* isolate, int constant_index) diff --git a/src/elements-kind.h b/src/elements-kind.h index b48a5df..fb97341 100644 --- a/src/elements-kind.h +++ b/src/elements-kind.h @@ -87,6 +87,11 @@ inline bool IsDictionaryElementsKind(ElementsKind kind) { } +inline bool IsSloppyArgumentsElements(ElementsKind kind) { + return kind == SLOPPY_ARGUMENTS_ELEMENTS; +} + + inline bool IsExternalArrayElementsKind(ElementsKind kind) { return kind >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND && kind <= LAST_EXTERNAL_ARRAY_ELEMENTS_KIND; diff --git a/src/ic/arm/ic-arm.cc b/src/ic/arm/ic-arm.cc index 1b6cf72..ae13161 100644 --- a/src/ic/arm/ic-arm.cc +++ b/src/ic/arm/ic-arm.cc @@ -369,32 +369,6 @@ static MemOperand GenerateUnmappedArgumentsLookup(MacroAssembler* masm, } -void KeyedLoadIC::GenerateSloppyArguments(MacroAssembler* masm) { - // The return address is in lr. - Register receiver = LoadDescriptor::ReceiverRegister(); - Register key = LoadDescriptor::NameRegister(); - DCHECK(receiver.is(r1)); - DCHECK(key.is(r2)); - - Label slow, notin; - MemOperand mapped_location = GenerateMappedArgumentsLookup( - masm, receiver, key, r0, r3, r4, ¬in, &slow); - __ ldr(r0, mapped_location); - __ Ret(); - __ bind(¬in); - // The unmapped lookup expects that the parameter map is in r0. - MemOperand unmapped_location = - GenerateUnmappedArgumentsLookup(masm, key, r0, r3, &slow); - __ ldr(r0, unmapped_location); - __ LoadRoot(r3, Heap::kTheHoleValueRootIndex); - __ cmp(r0, r3); - __ b(eq, &slow); - __ Ret(); - __ bind(&slow); - GenerateMiss(masm); -} - - void KeyedStoreIC::GenerateSloppyArguments(MacroAssembler* masm) { Register receiver = StoreDescriptor::ReceiverRegister(); Register key = StoreDescriptor::NameRegister(); diff --git a/src/ic/arm64/ic-arm64.cc b/src/ic/arm64/ic-arm64.cc index 9e5427f..76f9c24 100644 --- a/src/ic/arm64/ic-arm64.cc +++ b/src/ic/arm64/ic-arm64.cc @@ -370,35 +370,6 @@ void LoadIC::GenerateRuntimeGetProperty(MacroAssembler* masm) { } -void KeyedLoadIC::GenerateSloppyArguments(MacroAssembler* masm) { - // The return address is in lr. - Register result = x0; - Register receiver = LoadDescriptor::ReceiverRegister(); - Register key = LoadDescriptor::NameRegister(); - DCHECK(receiver.is(x1)); - DCHECK(key.is(x2)); - - Label miss, unmapped; - - Register map_scratch = x0; - MemOperand mapped_location = GenerateMappedArgumentsLookup( - masm, receiver, key, map_scratch, x3, x4, &unmapped, &miss); - __ Ldr(result, mapped_location); - __ Ret(); - - __ Bind(&unmapped); - // Parameter map is left in map_scratch when a jump on unmapped is done. - MemOperand unmapped_location = - GenerateUnmappedArgumentsLookup(masm, key, map_scratch, x3, &miss); - __ Ldr(result, unmapped_location); - __ JumpIfRoot(result, Heap::kTheHoleValueRootIndex, &miss); - __ Ret(); - - __ Bind(&miss); - GenerateMiss(masm); -} - - void KeyedStoreIC::GenerateSloppyArguments(MacroAssembler* masm) { ASM_LOCATION("KeyedStoreIC::GenerateSloppyArguments"); Label slow, notin; diff --git a/src/ic/handler-compiler.cc b/src/ic/handler-compiler.cc index ecd3e3b..4ed92ec 100644 --- a/src/ic/handler-compiler.cc +++ b/src/ic/handler-compiler.cc @@ -390,13 +390,13 @@ void ElementHandlerCompiler::CompileElementHandlers( ElementsKind elements_kind = receiver_map->elements_kind(); if (receiver_map->has_indexed_interceptor()) { cached_stub = LoadIndexedInterceptorStub(isolate()).GetCode(); + } else if (IsSloppyArgumentsElements(elements_kind)) { + cached_stub = KeyedLoadSloppyArgumentsStub(isolate()).GetCode(); } else if (IsFastElementsKind(elements_kind) || IsExternalArrayElementsKind(elements_kind) || IsFixedTypedArrayElementsKind(elements_kind)) { cached_stub = LoadFastElementStub(isolate(), is_js_array, elements_kind) .GetCode(); - } else if (elements_kind == SLOPPY_ARGUMENTS_ELEMENTS) { - cached_stub = isolate()->builtins()->KeyedLoadIC_SloppyArguments(); } else { DCHECK(elements_kind == DICTIONARY_ELEMENTS); cached_stub = LoadDictionaryElementStub(isolate()).GetCode(); diff --git a/src/ic/ia32/ic-ia32.cc b/src/ic/ia32/ic-ia32.cc index 2b4c954..67247d2 100644 --- a/src/ic/ia32/ic-ia32.cc +++ b/src/ic/ia32/ic-ia32.cc @@ -503,32 +503,6 @@ void KeyedLoadIC::GenerateString(MacroAssembler* masm) { } -void KeyedLoadIC::GenerateSloppyArguments(MacroAssembler* masm) { - // The return address is on the stack. - Register receiver = LoadDescriptor::ReceiverRegister(); - Register key = LoadDescriptor::NameRegister(); - DCHECK(receiver.is(edx)); - DCHECK(key.is(ecx)); - - Label slow, notin; - Factory* factory = masm->isolate()->factory(); - Operand mapped_location = GenerateMappedArgumentsLookup( - masm, receiver, key, ebx, eax, ¬in, &slow); - __ mov(eax, mapped_location); - __ Ret(); - __ bind(¬in); - // The unmapped lookup expects that the parameter map is in ebx. - Operand unmapped_location = - GenerateUnmappedArgumentsLookup(masm, key, ebx, eax, &slow); - __ cmp(unmapped_location, factory->the_hole_value()); - __ j(equal, &slow); - __ mov(eax, unmapped_location); - __ Ret(); - __ bind(&slow); - GenerateMiss(masm); -} - - void KeyedStoreIC::GenerateSloppyArguments(MacroAssembler* masm) { // Return address is on the stack. Label slow, notin; diff --git a/src/ic/ic-compiler.cc b/src/ic/ic-compiler.cc index 3b9f51c..aeae4ba 100644 --- a/src/ic/ic-compiler.cc +++ b/src/ic/ic-compiler.cc @@ -96,6 +96,8 @@ Handle PropertyICCompiler::ComputeKeyedLoadMonomorphic( Handle stub; if (receiver_map->has_indexed_interceptor()) { stub = LoadIndexedInterceptorStub(isolate).GetCode(); + } else if (receiver_map->has_sloppy_arguments_elements()) { + stub = KeyedLoadSloppyArgumentsStub(isolate).GetCode(); } else if (receiver_map->has_fast_elements() || receiver_map->has_external_array_elements() || receiver_map->has_fixed_typed_array_elements()) { diff --git a/src/ic/ic.cc b/src/ic/ic.cc index 50c9d72..13ba016 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -1187,11 +1187,7 @@ MaybeHandle KeyedLoadIC::Load(Handle object, if (state() == UNINITIALIZED) stub = string_stub(); } else if (object->IsJSObject()) { Handle receiver = Handle::cast(object); - if (receiver->elements()->map() == - isolate()->heap()->sloppy_arguments_elements_map()) { - stub = sloppy_arguments_stub(); - } else if (!Object::ToSmi(isolate(), key).is_null() && - (!target().is_identical_to(sloppy_arguments_stub()))) { + if (!Object::ToSmi(isolate(), key).is_null()) { stub = LoadElementStub(receiver); } } diff --git a/src/ic/ic.h b/src/ic/ic.h index 57d0d12..55ed80f 100644 --- a/src/ic/ic.h +++ b/src/ic/ic.h @@ -414,7 +414,6 @@ class KeyedLoadIC : public LoadIC { } static void GenerateGeneric(MacroAssembler* masm); static void GenerateString(MacroAssembler* masm); - static void GenerateSloppyArguments(MacroAssembler* masm); // Bit mask to be tested against bit field for the cases when // generic stub should go into slow case. @@ -434,9 +433,6 @@ class KeyedLoadIC : public LoadIC { private: Handle generic_stub() const { return generic_stub(isolate()); } - Handle sloppy_arguments_stub() { - return isolate()->builtins()->KeyedLoadIC_SloppyArguments(); - } Handle string_stub() { return isolate()->builtins()->KeyedLoadIC_String(); } diff --git a/src/ic/mips/ic-mips.cc b/src/ic/mips/ic-mips.cc index 72a85b6..d97a6ba 100644 --- a/src/ic/mips/ic-mips.cc +++ b/src/ic/mips/ic-mips.cc @@ -374,32 +374,6 @@ static MemOperand GenerateUnmappedArgumentsLookup(MacroAssembler* masm, } -void KeyedLoadIC::GenerateSloppyArguments(MacroAssembler* masm) { - // The return address is in ra. - Register receiver = LoadDescriptor::ReceiverRegister(); - Register key = LoadDescriptor::NameRegister(); - DCHECK(receiver.is(a1)); - DCHECK(key.is(a2)); - - Label slow, notin; - MemOperand mapped_location = GenerateMappedArgumentsLookup( - masm, receiver, key, a0, a3, t0, ¬in, &slow); - __ Ret(USE_DELAY_SLOT); - __ lw(v0, mapped_location); - __ bind(¬in); - // The unmapped lookup expects that the parameter map is in a0. - MemOperand unmapped_location = - GenerateUnmappedArgumentsLookup(masm, key, a0, a3, &slow); - __ lw(a0, unmapped_location); - __ LoadRoot(a3, Heap::kTheHoleValueRootIndex); - __ Branch(&slow, eq, a0, Operand(a3)); - __ Ret(USE_DELAY_SLOT); - __ mov(v0, a0); - __ bind(&slow); - GenerateMiss(masm); -} - - void KeyedStoreIC::GenerateSloppyArguments(MacroAssembler* masm) { Register receiver = StoreDescriptor::ReceiverRegister(); Register key = StoreDescriptor::NameRegister(); diff --git a/src/ic/x64/ic-x64.cc b/src/ic/x64/ic-x64.cc index dc1b86b..ad79f30 100644 --- a/src/ic/x64/ic-x64.cc +++ b/src/ic/x64/ic-x64.cc @@ -722,31 +722,6 @@ static Operand GenerateUnmappedArgumentsLookup(MacroAssembler* masm, } -void KeyedLoadIC::GenerateSloppyArguments(MacroAssembler* masm) { - // The return address is on the stack. - Register receiver = LoadDescriptor::ReceiverRegister(); - Register key = LoadDescriptor::NameRegister(); - DCHECK(receiver.is(rdx)); - DCHECK(key.is(rcx)); - - Label slow, notin; - Operand mapped_location = GenerateMappedArgumentsLookup( - masm, receiver, key, rbx, rax, rdi, ¬in, &slow); - __ movp(rax, mapped_location); - __ Ret(); - __ bind(¬in); - // The unmapped lookup expects that the parameter map is in rbx. - Operand unmapped_location = - GenerateUnmappedArgumentsLookup(masm, key, rbx, rax, &slow); - __ CompareRoot(unmapped_location, Heap::kTheHoleValueRootIndex); - __ j(equal, &slow); - __ movp(rax, unmapped_location); - __ Ret(); - __ bind(&slow); - GenerateMiss(masm); -} - - void KeyedStoreIC::GenerateSloppyArguments(MacroAssembler* masm) { // The return address is on the stack. Label slow, notin; -- 2.7.4