From 76483f871bb13d37cc1a3835460ac2ff07eeee19 Mon Sep 17 00:00:00 2001 From: "vitalyr@chromium.org" Date: Tue, 25 Jan 2011 15:51:10 +0000 Subject: [PATCH] Support StringLength in hydrogen (similar to ArrayLength). To avoid deopts a few extra changes were needed: o Enable megamorphic state for special property loads on primitives. We used to flip between monomorphic stubs. o Extract pure string (no string wrapper support) version of the string length stub. Review URL: http://codereview.chromium.org/6334015 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6472 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/ic-arm.cc | 5 +++-- src/arm/stub-cache-arm.cc | 28 +++++++++++++---------- src/ast.cc | 2 ++ src/ast.h | 13 ++++++----- src/builtins.cc | 7 +++++- src/builtins.h | 1 + src/hydrogen.cc | 8 +++++++ src/ia32/ic-ia32.cc | 6 +++-- src/ia32/stub-cache-ia32.cc | 30 ++++++++++++++----------- src/ic.cc | 55 ++++++++++++++++++++++++++++----------------- src/ic.h | 3 ++- src/stub-cache.h | 3 ++- src/x64/ic-x64.cc | 5 +++-- src/x64/stub-cache-x64.cc | 30 ++++++++++++++----------- 14 files changed, 124 insertions(+), 72 deletions(-) diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc index a3afaae..1421f06 100644 --- a/src/arm/ic-arm.cc +++ b/src/arm/ic-arm.cc @@ -379,7 +379,7 @@ void LoadIC::GenerateArrayLength(MacroAssembler* masm) { } -void LoadIC::GenerateStringLength(MacroAssembler* masm) { +void LoadIC::GenerateStringLength(MacroAssembler* masm, bool support_wrappers) { // ----------- S t a t e ------------- // -- r2 : name // -- lr : return address @@ -388,7 +388,8 @@ void LoadIC::GenerateStringLength(MacroAssembler* masm) { // ----------------------------------- Label miss; - StubCompiler::GenerateLoadStringLength(masm, r0, r1, r3, &miss); + StubCompiler::GenerateLoadStringLength(masm, r0, r1, r3, &miss, + support_wrappers); // Cache miss: Jump to runtime. __ bind(&miss); StubCompiler::GenerateLoadMiss(masm, Code::LOAD_IC); diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index ce1d854..ee98d12 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -370,27 +370,31 @@ void StubCompiler::GenerateLoadStringLength(MacroAssembler* masm, Register receiver, Register scratch1, Register scratch2, - Label* miss) { + Label* miss, + bool support_wrappers) { Label check_wrapper; // Check if the object is a string leaving the instance type in the // scratch1 register. - GenerateStringCheck(masm, receiver, scratch1, scratch2, miss, &check_wrapper); + GenerateStringCheck(masm, receiver, scratch1, scratch2, miss, + support_wrappers ? &check_wrapper : miss); // Load length directly from the string. __ ldr(r0, FieldMemOperand(receiver, String::kLengthOffset)); __ Ret(); - // Check if the object is a JSValue wrapper. - __ bind(&check_wrapper); - __ cmp(scratch1, Operand(JS_VALUE_TYPE)); - __ b(ne, miss); + if (support_wrappers) { + // Check if the object is a JSValue wrapper. + __ bind(&check_wrapper); + __ cmp(scratch1, Operand(JS_VALUE_TYPE)); + __ b(ne, miss); - // Unwrap the value and check if the wrapped value is a string. - __ ldr(scratch1, FieldMemOperand(receiver, JSValue::kValueOffset)); - GenerateStringCheck(masm, scratch1, scratch2, scratch2, miss, miss); - __ ldr(r0, FieldMemOperand(scratch1, String::kLengthOffset)); - __ Ret(); + // Unwrap the value and check if the wrapped value is a string. + __ ldr(scratch1, FieldMemOperand(receiver, JSValue::kValueOffset)); + GenerateStringCheck(masm, scratch1, scratch2, scratch2, miss, miss); + __ ldr(r0, FieldMemOperand(scratch1, String::kLengthOffset)); + __ Ret(); + } } @@ -2995,7 +2999,7 @@ MaybeObject* KeyedLoadStubCompiler::CompileLoadStringLength(String* name) { __ cmp(r0, Operand(Handle(name))); __ b(ne, &miss); - GenerateLoadStringLength(masm(), r1, r2, r3, &miss); + GenerateLoadStringLength(masm(), r1, r2, r3, &miss, true); __ bind(&miss); __ DecrementCounter(&Counters::keyed_load_string_length, 1, r2, r3); diff --git a/src/ast.cc b/src/ast.cc index e8b3e03..11cf334 100644 --- a/src/ast.cc +++ b/src/ast.cc @@ -521,6 +521,8 @@ void Property::RecordTypeFeedback(TypeFeedbackOracle* oracle) { if (key()->IsPropertyName()) { if (oracle->LoadIsBuiltin(this, Builtins::LoadIC_ArrayLength)) { is_array_length_ = true; + } else if (oracle->LoadIsBuiltin(this, Builtins::LoadIC_StringLength)) { + is_string_length_ = true; } else if (oracle->LoadIsBuiltin(this, Builtins::LoadIC_FunctionPrototype)) { is_function_prototype_ = true; diff --git a/src/ast.h b/src/ast.h index a897e88..762a4fa 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1205,9 +1205,10 @@ class Property: public Expression { key_(key), pos_(pos), type_(type), - is_monomorphic_(false), receiver_types_(NULL), + is_monomorphic_(false), is_array_length_(false), + is_string_length_(false), is_function_prototype_(false), is_arguments_access_(false) { } @@ -1221,6 +1222,7 @@ class Property: public Expression { int position() const { return pos_; } bool is_synthetic() const { return type_ == SYNTHETIC; } + bool IsStringLength() const { return is_string_length_; } bool IsFunctionPrototype() const { return is_function_prototype_; } // Marks that this is actually an argument rewritten to a keyed property @@ -1249,11 +1251,12 @@ class Property: public Expression { int pos_; Type type_; - bool is_monomorphic_; ZoneMapList* receiver_types_; - bool is_array_length_; - bool is_function_prototype_; - bool is_arguments_access_; + bool is_monomorphic_ : 1; + bool is_array_length_ : 1; + bool is_string_length_ : 1; + bool is_function_prototype_ : 1; + bool is_arguments_access_ : 1; Handle monomorphic_receiver_type_; // Dummy property used during preparsing. diff --git a/src/builtins.cc b/src/builtins.cc index 7c2c2bc..58dd439 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -1228,7 +1228,12 @@ static void Generate_LoadIC_ArrayLength(MacroAssembler* masm) { static void Generate_LoadIC_StringLength(MacroAssembler* masm) { - LoadIC::GenerateStringLength(masm); + LoadIC::GenerateStringLength(masm, false); +} + + +static void Generate_LoadIC_StringWrapperLength(MacroAssembler* masm) { + LoadIC::GenerateStringLength(masm, true); } diff --git a/src/builtins.h b/src/builtins.h index 39f3546..88d31c7 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -86,6 +86,7 @@ enum BuiltinExtraArguments { V(LoadIC_Normal, LOAD_IC, MONOMORPHIC) \ V(LoadIC_ArrayLength, LOAD_IC, MONOMORPHIC) \ V(LoadIC_StringLength, LOAD_IC, MONOMORPHIC) \ + V(LoadIC_StringWrapperLength, LOAD_IC, MONOMORPHIC) \ V(LoadIC_FunctionPrototype, LOAD_IC, MONOMORPHIC) \ V(LoadIC_Megamorphic, LOAD_IC, MEGAMORPHIC) \ \ diff --git a/src/hydrogen.cc b/src/hydrogen.cc index ae91065..7ede51d 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -3766,6 +3766,14 @@ void HGraphBuilder::VisitProperty(Property* expr) { AddInstruction(new HCheckInstanceType(array, JS_ARRAY_TYPE, JS_ARRAY_TYPE)); instr = new HJSArrayLength(array); + } else if (expr->IsStringLength()) { + HValue* string = Pop(); + AddInstruction(new HCheckNonSmi(string)); + AddInstruction(new HCheckInstanceType(string, + FIRST_STRING_TYPE, + LAST_STRING_TYPE)); + instr = new HStringLength(string); + } else if (expr->IsFunctionPrototype()) { HValue* function = Pop(); AddInstruction(new HCheckNonSmi(function)); diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index c234b36..c136977 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -388,7 +388,8 @@ void LoadIC::GenerateArrayLength(MacroAssembler* masm) { } -void LoadIC::GenerateStringLength(MacroAssembler* masm) { +void LoadIC::GenerateStringLength(MacroAssembler* masm, + bool support_wrappers) { // ----------- S t a t e ------------- // -- eax : receiver // -- ecx : name @@ -396,7 +397,8 @@ void LoadIC::GenerateStringLength(MacroAssembler* masm) { // ----------------------------------- Label miss; - StubCompiler::GenerateLoadStringLength(masm, eax, edx, ebx, &miss); + StubCompiler::GenerateLoadStringLength(masm, eax, edx, ebx, &miss, + support_wrappers); __ bind(&miss); StubCompiler::GenerateLoadMiss(masm, Code::LOAD_IC); } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index 6d353c2..f8e9d72 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -327,28 +327,32 @@ void StubCompiler::GenerateLoadStringLength(MacroAssembler* masm, Register receiver, Register scratch1, Register scratch2, - Label* miss) { + Label* miss, + bool support_wrappers) { Label check_wrapper; // Check if the object is a string leaving the instance type in the // scratch register. - GenerateStringCheck(masm, receiver, scratch1, miss, &check_wrapper); + GenerateStringCheck(masm, receiver, scratch1, miss, + support_wrappers ? &check_wrapper : miss); // Load length from the string and convert to a smi. __ mov(eax, FieldOperand(receiver, String::kLengthOffset)); __ ret(0); - // Check if the object is a JSValue wrapper. - __ bind(&check_wrapper); - __ cmp(scratch1, JS_VALUE_TYPE); - __ j(not_equal, miss, not_taken); + if (support_wrappers) { + // Check if the object is a JSValue wrapper. + __ bind(&check_wrapper); + __ cmp(scratch1, JS_VALUE_TYPE); + __ j(not_equal, miss, not_taken); - // Check if the wrapped value is a string and load the length - // directly if it is. - __ mov(scratch2, FieldOperand(receiver, JSValue::kValueOffset)); - GenerateStringCheck(masm, scratch2, scratch1, miss, miss); - __ mov(eax, FieldOperand(scratch2, String::kLengthOffset)); - __ ret(0); + // Check if the wrapped value is a string and load the length + // directly if it is. + __ mov(scratch2, FieldOperand(receiver, JSValue::kValueOffset)); + GenerateStringCheck(masm, scratch2, scratch1, miss, miss); + __ mov(eax, FieldOperand(scratch2, String::kLengthOffset)); + __ ret(0); + } } @@ -3089,7 +3093,7 @@ MaybeObject* KeyedLoadStubCompiler::CompileLoadStringLength(String* name) { __ cmp(Operand(eax), Immediate(Handle(name))); __ j(not_equal, &miss, not_taken); - GenerateLoadStringLength(masm(), edx, ecx, ebx, &miss); + GenerateLoadStringLength(masm(), edx, ecx, ebx, &miss, true); __ bind(&miss); __ DecrementCounter(&Counters::keyed_load_string_length, 1); GenerateLoadMiss(masm(), Code::KEYED_LOAD_IC); diff --git a/src/ic.cc b/src/ic.cc index dcb4712..9a277d6 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -822,6 +822,9 @@ MaybeObject* LoadIC::Load(State state, } if (FLAG_use_ic) { + Code* non_monomorphic_stub = + (state == UNINITIALIZED) ? pre_monomorphic_stub() : megamorphic_stub(); + // Use specialized code for getting the length of strings and // string wrapper objects. The length property of string wrapper // objects is read-only and therefore always returns the length of @@ -829,22 +832,27 @@ MaybeObject* LoadIC::Load(State state, if ((object->IsString() || object->IsStringWrapper()) && name->Equals(Heap::length_symbol())) { HandleScope scope; - // Get the string if we have a string wrapper object. - if (object->IsJSValue()) { - object = Handle(Handle::cast(object)->value()); - } #ifdef DEBUG if (FLAG_trace_ic) PrintF("[LoadIC : +#length /string]\n"); #endif - Map* map = HeapObject::cast(*object)->map(); - if (object->IsString()) { - const int offset = String::kLengthOffset; - PatchInlinedLoad(address(), map, offset); + if (state == PREMONOMORPHIC) { + if (object->IsString()) { + Map* map = HeapObject::cast(*object)->map(); + const int offset = String::kLengthOffset; + PatchInlinedLoad(address(), map, offset); + set_target(Builtins::builtin(Builtins::LoadIC_StringLength)); + } else { + set_target(Builtins::builtin(Builtins::LoadIC_StringWrapperLength)); + } + } else if (state == MONOMORPHIC && object->IsStringWrapper()) { + set_target(Builtins::builtin(Builtins::LoadIC_StringWrapperLength)); + } else { + set_target(non_monomorphic_stub); + } + // Get the string if we have a string wrapper object. + if (object->IsJSValue()) { + object = Handle(Handle::cast(object)->value()); } - - Code* target = NULL; - target = Builtins::builtin(Builtins::LoadIC_StringLength); - set_target(target); return Smi::FromInt(String::cast(*object)->length()); } @@ -853,12 +861,14 @@ MaybeObject* LoadIC::Load(State state, #ifdef DEBUG if (FLAG_trace_ic) PrintF("[LoadIC : +#length /array]\n"); #endif - Map* map = HeapObject::cast(*object)->map(); - const int offset = JSArray::kLengthOffset; - PatchInlinedLoad(address(), map, offset); - - Code* target = Builtins::builtin(Builtins::LoadIC_ArrayLength); - set_target(target); + if (state == PREMONOMORPHIC) { + Map* map = HeapObject::cast(*object)->map(); + const int offset = JSArray::kLengthOffset; + PatchInlinedLoad(address(), map, offset); + set_target(Builtins::builtin(Builtins::LoadIC_ArrayLength)); + } else { + set_target(non_monomorphic_stub); + } return JSArray::cast(*object)->length(); } @@ -868,8 +878,11 @@ MaybeObject* LoadIC::Load(State state, #ifdef DEBUG if (FLAG_trace_ic) PrintF("[LoadIC : +#prototype /function]\n"); #endif - Code* target = Builtins::builtin(Builtins::LoadIC_FunctionPrototype); - set_target(target); + if (state == PREMONOMORPHIC) { + set_target(Builtins::builtin(Builtins::LoadIC_FunctionPrototype)); + } else { + set_target(non_monomorphic_stub); + } return Accessors::FunctionGetPrototype(*object, 0); } } @@ -1092,6 +1105,8 @@ MaybeObject* KeyedLoadIC::Load(State state, } if (FLAG_use_ic) { + // TODO(1073): don't ignore the current stub state. + // Use specialized code for getting the length of strings. if (object->IsString() && name->Equals(Heap::length_symbol())) { Handle string = Handle::cast(object); diff --git a/src/ic.h b/src/ic.h index 55cb34a..409ad38 100644 --- a/src/ic.h +++ b/src/ic.h @@ -284,7 +284,8 @@ class LoadIC: public IC { // Specialized code generator routines. static void GenerateArrayLength(MacroAssembler* masm); - static void GenerateStringLength(MacroAssembler* masm); + static void GenerateStringLength(MacroAssembler* masm, + bool support_wrappers); static void GenerateFunctionPrototype(MacroAssembler* masm); // Clear the use of the inlined version. diff --git a/src/stub-cache.h b/src/stub-cache.h index 1f534d9..e6709ad 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -427,7 +427,8 @@ class StubCompiler BASE_EMBEDDED { Register receiver, Register scratch1, Register scratch2, - Label* miss_label); + Label* miss_label, + bool support_wrappers); static void GenerateLoadFunctionPrototype(MacroAssembler* masm, Register receiver, diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc index e31a341..d060e31 100644 --- a/src/x64/ic-x64.cc +++ b/src/x64/ic-x64.cc @@ -397,7 +397,7 @@ void LoadIC::GenerateArrayLength(MacroAssembler* masm) { } -void LoadIC::GenerateStringLength(MacroAssembler* masm) { +void LoadIC::GenerateStringLength(MacroAssembler* masm, bool support_wrappers) { // ----------- S t a t e ------------- // -- rax : receiver // -- rcx : name @@ -405,7 +405,8 @@ void LoadIC::GenerateStringLength(MacroAssembler* masm) { // ----------------------------------- Label miss; - StubCompiler::GenerateLoadStringLength(masm, rax, rdx, rbx, &miss); + StubCompiler::GenerateLoadStringLength(masm, rax, rdx, rbx, &miss, + support_wrappers); __ bind(&miss); StubCompiler::GenerateLoadMiss(masm, Code::LOAD_IC); } diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index c86f43d..ca095b7 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -307,28 +307,32 @@ void StubCompiler::GenerateLoadStringLength(MacroAssembler* masm, Register receiver, Register scratch1, Register scratch2, - Label* miss) { + Label* miss, + bool support_wrappers) { Label check_wrapper; // Check if the object is a string leaving the instance type in the // scratch register. - GenerateStringCheck(masm, receiver, scratch1, miss, &check_wrapper); + GenerateStringCheck(masm, receiver, scratch1, miss, + support_wrappers ? &check_wrapper : miss); // Load length directly from the string. __ movq(rax, FieldOperand(receiver, String::kLengthOffset)); __ ret(0); - // Check if the object is a JSValue wrapper. - __ bind(&check_wrapper); - __ cmpl(scratch1, Immediate(JS_VALUE_TYPE)); - __ j(not_equal, miss); + if (support_wrappers) { + // Check if the object is a JSValue wrapper. + __ bind(&check_wrapper); + __ cmpl(scratch1, Immediate(JS_VALUE_TYPE)); + __ j(not_equal, miss); - // Check if the wrapped value is a string and load the length - // directly if it is. - __ movq(scratch2, FieldOperand(receiver, JSValue::kValueOffset)); - GenerateStringCheck(masm, scratch2, scratch1, miss, miss); - __ movq(rax, FieldOperand(scratch2, String::kLengthOffset)); - __ ret(0); + // Check if the wrapped value is a string and load the length + // directly if it is. + __ movq(scratch2, FieldOperand(receiver, JSValue::kValueOffset)); + GenerateStringCheck(masm, scratch2, scratch1, miss, miss); + __ movq(rax, FieldOperand(scratch2, String::kLengthOffset)); + __ ret(0); + } } @@ -2933,7 +2937,7 @@ MaybeObject* KeyedLoadStubCompiler::CompileLoadStringLength(String* name) { __ Cmp(rax, Handle(name)); __ j(not_equal, &miss); - GenerateLoadStringLength(masm(), rdx, rcx, rbx, &miss); + GenerateLoadStringLength(masm(), rdx, rcx, rbx, &miss, true); __ bind(&miss); __ DecrementCounter(&Counters::keyed_load_string_length, 1); GenerateLoadMiss(masm(), Code::KEYED_LOAD_IC); -- 2.7.4