From 9dadae1bfba214d9cffbc4c0dcd31c4b8f76d104 Mon Sep 17 00:00:00 2001 From: "bak@chromium.org" Date: Tue, 7 Oct 2008 09:28:04 +0000 Subject: [PATCH] - Fixed Issue 3201: Embedded Google Calendar crashes the renderer ExtendStorage did not work with keyed store IC. - Reduced instructions generated when performing a tail call to kSharedStoreIC_ExtendStorage - Moved test/mjsunit/bugs/bug-109.js to test/mjsunit/keyed-storage-extend.js Review URL: http://codereview.chromium.org/6526 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@455 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/builtins.cc | 5 +++ src/builtins.h | 1 + src/ic-arm.cc | 6 ++- src/ic-ia32.cc | 23 +++++++++- src/ic.cc | 2 +- src/ic.h | 23 +++++----- src/stub-cache-arm.cc | 51 +++++++++++----------- src/stub-cache-ia32.cc | 45 ++++++++++++------- src/stub-cache.h | 1 + .../{bugs/bug-109.js => keyed-storage-extend.js} | 0 10 files changed, 102 insertions(+), 55 deletions(-) rename test/mjsunit/{bugs/bug-109.js => keyed-storage-extend.js} (100%) diff --git a/src/builtins.cc b/src/builtins.cc index a0684a2..43049f4 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -575,6 +575,11 @@ static void Generate_KeyedStoreIC_Generic(MacroAssembler* masm) { } +static void Generate_KeyedStoreIC_ExtendStorage(MacroAssembler* masm) { + KeyedStoreIC::GenerateExtendStorage(masm); +} + + static void Generate_KeyedStoreIC_Miss(MacroAssembler* masm) { KeyedStoreIC::GenerateMiss(masm); } diff --git a/src/builtins.h b/src/builtins.h index 531c9e2..14d4ee6 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -63,6 +63,7 @@ namespace v8 { namespace internal { V(KeyedStoreIC_Miss, BUILTIN, UNINITIALIZED) \ \ V(StoreIC_ExtendStorage, BUILTIN, UNINITIALIZED) \ + V(KeyedStoreIC_ExtendStorage, BUILTIN, UNINITIALIZED) \ \ V(LoadIC_Initialize, LOAD_IC, UNINITIALIZED) \ V(LoadIC_PreMonomorphic, LOAD_IC, PREMONOMORPHIC) \ diff --git a/src/ic-arm.cc b/src/ic-arm.cc index 6b23fc0..5a60322 100644 --- a/src/ic-arm.cc +++ b/src/ic-arm.cc @@ -539,6 +539,9 @@ void KeyedStoreIC::Generate(MacroAssembler* masm, void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) { } +void KeyedStoreIC::GenerateExtendStorage(MacroAssembler* masm) { +} + void StoreIC::GenerateMegamorphic(MacroAssembler* masm) { // ----------- S t a t e ------------- @@ -570,7 +573,8 @@ void StoreIC::GenerateExtendStorage(MacroAssembler* masm) { __ stm(db_w, sp, r0.bit() | r2.bit() | r3.bit()); // Perform tail call to the entry. - __ TailCallRuntime(ExternalReference(IC_Utility(kStoreIC_ExtendStorage)), 3); + __ TailCallRuntime( + ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)), 3); } diff --git a/src/ic-ia32.cc b/src/ic-ia32.cc index fa18e93..026c861 100644 --- a/src/ic-ia32.cc +++ b/src/ic-ia32.cc @@ -703,7 +703,8 @@ void StoreIC::GenerateExtendStorage(MacroAssembler* masm) { __ push(eax); __ push(ebx); // Perform tail call to the entry. - __ TailCallRuntime(ExternalReference(IC_Utility(kStoreIC_ExtendStorage)), 3); + __ TailCallRuntime( + ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)), 3); } @@ -750,6 +751,26 @@ void KeyedStoreIC::Generate(MacroAssembler* masm, const ExternalReference& f) { } +void KeyedStoreIC::GenerateExtendStorage(MacroAssembler* masm) { + // ----------- S t a t e ------------- + // -- eax : value + // -- esp[0] : return address + // -- esp[4] : key + // -- esp[8] : receiver + // ----------------------------------- + + // Move the return address below the arguments. + __ pop(ecx); + __ push(Operand(esp, 1 * kPointerSize)); + __ push(Operand(esp, 1 * kPointerSize)); + __ push(eax); + __ push(ecx); + + // Do tail-call to runtime routine. + __ TailCallRuntime( + ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)), 3); +} + #undef __ diff --git a/src/ic.cc b/src/ic.cc index 10bcf6b..61dc3d0 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -1115,7 +1115,7 @@ Object* StoreIC_Miss(Arguments args) { // Extend storage is called in a store inline cache when // it is necessary to extend the properties array of a // JSObject. -Object* StoreIC_ExtendStorage(Arguments args) { +Object* SharedStoreIC_ExtendStorage(Arguments args) { NoHandleAllocation na; ASSERT(args.length() == 3); diff --git a/src/ic.h b/src/ic.h index ed655bc..fd5c72b 100644 --- a/src/ic.h +++ b/src/ic.h @@ -34,17 +34,17 @@ namespace v8 { namespace internal { // IC_UTIL_LIST defines all utility functions called from generated // inline caching code. The argument for the macro, ICU, is the function name. -#define IC_UTIL_LIST(ICU) \ - ICU(LoadIC_Miss) \ - ICU(KeyedLoadIC_Miss) \ - ICU(CallIC_Miss) \ - ICU(StoreIC_Miss) \ - ICU(StoreIC_ExtendStorage) \ - ICU(KeyedStoreIC_Miss) \ - /* Utilities for IC stubs. */ \ - ICU(LoadCallbackProperty) \ - ICU(StoreCallbackProperty) \ - ICU(LoadInterceptorProperty) \ +#define IC_UTIL_LIST(ICU) \ + ICU(LoadIC_Miss) \ + ICU(KeyedLoadIC_Miss) \ + ICU(CallIC_Miss) \ + ICU(StoreIC_Miss) \ + ICU(SharedStoreIC_ExtendStorage) \ + ICU(KeyedStoreIC_Miss) \ + /* Utilities for IC stubs. */ \ + ICU(LoadCallbackProperty) \ + ICU(StoreCallbackProperty) \ + ICU(LoadInterceptorProperty) \ ICU(StoreInterceptorProperty) // @@ -333,6 +333,7 @@ class KeyedStoreIC: public IC { static void GenerateInitialize(MacroAssembler* masm); static void GenerateMiss(MacroAssembler* masm); static void GenerateGeneric(MacroAssembler* masm); + static void GenerateExtendStorage(MacroAssembler* masm); private: static void Generate(MacroAssembler* masm, const ExternalReference& f); diff --git a/src/stub-cache-arm.cc b/src/stub-cache-arm.cc index dee8c3e..2c7444d 100644 --- a/src/stub-cache-arm.cc +++ b/src/stub-cache-arm.cc @@ -411,41 +411,42 @@ Object* StoreStubCompiler::CompileStoreField(JSObject* object, // checks. ASSERT(object->IsJSGlobalObject() || !object->IsAccessCheckNeeded()); - // Get the properties array - __ ldr(r1, FieldMemOperand(r3, JSObject::kPropertiesOffset)); - // Perform map transition for the receiver if necessary. - if (transition != NULL) { - if (object->map()->unused_property_fields() == 0) { - // The properties must be extended before we can store the value. - // We jump to a runtime call that extends the propeties array. - __ mov(r2, Operand(Handle(transition))); - Handle ic(Builtins::builtin(Builtins::StoreIC_ExtendStorage)); - __ Jump(ic, RelocInfo::CODE_TARGET); - } else { + if ((transition != NULL) && (object->map()->unused_property_fields() == 0)) { + // The properties must be extended before we can store the value. + // We jump to a runtime call that extends the propeties array. + __ mov(r2, Operand(Handle(transition))); + // Please note, if we implement keyed store for arm we need + // to call the Builtins::KeyedStoreIC_ExtendStorage. + Handle ic(Builtins::builtin(Builtins::StoreIC_ExtendStorage)); + __ Jump(ic, RelocInfo::CODE_TARGET); + } else { + // Get the properties array + __ ldr(r1, FieldMemOperand(r3, JSObject::kPropertiesOffset)); + + if (transition != NULL) { // Update the map of the object; no write barrier updating is // needed because the map is never in new space. __ mov(ip, Operand(Handle(transition))); __ str(ip, FieldMemOperand(r3, HeapObject::kMapOffset)); } - } - // Write to the properties array. - int offset = index * kPointerSize + Array::kHeaderSize; - __ str(r0, FieldMemOperand(r1, offset)); + // Write to the properties array. + int offset = index * kPointerSize + Array::kHeaderSize; + __ str(r0, FieldMemOperand(r1, offset)); - // Skip updating write barrier if storing a smi. - __ tst(r0, Operand(kSmiTagMask)); - __ b(eq, &exit); + // Skip updating write barrier if storing a smi. + __ tst(r0, Operand(kSmiTagMask)); + __ b(eq, &exit); - // Update the write barrier for the array address. - __ mov(r3, Operand(offset)); - __ RecordWrite(r1, r3, r2); // OK to clobber r2, since we return - - // Return the value (register r0). - __ bind(&exit); - __ Ret(); + // Update the write barrier for the array address. + __ mov(r3, Operand(offset)); + __ RecordWrite(r1, r3, r2); // OK to clobber r2, since we return + // Return the value (register r0). + __ bind(&exit); + __ Ret(); + } // Handle store cache miss. __ bind(&miss); __ mov(r2, Operand(Handle(name))); // restore name diff --git a/src/stub-cache-ia32.cc b/src/stub-cache-ia32.cc index 5bb9794..b0b52a0 100644 --- a/src/stub-cache-ia32.cc +++ b/src/stub-cache-ia32.cc @@ -406,6 +406,7 @@ void StubCompiler::GenerateLoadMiss(MacroAssembler* masm, Code::Kind kind) { void StubCompiler::GenerateStoreField(MacroAssembler* masm, + Builtins::Name storage_extend, JSObject* object, int index, Map* transition, @@ -431,23 +432,23 @@ void StubCompiler::GenerateStoreField(MacroAssembler* masm, // checks. ASSERT(object->IsJSGlobalObject() || !object->IsAccessCheckNeeded()); + // Perform map transition for the receiver if necessary. + if ((transition != NULL) && (object->map()->unused_property_fields() == 0)) { + // The properties must be extended before we can store the value. + // We jump to a runtime call that extends the propeties array. + __ mov(Operand(ecx), Immediate(Handle(transition))); + Handle ic(Builtins::builtin(storage_extend)); + __ jmp(ic, RelocInfo::CODE_TARGET); + return; + } + // Get the properties array (optimistically). __ mov(scratch, FieldOperand(receiver_reg, JSObject::kPropertiesOffset)); - - // Perform map transition for the receiver if necessary. if (transition != NULL) { - if (object->map()->unused_property_fields() == 0) { - // The properties must be extended before we can store the value. - // We jump to a runtime call that extends the propeties array. - __ mov(Operand(ecx), Immediate(Handle(transition))); - Handle ic(Builtins::builtin(Builtins::StoreIC_ExtendStorage)); - __ jmp(ic, RelocInfo::CODE_TARGET); - } else { - // Update the map of the object; no write barrier updating is - // needed because the map is never in new space. - __ mov(FieldOperand(receiver_reg, HeapObject::kMapOffset), - Immediate(Handle(transition))); - } + // Update the map of the object; no write barrier updating is + // needed because the map is never in new space. + __ mov(FieldOperand(receiver_reg, HeapObject::kMapOffset), + Immediate(Handle(transition))); } // Write to the properties array. @@ -737,7 +738,13 @@ Object* StoreStubCompiler::CompileStoreField(JSObject* object, __ mov(ebx, Operand(esp, 1 * kPointerSize)); // Generate store field code. Trashes the name register. - GenerateStoreField(masm(), object, index, transition, ebx, ecx, edx, &miss); + GenerateStoreField(masm(), + Builtins::StoreIC_ExtendStorage, + object, + index, + transition, + ebx, ecx, edx, + &miss); // Handle store cache miss. __ bind(&miss); @@ -887,7 +894,13 @@ Object* KeyedStoreStubCompiler::CompileStoreField(JSObject* object, __ mov(ebx, Operand(esp, 2 * kPointerSize)); // Generate store field code. Trashes the name register. - GenerateStoreField(masm(), object, index, transition, ebx, ecx, edx, &miss); + GenerateStoreField(masm(), + Builtins::KeyedStoreIC_ExtendStorage, + object, + index, + transition, + ebx, ecx, edx, + &miss); // Handle store cache miss. __ bind(&miss); diff --git a/src/stub-cache.h b/src/stub-cache.h index e735405..4bf13d9 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -356,6 +356,7 @@ class StubCompiler BASE_EMBEDDED { Register scratch2, Label* miss_label); static void GenerateStoreField(MacroAssembler* masm, + Builtins::Name storage_extend, JSObject* object, int index, Map* transition, diff --git a/test/mjsunit/bugs/bug-109.js b/test/mjsunit/keyed-storage-extend.js similarity index 100% rename from test/mjsunit/bugs/bug-109.js rename to test/mjsunit/keyed-storage-extend.js -- 2.7.4