From: vitalyr@chromium.org Date: Thu, 30 Sep 2010 17:39:31 +0000 (+0000) Subject: Use existing global cell status as a hint when generating loads. X-Git-Tag: upstream/4.7.83~21130 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7228d867cb7af82d2467075388ab4bcb435618bb;p=platform%2Fupstream%2Fv8.git Use existing global cell status as a hint when generating loads. Review URL: http://codereview.chromium.org/3537003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5569 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc index eab4c6e..7f83d14 100644 --- a/src/arm/ic-arm.cc +++ b/src/arm/ic-arm.cc @@ -969,7 +969,8 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { bool LoadIC::PatchInlinedContextualLoad(Address address, Object* map, - Object* cell) { + Object* cell, + bool is_dont_delete) { // TODO(): implement this. return false; } diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 9c8573c..c006b86 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -9149,7 +9149,8 @@ class DeferredReferenceGetNamedValue: public DeferredCode { : dst_(dst), receiver_(receiver), name_(name), - is_contextual_(is_contextual) { + is_contextual_(is_contextual), + is_dont_delete_(false) { set_comment(is_contextual ? "[ DeferredReferenceGetNamedValue (contextual)" : "[ DeferredReferenceGetNamedValue"); @@ -9159,12 +9160,18 @@ class DeferredReferenceGetNamedValue: public DeferredCode { Label* patch_site() { return &patch_site_; } + void set_is_dont_delete(bool value) { + ASSERT(is_contextual_); + is_dont_delete_ = value; + } + private: Label patch_site_; Register dst_; Register receiver_; Handle name_; bool is_contextual_; + bool is_dont_delete_; }; @@ -9181,8 +9188,8 @@ void DeferredReferenceGetNamedValue::Generate() { // The call must be followed by: // - a test eax instruction to indicate that the inobject property // case was inlined. - // - a mov ecx instruction to indicate that the contextual property - // load was inlined. + // - a mov ecx or mov edx instruction to indicate that the + // contextual property load was inlined. // // Store the delta to the map check instruction here in the test // instruction. Use masm_-> instead of the __ macro since the @@ -9191,8 +9198,11 @@ void DeferredReferenceGetNamedValue::Generate() { // Here we use masm_-> instead of the __ macro because this is the // instruction that gets patched and coverage code gets in the way. if (is_contextual_) { - masm_->mov(ecx, -delta_to_patch_site); + masm_->mov(is_dont_delete_ ? edx : ecx, -delta_to_patch_site); __ IncrementCounter(&Counters::named_load_global_inline_miss, 1); + if (is_dont_delete_) { + __ IncrementCounter(&Counters::dont_delete_hint_miss, 1); + } } else { masm_->test(eax, Immediate(-delta_to_patch_site)); __ IncrementCounter(&Counters::named_load_inline_miss, 1); @@ -9436,9 +9446,34 @@ Result CodeGenerator::EmitNamedLoad(Handle name, bool is_contextual) { } __ mov(result.reg(), FieldOperand(result.reg(), JSGlobalPropertyCell::kValueOffset)); - __ cmp(result.reg(), Factory::the_hole_value()); - deferred->Branch(equal); + bool is_dont_delete = false; + if (!info_->closure().is_null()) { + // When doing lazy compilation we can check if the global cell + // already exists and use its "don't delete" status as a hint. + AssertNoAllocation no_gc; + v8::internal::GlobalObject* global_object = + info_->closure()->context()->global(); + LookupResult lookup; + global_object->LocalLookupRealNamedProperty(*name, &lookup); + if (lookup.IsProperty() && lookup.type() == NORMAL) { + ASSERT(lookup.holder() == global_object); + ASSERT(global_object->property_dictionary()->ValueAt( + lookup.GetDictionaryEntry())->IsJSGlobalPropertyCell()); + is_dont_delete = lookup.IsDontDelete(); + } + } + deferred->set_is_dont_delete(is_dont_delete); + if (!is_dont_delete) { + __ cmp(result.reg(), Factory::the_hole_value()); + deferred->Branch(equal); + } else if (FLAG_debug_code) { + __ cmp(result.reg(), Factory::the_hole_value()); + __ Check(not_equal, "DontDelete cells can't contain the hole"); + } __ IncrementCounter(&Counters::named_load_global_inline, 1); + if (is_dont_delete) { + __ IncrementCounter(&Counters::dont_delete_hint_hit, 1); + } } else { // The initial (invalid) offset has to be large enough to force a 32-bit // instruction encoding to allow patching with an arbitrary offset. Use diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index a2990a2..b5f4dee 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -1662,17 +1662,37 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { // One byte opcode for mov ecx,0xXXXXXXXX. +// Marks inlined contextual loads using all kinds of cells. Generated +// code has the hole check: +// mov reg, +// mov reg, (, value offset) +// cmp reg, +// je slow +// ;; use reg static const byte kMovEcxByte = 0xB9; +// One byte opcode for mov edx,0xXXXXXXXX. +// Marks inlined contextual loads using only "don't delete" +// cells. Generated code doesn't have the hole check: +// mov reg, +// mov reg, (, value offset) +// ;; use reg +static const byte kMovEdxByte = 0xBA; + bool LoadIC::PatchInlinedContextualLoad(Address address, Object* map, - Object* cell) { + Object* cell, + bool is_dont_delete) { // The address of the instruction following the call. Address mov_instruction_address = address + Assembler::kCallTargetAddressOffset; - // If the instruction following the call is not a cmp eax, nothing - // was inlined. - if (*mov_instruction_address != kMovEcxByte) return false; + // If the instruction following the call is not a mov ecx/edx, + // nothing was inlined. + byte b = *mov_instruction_address; + if (b != kMovEcxByte && b != kMovEdxByte) return false; + // If we don't have the hole check generated, we can only support + // "don't delete" cells. + if (b == kMovEdxByte && !is_dont_delete) return false; Address delta_address = mov_instruction_address + 1; // The delta to the start of the map check instruction. diff --git a/src/ic.cc b/src/ic.cc index a9c2a48..adf365a 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -299,7 +299,10 @@ void LoadIC::ClearInlinedVersion(Address address) { // present) to guarantee failure by holding an invalid map (the null // value). The offset can be patched to anything. PatchInlinedLoad(address, Heap::null_value(), 0); - PatchInlinedContextualLoad(address, Heap::null_value(), Heap::null_value()); + PatchInlinedContextualLoad(address, + Heap::null_value(), + Heap::null_value(), + true); } @@ -848,7 +851,10 @@ Object* LoadIC::Load(State state, Handle object, Handle name) { JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast( lookup.holder()->property_dictionary()->ValueAt( lookup.GetDictionaryEntry())); - if (PatchInlinedContextualLoad(address(), map, cell)) { + if (PatchInlinedContextualLoad(address(), + map, + cell, + lookup.IsDontDelete())) { set_target(megamorphic_stub()); TRACE_IC_NAMED("[LoadIC : inline contextual patch %s]\n", name); ASSERT(cell->value() != Heap::the_hole_value()); diff --git a/src/ic.h b/src/ic.h index a5fada0..437e45a 100644 --- a/src/ic.h +++ b/src/ic.h @@ -300,7 +300,8 @@ class LoadIC: public IC { static bool PatchInlinedContextualLoad(Address address, Object* map, - Object* cell); + Object* cell, + bool is_dont_delete); friend class IC; }; diff --git a/src/v8-counters.h b/src/v8-counters.h index a8eb9d2..c664638 100644 --- a/src/v8-counters.h +++ b/src/v8-counters.h @@ -161,6 +161,8 @@ namespace internal { SC(named_load_inline_miss, V8.NamedLoadInlineMiss) \ SC(named_load_global_inline, V8.NamedLoadGlobalInline) \ SC(named_load_global_inline_miss, V8.NamedLoadGlobalInlineMiss) \ + SC(dont_delete_hint_hit, V8.DontDeleteHintHit) \ + SC(dont_delete_hint_miss, V8.DontDeleteHintMiss) \ SC(named_load_global_stub, V8.NamedLoadGlobalStub) \ SC(named_load_global_stub_miss, V8.NamedLoadGlobalStubMiss) \ SC(keyed_store_field, V8.KeyedStoreField) \ diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc index a76ba46..1d95b7f 100644 --- a/src/x64/ic-x64.cc +++ b/src/x64/ic-x64.cc @@ -1729,7 +1729,8 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { bool LoadIC::PatchInlinedContextualLoad(Address address, Object* map, - Object* cell) { + Object* cell, + bool is_dont_delete) { // TODO(): implement this. return false; } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 527c9a3..f1a6ead 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8099,7 +8099,7 @@ static int GetGlobalObjectsCount() { } -static int GetSurvivingGlobalObjectsCount() { +static void CheckSurvivingGlobalObjectsCount(int expected) { // We need to collect all garbage twice to be sure that everything // has been collected. This is because inline caches are cleared in // the first garbage collection but some of the maps have already @@ -8109,9 +8109,9 @@ static int GetSurvivingGlobalObjectsCount() { i::Heap::CollectAllGarbage(false); int count = GetGlobalObjectsCount(); #ifdef DEBUG - if (count > 0) i::Heap::TracePathToGlobal(); + if (count != expected) i::Heap::TracePathToGlobal(); #endif - return count; + CHECK_EQ(expected, count); } @@ -8120,25 +8120,23 @@ TEST(DontLeakGlobalObjects) { v8::V8::Initialize(); - int count = GetSurvivingGlobalObjectsCount(); - for (int i = 0; i < 5; i++) { { v8::HandleScope scope; LocalContext context; } - CHECK_EQ(count, GetSurvivingGlobalObjectsCount()); + CheckSurvivingGlobalObjectsCount(0); { v8::HandleScope scope; LocalContext context; v8_compile("Date")->Run(); } - CHECK_EQ(count, GetSurvivingGlobalObjectsCount()); + CheckSurvivingGlobalObjectsCount(0); { v8::HandleScope scope; LocalContext context; v8_compile("/aaa/")->Run(); } - CHECK_EQ(count, GetSurvivingGlobalObjectsCount()); + CheckSurvivingGlobalObjectsCount(0); { v8::HandleScope scope; const char* extension_list[] = { "v8/gc" }; @@ -8146,7 +8144,7 @@ TEST(DontLeakGlobalObjects) { LocalContext context(&extensions); v8_compile("gc();")->Run(); } - CHECK_EQ(count, GetSurvivingGlobalObjectsCount()); + CheckSurvivingGlobalObjectsCount(0); } } @@ -11483,3 +11481,141 @@ TEST(BooleanCheckMultipleContexts) { ExpectString(code, ""); } } + + +TEST(DontDeleteCellLoadIC) { + const char* function_code = + "function readCell() { while (true) { return cell; } }"; + + { + // Run the code twice in the first context to initialize the load + // IC for a don't delete cell. + v8::HandleScope scope; + LocalContext context1; + CompileRun("var cell = \"first\";"); + ExpectBoolean("delete cell", false); + CompileRun(function_code); + ExpectString("readCell()", "first"); + ExpectString("readCell()", "first"); + } + + { + // Use a deletable cell in the second context. + v8::HandleScope scope; + LocalContext context2; + CompileRun("cell = \"second\";"); + CompileRun(function_code); + ExpectString("readCell()", "second"); + ExpectBoolean("delete cell", true); + ExpectString("(function() {" + " try {" + " return readCell();" + " } catch(e) {" + " return e.toString();" + " }" + "})()", + "ReferenceError: cell is not defined"); + CompileRun("cell = \"new_second\";"); + i::Heap::CollectAllGarbage(true); + ExpectString("readCell()", "new_second"); + ExpectString("readCell()", "new_second"); + } +} + + +TEST(DontDeleteCellLoadICForceDelete) { + const char* function_code = + "function readCell() { while (true) { return cell; } }"; + + // Run the code twice to initialize the load IC for a don't delete + // cell. + v8::HandleScope scope; + LocalContext context; + CompileRun("var cell = \"value\";"); + ExpectBoolean("delete cell", false); + CompileRun(function_code); + ExpectString("readCell()", "value"); + ExpectString("readCell()", "value"); + + // Delete the cell using the API and check the inlined code works + // correctly. + CHECK(context->Global()->ForceDelete(v8_str("cell"))); + ExpectString("(function() {" + " try {" + " return readCell();" + " } catch(e) {" + " return e.toString();" + " }" + "})()", + "ReferenceError: cell is not defined"); +} + + +TEST(DontDeleteCellLoadICAPI) { + const char* function_code = + "function readCell() { while (true) { return cell; } }"; + + // Run the code twice to initialize the load IC for a don't delete + // cell created using the API. + v8::HandleScope scope; + LocalContext context; + context->Global()->Set(v8_str("cell"), v8_str("value"), v8::DontDelete); + ExpectBoolean("delete cell", false); + CompileRun(function_code); + ExpectString("readCell()", "value"); + ExpectString("readCell()", "value"); + + // Delete the cell using the API and check the inlined code works + // correctly. + CHECK(context->Global()->ForceDelete(v8_str("cell"))); + ExpectString("(function() {" + " try {" + " return readCell();" + " } catch(e) {" + " return e.toString();" + " }" + "})()", + "ReferenceError: cell is not defined"); +} + + +TEST(GlobalLoadICGC) { + const char* function_code = + "function readCell() { while (true) { return cell; } }"; + + // Check inline load code for a don't delete cell is cleared during + // GC. + { + v8::HandleScope scope; + LocalContext context; + CompileRun("var cell = \"value\";"); + ExpectBoolean("delete cell", false); + CompileRun(function_code); + ExpectString("readCell()", "value"); + ExpectString("readCell()", "value"); + } + { + v8::HandleScope scope; + LocalContext context2; + // Hold the code object in the second context. + CompileRun(function_code); + CheckSurvivingGlobalObjectsCount(1); + } + + // Check inline load code for a deletable cell is cleared during GC. + { + v8::HandleScope scope; + LocalContext context; + CompileRun("cell = \"value\";"); + CompileRun(function_code); + ExpectString("readCell()", "value"); + ExpectString("readCell()", "value"); + } + { + v8::HandleScope scope; + LocalContext context2; + // Hold the code object in the second context. + CompileRun(function_code); + CheckSurvivingGlobalObjectsCount(1); + } +}