From 92bb8970816fab000b0e3933c664d0641f727040 Mon Sep 17 00:00:00 2001 From: "sgjesse@chromium.org" Date: Tue, 30 Jun 2009 14:07:29 +0000 Subject: [PATCH] Tweaks to global object inline cache code. Remove the check for deleted properties in the global load inline cache if the property is known to be read only. Propegate the in loop flag for the global call inline cache. Changed the propagation of the code flags in the call stub compiler to compute these the same way for all types of call stubs and assert that the flags for the generated code is the same as those used for the cache lookup. Addressed a few comments from previous review in test-api.cc. Review URL: http://codereview.chromium.org/150101 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2308 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 23 +++++++++++------------ src/disassembler.cc | 3 +++ src/ia32/stub-cache-ia32.cc | 23 +++++++++++------------ src/ic.cc | 3 ++- src/stub-cache.cc | 26 ++++++++++++++++---------- src/stub-cache.h | 16 +++++++++------- test/cctest/test-api.cc | 10 +++++----- 7 files changed, 57 insertions(+), 47 deletions(-) diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 86dc1d1..71f2225 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -496,9 +496,7 @@ Object* StubCompiler::CompileLazyCompile(Code::Flags flags) { Object* CallStubCompiler::CompileCallField(Object* object, JSObject* holder, int index, - String* name, - Code::Flags flags) { - ASSERT_EQ(FIELD, Code::ExtractTypeFromFlags(flags)); + String* name) { // ----------- S t a t e ------------- // -- lr: return address // ----------------------------------- @@ -540,16 +538,14 @@ Object* CallStubCompiler::CompileCallField(Object* object, __ Jump(ic, RelocInfo::CODE_TARGET); // Return the generated code. - return GetCodeWithFlags(flags, name); + return GetCode(FIELD, name); } Object* CallStubCompiler::CompileCallConstant(Object* object, JSObject* holder, JSFunction* function, - CheckType check, - Code::Flags flags) { - ASSERT_EQ(CONSTANT_FUNCTION, Code::ExtractTypeFromFlags(flags)); + CheckType check) { // ----------- S t a t e ------------- // -- lr: return address // ----------------------------------- @@ -664,7 +660,7 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, if (function->shared()->name()->IsString()) { function_name = String::cast(function->shared()->name()); } - return GetCodeWithFlags(flags, function_name); + return GetCode(CONSTANT_FUNCTION, function_name); } @@ -1018,7 +1014,8 @@ Object* LoadStubCompiler::CompileLoadInterceptor(JSObject* object, Object* LoadStubCompiler::CompileLoadGlobal(JSGlobalObject* object, JSGlobalPropertyCell* cell, - String* name) { + String* name, + bool is_dont_delete) { // ----------- S t a t e ------------- // -- r2 : name // -- lr : return address @@ -1038,9 +1035,11 @@ Object* LoadStubCompiler::CompileLoadGlobal(JSGlobalObject* object, __ mov(r3, Operand(Handle(cell))); __ ldr(r0, FieldMemOperand(r3, JSGlobalPropertyCell::kValueOffset)); - // Check for deleted property. - __ cmp(r0, Operand(Factory::the_hole_value())); - __ b(eq, &miss); + // Check for deleted property if property can actually be deleted. + if (!is_dont_delete) { + __ cmp(r0, Operand(Factory::the_hole_value())); + __ b(eq, &miss); + } __ Ret(); diff --git a/src/disassembler.cc b/src/disassembler.cc index 937047f..e2f908d 100644 --- a/src/disassembler.cc +++ b/src/disassembler.cc @@ -243,6 +243,9 @@ static int DecodeIt(FILE* f, PropertyType type = code->type(); out.AddFormatted(", %s", Code::PropertyType2String(type)); } + if (code->ic_in_loop() == IN_LOOP) { + out.AddFormatted(", in_loop"); + } if (kind == Code::CALL_IC) { out.AddFormatted(", argc = %d", code->arguments_count()); } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index d2fbb83..83beb65 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -475,9 +475,7 @@ Object* StubCompiler::CompileLazyCompile(Code::Flags flags) { Object* CallStubCompiler::CompileCallField(Object* object, JSObject* holder, int index, - String* name, - Code::Flags flags) { - ASSERT_EQ(FIELD, Code::ExtractTypeFromFlags(flags)); + String* name) { // ----------- S t a t e ------------- // ----------------------------------- Label miss; @@ -518,16 +516,14 @@ Object* CallStubCompiler::CompileCallField(Object* object, __ jmp(ic, RelocInfo::CODE_TARGET); // Return the generated code. - return GetCodeWithFlags(flags, name); + return GetCode(FIELD, name); } Object* CallStubCompiler::CompileCallConstant(Object* object, JSObject* holder, JSFunction* function, - CheckType check, - Code::Flags flags) { - ASSERT_EQ(CONSTANT_FUNCTION, Code::ExtractTypeFromFlags(flags)); + CheckType check) { // ----------- S t a t e ------------- // ----------------------------------- Label miss; @@ -643,7 +639,7 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, if (function->shared()->name()->IsString()) { function_name = String::cast(function->shared()->name()); } - return GetCodeWithFlags(flags, function_name); + return GetCode(CONSTANT_FUNCTION, function_name); } @@ -1098,7 +1094,8 @@ Object* LoadStubCompiler::CompileLoadInterceptor(JSObject* receiver, Object* LoadStubCompiler::CompileLoadGlobal(JSGlobalObject* object, JSGlobalPropertyCell* cell, - String* name) { + String* name, + bool is_dont_delete) { // ----------- S t a t e ------------- // -- ecx : name // -- esp[0] : return address @@ -1118,9 +1115,11 @@ Object* LoadStubCompiler::CompileLoadGlobal(JSGlobalObject* object, __ mov(eax, Immediate(Handle(cell))); __ mov(eax, FieldOperand(eax, JSGlobalPropertyCell::kValueOffset)); - // Check for deleted property. - __ cmp(eax, Factory::the_hole_value()); - __ j(equal, &miss, not_taken); + // Check for deleted property if property can actually be deleted. + if (!is_dont_delete) { + __ cmp(eax, Factory::the_hole_value()); + __ j(equal, &miss, not_taken); + } __ ret(0); diff --git a/src/ic.cc b/src/ic.cc index 9483589..e062dd9 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -632,7 +632,8 @@ void LoadIC::UpdateCaches(LookupResult* lookup, if (lookup->holder() != *global) return; JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(global->GetPropertyCell(lookup)); - code = StubCache::ComputeLoadGlobal(*name, *global, cell); + code = StubCache::ComputeLoadGlobal(*name, *global, + cell, lookup->IsDontDelete()); } else { // There is only one shared stub for loading normalized // properties. It does not traverse the prototype chain, so the diff --git a/src/stub-cache.cc b/src/stub-cache.cc index 459fe55..1999d13 100644 --- a/src/stub-cache.cc +++ b/src/stub-cache.cc @@ -174,12 +174,13 @@ Object* StubCache::ComputeLoadNormal(String* name, JSObject* receiver) { Object* StubCache::ComputeLoadGlobal(String* name, JSGlobalObject* receiver, - JSGlobalPropertyCell* cell) { + JSGlobalPropertyCell* cell, + bool is_dont_delete) { Code::Flags flags = Code::ComputeMonomorphicFlags(Code::LOAD_IC, NORMAL); Object* code = receiver->map()->FindInCodeCache(name, flags); if (code->IsUndefined()) { LoadStubCompiler compiler; - code = compiler.CompileLoadGlobal(receiver, cell, name); + code = compiler.CompileLoadGlobal(receiver, cell, name, is_dont_delete); if (code->IsFailure()) return code; LOG(CodeCreateEvent(Logger::LOAD_IC_TAG, Code::cast(code), name)); Object* result = receiver->map()->UpdateCodeCache(name, Code::cast(code)); @@ -443,9 +444,10 @@ Object* StubCache::ComputeCallConstant(int argc, // caches. if (!function->is_compiled()) return Failure::InternalError(); // Compile the stub - only create stubs for fully compiled functions. - CallStubCompiler compiler(argc); - code = compiler.CompileCallConstant(object, holder, function, check, flags); + CallStubCompiler compiler(argc, in_loop); + code = compiler.CompileCallConstant(object, holder, function, check); if (code->IsFailure()) return code; + ASSERT_EQ(flags, Code::cast(code)->flags()); LOG(CodeCreateEvent(Logger::CALL_IC_TAG, Code::cast(code), name)); Object* result = map->UpdateCodeCache(name, Code::cast(code)); if (result->IsFailure()) return result; @@ -476,9 +478,10 @@ Object* StubCache::ComputeCallField(int argc, argc); Object* code = map->FindInCodeCache(name, flags); if (code->IsUndefined()) { - CallStubCompiler compiler(argc); - code = compiler.CompileCallField(object, holder, index, name, flags); + CallStubCompiler compiler(argc, in_loop); + code = compiler.CompileCallField(object, holder, index, name); if (code->IsFailure()) return code; + ASSERT_EQ(flags, Code::cast(code)->flags()); LOG(CodeCreateEvent(Logger::CALL_IC_TAG, Code::cast(code), name)); Object* result = map->UpdateCodeCache(name, Code::cast(code)); if (result->IsFailure()) return result; @@ -509,9 +512,10 @@ Object* StubCache::ComputeCallInterceptor(int argc, argc); Object* code = map->FindInCodeCache(name, flags); if (code->IsUndefined()) { - CallStubCompiler compiler(argc); + CallStubCompiler compiler(argc, NOT_IN_LOOP); code = compiler.CompileCallInterceptor(object, holder, name); if (code->IsFailure()) return code; + ASSERT_EQ(flags, Code::cast(code)->flags()); LOG(CodeCreateEvent(Logger::CALL_IC_TAG, Code::cast(code), name)); Object* result = map->UpdateCodeCache(name, Code::cast(code)); if (result->IsFailure()) return result; @@ -536,7 +540,8 @@ Object* StubCache::ComputeCallGlobal(int argc, JSGlobalObject* receiver, JSGlobalPropertyCell* cell, JSFunction* function) { - Code::Flags flags = Code::ComputeMonomorphicFlags(Code::CALL_IC, NORMAL); + Code::Flags flags = + Code::ComputeMonomorphicFlags(Code::CALL_IC, NORMAL, in_loop, argc); Object* code = receiver->map()->FindInCodeCache(name, flags); if (code->IsUndefined()) { // If the function hasn't been compiled yet, we cannot do it now @@ -544,9 +549,10 @@ Object* StubCache::ComputeCallGlobal(int argc, // internal error which will make sure we do not update any // caches. if (!function->is_compiled()) return Failure::InternalError(); - CallStubCompiler compiler(argc); + CallStubCompiler compiler(argc, in_loop); code = compiler.CompileCallGlobal(receiver, cell, function, name); if (code->IsFailure()) return code; + ASSERT_EQ(flags, Code::cast(code)->flags()); LOG(CodeCreateEvent(Logger::CALL_IC_TAG, Code::cast(code), name)); Object* result = receiver->map()->UpdateCodeCache(name, Code::cast(code)); if (result->IsFailure()) return code; @@ -992,7 +998,7 @@ Object* CallStubCompiler::GetCode(PropertyType type, String* name) { int argc = arguments_.immediate(); Code::Flags flags = Code::ComputeMonomorphicFlags(Code::CALL_IC, type, - NOT_IN_LOOP, + in_loop_, argc); return GetCodeWithFlags(flags, name); } diff --git a/src/stub-cache.h b/src/stub-cache.h index 0a00807..577e04b 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -80,7 +80,8 @@ class StubCache : public AllStatic { static Object* ComputeLoadGlobal(String* name, JSGlobalObject* receiver, - JSGlobalPropertyCell* cell); + JSGlobalPropertyCell* cell, + bool is_dont_delete); // --- @@ -434,7 +435,8 @@ class LoadStubCompiler: public StubCompiler { Object* CompileLoadGlobal(JSGlobalObject* object, JSGlobalPropertyCell* holder, - String* name); + String* name, + bool is_dont_delete); private: Object* GetCode(PropertyType type, String* name); @@ -501,18 +503,17 @@ class KeyedStoreStubCompiler: public StubCompiler { class CallStubCompiler: public StubCompiler { public: - explicit CallStubCompiler(int argc) : arguments_(argc) { } + explicit CallStubCompiler(int argc, InLoopFlag in_loop) + : arguments_(argc), in_loop_(in_loop) { } Object* CompileCallField(Object* object, JSObject* holder, int index, - String* name, - Code::Flags flags); + String* name); Object* CompileCallConstant(Object* object, JSObject* holder, JSFunction* function, - CheckType check, - Code::Flags flags); + CheckType check); Object* CompileCallInterceptor(Object* object, JSObject* holder, String* name); @@ -523,6 +524,7 @@ class CallStubCompiler: public StubCompiler { private: const ParameterCount arguments_; + const InLoopFlag in_loop_; const ParameterCount& arguments() { return arguments_; } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index f1f247c..426b720 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -6149,16 +6149,16 @@ THREADED_TEST(TurnOnAccessCheck) { // Call f1 one time and f2 a number of times. This will ensure that f1 still // uses the runtime system to retreive property a whereas f2 uses global load - // inline cache is used. - CHECK(!f1->Call(global, 0, NULL)->IsUndefined()); + // inline cache. + CHECK(f1->Call(global, 0, NULL)->Equals(v8_num(1))); for (int i = 0; i < 4; i++) { - CHECK(!f2->Call(global, 0, NULL)->IsUndefined()); + CHECK(f2->Call(global, 0, NULL)->Equals(v8_num(1))); } // Same for g1 and g2. - CHECK(!g1->Call(global, 0, NULL)->IsUndefined()); + CHECK(g1->Call(global, 0, NULL)->Equals(v8_num(1))); for (int i = 0; i < 4; i++) { - CHECK(!g2->Call(global, 0, NULL)->IsUndefined()); + CHECK(g2->Call(global, 0, NULL)->Equals(v8_num(1))); } // Detach the global and turn on access check. -- 2.7.4