Revision: 5569
Author: [email protected]
Date: Thu Sep 30 10:39:31 2010
Log: Use existing global cell status as a hint when generating loads.
Review URL: http://codereview.chromium.org/3537003
http://code.google.com/p/v8/source/detail?r=5569
Modified:
/branches/bleeding_edge/src/arm/ic-arm.cc
/branches/bleeding_edge/src/ia32/codegen-ia32.cc
/branches/bleeding_edge/src/ia32/ic-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/ic.h
/branches/bleeding_edge/src/v8-counters.h
/branches/bleeding_edge/src/x64/ic-x64.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/arm/ic-arm.cc Wed Sep 29 08:08:38 2010
+++ /branches/bleeding_edge/src/arm/ic-arm.cc Thu Sep 30 10:39:31 2010
@@ -969,7 +969,8 @@
bool LoadIC::PatchInlinedContextualLoad(Address address,
Object* map,
- Object* cell) {
+ Object* cell,
+ bool is_dont_delete) {
// TODO(<bug#>): implement this.
return false;
}
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Fri Sep 24 00:53:59
2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Thu Sep 30 10:39:31
2010
@@ -9149,7 +9149,8 @@
: 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");
@@ -9158,6 +9159,11 @@
virtual void Generate();
Label* patch_site() { return &patch_site_; }
+
+ void set_is_dont_delete(bool value) {
+ ASSERT(is_contextual_);
+ is_dont_delete_ = value;
+ }
private:
Label patch_site_;
@@ -9165,6 +9171,7 @@
Register receiver_;
Handle<String> name_;
bool is_contextual_;
+ bool is_dont_delete_;
};
@@ -9181,8 +9188,8 @@
// 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 @@
// 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 @@
}
__ 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
=======================================
--- /branches/bleeding_edge/src/ia32/ic-ia32.cc Wed Sep 29 08:08:38 2010
+++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Sep 30 10:39:31 2010
@@ -1662,17 +1662,37 @@
// One byte opcode for mov ecx,0xXXXXXXXX.
+// Marks inlined contextual loads using all kinds of cells. Generated
+// code has the hole check:
+// mov reg, <cell>
+// mov reg, (<cell>, value offset)
+// cmp reg, <the hole>
+// 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, <cell>
+// mov reg, (<cell>, 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.
=======================================
--- /branches/bleeding_edge/src/ic.cc Wed Sep 29 10:38:37 2010
+++ /branches/bleeding_edge/src/ic.cc Thu Sep 30 10:39:31 2010
@@ -299,7 +299,10 @@
// 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 @@
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());
=======================================
--- /branches/bleeding_edge/src/ic.h Mon Sep 20 06:50:27 2010
+++ /branches/bleeding_edge/src/ic.h Thu Sep 30 10:39:31 2010
@@ -300,7 +300,8 @@
static bool PatchInlinedContextualLoad(Address address,
Object* map,
- Object* cell);
+ Object* cell,
+ bool is_dont_delete);
friend class IC;
};
=======================================
--- /branches/bleeding_edge/src/v8-counters.h Mon Sep 20 06:50:27 2010
+++ /branches/bleeding_edge/src/v8-counters.h Thu Sep 30 10:39:31 2010
@@ -161,6 +161,8 @@
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) \
=======================================
--- /branches/bleeding_edge/src/x64/ic-x64.cc Thu Sep 30 04:48:03 2010
+++ /branches/bleeding_edge/src/x64/ic-x64.cc Thu Sep 30 10:39:31 2010
@@ -1729,7 +1729,8 @@
bool LoadIC::PatchInlinedContextualLoad(Address address,
Object* map,
- Object* cell) {
+ Object* cell,
+ bool is_dont_delete) {
// TODO(<bug#>): implement this.
return false;
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Thu Sep 30 00:22:53 2010
+++ /branches/bleeding_edge/test/cctest/test-api.cc Thu Sep 30 10:39:31 2010
@@ -8099,7 +8099,7 @@
}
-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 @@
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 @@
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 @@
LocalContext context(&extensions);
v8_compile("gc();")->Run();
}
- CHECK_EQ(count, GetSurvivingGlobalObjectsCount());
+ CheckSurvivingGlobalObjectsCount(0);
}
}
@@ -11483,3 +11481,141 @@
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);
+ }
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
